Make MediaTrackGraph a tail-dispatchable(-only) nsISerialEventTarget
Categories
(Core :: WebRTC: Audio/Video, task, P3)
Tracking
()
People
(Reporter: pehrsons, Assigned: pehrsons)
References
(Blocks 2 open bugs, Regressed 2 open bugs)
Details
Attachments
(12 files, 1 obsolete file)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
The idea is to scrap the current front-/back-buffers of messages used for dispatch to main thread stable state, and instead use (mainly) common code (see TaskDispatcher.h, nsIDirectTaskDispatcher, nsITailDispatcher (bug 1645785) or AbstractThread). This enables higher-order stuff like MozPromise, MediaEvents, StateMirroring for use with the graph.
| Assignee | ||
Comment 1•8 months ago
|
||
| Assignee | ||
Comment 2•8 months ago
|
||
| Assignee | ||
Comment 3•8 months ago
|
||
| Assignee | ||
Comment 4•8 months ago
|
||
This patch makes MediaTrackGraphImpl implement AbstractThread and its base
interface nsISerialEventTarget.
This makes the backing store for ControlMessages change to nsIRunnable.
To facilitate runnables that can optionally run during shutdown, a new
convention is introduced: An nsICancelableRunnable that ignores cancellation
will run during shutdown. The dispatch of an nsIRunnable that does not implement
nsICancelableRunnable will result in the adding of a wrapper that
releases the nsIRunnable on cancellation.
MessageBlocks are removed, as the graph no longer will manage the batching of
tasks dispatched from main thread. The main thread's tail dispatcher will manage
batching instead, and dispatched per-thread group runnables will enter
mBackMessageQueue regularly. The same mechanism now works from any thread. Only
main thread dispatches affect the graph's lifecycle, however.
For a non-main thread to dispatch tasks to the graph, it will have to be sure to
keep the graph alive. The only guarantee for this is still to hold onto a live
track.
Any thread that is used to dispatch a task to the graph must support tail
dispatch. While not strictly required for the function of the graph, this
enforces the old mechanism of running messages from main thread atomically, and
spreads that behavior to any other thread. It also reduces contention on the
graph's Monitor.
Dispatching from the graph to any other thread should also use tail dispatch for
performance reasons -- e.g dispatching to the busy main thread may suffer from
lock contention which is not ideal on the real-time audio thread. However,
enforcing tail dispatch when dispatching from the graph thread is not possible
without going through a proxy function. This is by convention for now.
| Assignee | ||
Comment 5•8 months ago
|
||
This patch does two things to support AudioWorklet:
- Supports target shutdown tasks.
- Exempts the IPC I/O Child thread from the tail dispatch requirement.
The IPC I/O Child thread does not support tail dispatch, and there are no
ordering requirements between those messages and any from other threads.
Prior to this patch, these messages went through GraphRunner::mThread. It was
given to the MessageChannel opened in the AudioWorklet through
GetCurrentSerialEventTarget().
| Assignee | ||
Comment 6•8 months ago
|
||
| Assignee | ||
Comment 7•8 months ago
|
||
| Assignee | ||
Comment 8•8 months ago
|
||
| Assignee | ||
Comment 9•8 months ago
|
||
For MediaTrackGraph to be able to switch to its manual batching of control
messages to tail dispatch, the message atomicity guarantee it provides needs to
be retained. This patch introduces an atomicity mode for tail dispatch to
support the graph.
| Assignee | ||
Comment 10•8 months ago
|
||
This test is racy. The contestants are:
- The canvas capturing the first frame and adding it to the graph track on the
main thread. - VideoOutput handling the enabled state change and inventing a black frame for
rendering on the graph thread.
With this patch series, dispatches from main to the graph thread now use tail
dispatch, which is slightly different from the previous behavior of queueing up
runnables for dispatch during stable state. This means the timing
characteristics of when such dispatches happen has changed, which affects the
timing of when the runnables run in the graph.
Exact expected behavior in this case is still being discussed on the spec,
see e.g. https://github.com/w3c/mediacapture-main/issues/1011.
The expected outcome of this test has changed
Updated•5 months ago
|
| Assignee | ||
Comment 11•5 months ago
|
||
| Assignee | ||
Comment 12•3 months ago
|
||
Updated•3 months ago
|
| Assignee | ||
Updated•3 months ago
|
| Assignee | ||
Comment 13•2 months ago
|
||
Comment 14•15 days ago
|
||
Comment 15•15 days ago
|
||
Comment 16•15 days ago
|
||
Backed out for causing bustage @ AbstractThread.h and Gtest failures @ StateWatching.h
| Assignee | ||
Updated•15 days ago
|
Comment 17•14 days ago
|
||
Comment 18•14 days ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/0074d8c37b03
https://hg.mozilla.org/mozilla-central/rev/07992a96a786
https://hg.mozilla.org/mozilla-central/rev/057cda252e8b
https://hg.mozilla.org/mozilla-central/rev/7668d22f06ad
https://hg.mozilla.org/mozilla-central/rev/e7ad1bf72931
https://hg.mozilla.org/mozilla-central/rev/1b7fbc53f0e0
https://hg.mozilla.org/mozilla-central/rev/af7418e64820
https://hg.mozilla.org/mozilla-central/rev/7aaca6bbf089
https://hg.mozilla.org/mozilla-central/rev/16047aca2a0b
https://hg.mozilla.org/mozilla-central/rev/b6637fb55859
https://hg.mozilla.org/mozilla-central/rev/6b7d59a2dc94
https://hg.mozilla.org/mozilla-central/rev/81f70526e4dc
Comment 19•1 day ago
|
||
Updated•1 day ago
|
Updated•22 hours ago
|
Description
•