1173001 - Make MediaDecoderReader compatible with mirroring

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Bobby Holley (:bholley)]

This is necessary in order to support mirroring in MediaDecoderReader.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Part 1 - Initialize reader task queue in the constructor. v1

This is a necessary step towards having mirrored/canonical values, since those
need to know the task queue in their constructor.

Comment 2

[Bobby Holley (:bholley)]

Attachment: Part 2 - Fix up some task queue naming to make MediaDecoderReader consistent with MDSM. v1

Comment 3

[Bobby Holley (:bholley)]

Generalizing the bug a bit.

Comment 4

[Bobby Holley (:bholley)]

Comment on attachment 8617413 [details] [diff] [review]
Part 1 - Initialize reader task queue in the constructor. v1

This is going to need some tweaking.

Comment 5

[Jean-Yves Avenard [:jya]]

While at it, it would be great if the task queue constructor could be called on something else but the main thread.

As it reads a preference to find out the size of the threads pool, this asserts when called on a thread other than the main thread.

Comment 6

[Bobby Holley (:bholley)]

(In reply to Jean-Yves Avenard [:jya] from comment #5)
> While at it, it would be great if the task queue constructor could be called
> on something else but the main thread.

That seems like a good thing to fix, but I don't plan on doing it in this bug.

Comment 7

[Bobby Holley (:bholley)]

Attachment: Part 0 - Explicitly instantiate WebReader in MediaSourceReader. v1

This will allow us to pass the borrowed task queue. It's also more explicit
about the fact that we only support mp4 and (sort of) webm.

Comment 8

[Bobby Holley (:bholley)]

Attachment: Part 2 - Initialize reader task queue in the constructor. v1

This is a necessary step towards having mirrored/canonical values, since those
need to know the task queue in their constructor.

Comment 9

[Bobby Holley (:bholley)]

Attachment: Part 3 - Fix up some task queue naming to make MediaDecoderReader consistent with MDSM. v1

Comment 10

[Bobby Holley (:bholley)]

Attachment: Part 4 - Enable tail dispatch for the decode task queue. v1

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8617698 [details] [diff] [review]
Part 2 - Initialize reader task queue in the constructor. v1

Review of attachment 8617698 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/MediaDecoderReader.h
@@ +312,5 @@
>    // Reference to the owning decoder object.
>    AbstractMediaDecoder* mDecoder;
>  
> +  // Decode task queue.
> +  nsRefPtr<MediaTaskQueue> mTaskQueue;

Why changing from private to protected?

Comment 12

[Bobby Holley (:bholley)]

(In reply to JW Wang [:jwwang] from comment #11)
> Why changing from private to protected?

It's just about the initialization order - the task queue needs to be initialized early so that mirror initializers can use it.

Comment 13

[Bobby Holley (:bholley)]

Thanks for the fast reviews!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a41aee7baf2

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6289cdbce7b7
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7b684137f22
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e722555cc6d
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2f35acc27a2

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6289cdbce7b7
https://hg.mozilla.org/mozilla-central/rev/e7b684137f22
https://hg.mozilla.org/mozilla-central/rev/0e722555cc6d
https://hg.mozilla.org/mozilla-central/rev/f2f35acc27a2