1650696 - Move threading management away from the MediaDataDecoder

Status: RESOLVED FIXED

(Core :: Audio/Video, enhancement, P3)

Description

[Shravan Narayan]

Calls to switch threads/InvokeAsync can be removed from MediaDataDecoders except for the decoders that actually requires multi-threading. Instead for single threaded decoders, the work would be done on the current thread, so the caller is expected to invoke the MediaDataDecoder functions on the right thread.

This architecture would allow for a more straightforward port for wasm sandboxed media decoders.

Comment 1

[Jean-Yves Avenard [:jya]]

Ok.

Here is the plan:

MediaDataDecoder will no longer be expected to work on a given TaskQueue; they will perform the job whereever they want it to. So for audio decoder this will typically be a sync job immediately returning a resolved promise as usual.

All the video decoders using multi-threaded framework will have to deal with thread as needed.

From an implementation point of you, it will be up to the user of the MediaDataDecoder (e.g. MediaFormatReader) to dispatch the given decoding operation on the thread/taskqueue it wants.

This will likely result in a much easier implementation for all decoders and simplify how to deal with the decoding TaskQueue life cycle.

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1650696 - P1. Remove remnant of AbstractThread in EME and GMP decoders. r?jolin

Once upon a time, having an AbstractThread was required in order to use MozPromise; this is no longer the case.

Also, now all nsIThread support DirectTask dispatching making the need to wrap a nsIThread in a XPCOMThreadWrapper unecessary.

We probably don't need a dedicated GMPThread and could likely use a BackgroundTaskqueue instead.

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1650696 - P2. Add missing methods to MediaDataDecoderProxy. r?jolin

Depends on D86927

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1650696 - P4. Remove the expectation for a MediaDataDecoder to work on a specified TaskQueue. r?jolin

It will now be up to the caller to determine where the decoder is going to run. This allows to simplify the audio decoders so that they can run synchronously and be wrapped in a Wasm sandbox (which doesn't support multi-threading)

The structure guarantees that all MediaDataDecoder methods are called on the same thread it's been initialised.

To achieve this, wherever a MediaDataDecoder was created, we wrap it in a MediaDataDemuxerProxy that ensures that all methods are running on the given thread.

We keep the behaviour of all methods in all MediaDataDecoder to assert that they are running on the expected thread for diagnostic purposes. It could go in the future.

Depends on D86928

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1650696 - P3. Add missing methods to AllocationPolicy. r?jolin

Depends on D86929

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1650696 - P5. Explicitly use decoder taskqueue with process-incentive codec (videos). r?jolin

Upon further thinking, I believe we should still make individual decoders behave in a way that they never block the thread on which they are called for too long. So we revert the earlier change for the video decoders and have them dispatch on their own taskqueue.

The Apple decoder is mostly entirely asynchronous, with the exception of the drain method which could block.
We exclude the android and omx decoders are the framework they use is 100% asynchronous and already operate on another thread.

Depends on D86929

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52cfcbf99785
P1. Remove remnant of AbstractThread in EME and GMP decoders. r=jolin
https://hg.mozilla.org/integration/autoland/rev/3ddc585f69cf
P2. Add missing methods to MediaDataDecoderProxy. r=jolin
https://hg.mozilla.org/integration/autoland/rev/aee9055c07c2
P3. Add missing methods to AllocationPolicy. r=jolin
https://hg.mozilla.org/integration/autoland/rev/f7243c8b4772
P4. Remove the expectation for a MediaDataDecoder to work on a specified TaskQueue. r=jolin

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/52cfcbf99785
https://hg.mozilla.org/mozilla-central/rev/3ddc585f69cf
https://hg.mozilla.org/mozilla-central/rev/aee9055c07c2
https://hg.mozilla.org/mozilla-central/rev/f7243c8b4772