1644647 - Linearize AudioContextOperation resolution

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P3)

Description

[Karl Tomlinson (:karlt)]

+++ This bug was initially created as a clone of Bug #1285290 +++

We still have a fast-path when something is still using the AudioCallbackDriver, where we tell the content to resolve the promise immediately without sending it to the GraphDriver.

This causes the events to not stay in order, and blow up the state change assert back in AudioContext::OnStateChanged.

Comment 1

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&selectedTaskRun=U9vBFdIoQwGPPi9Gn13UWw-0&revision=f4d6e452e3fb398b75224b7fbb85bee0eed98078

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: Bug 1644647 explicitly initialize mFallbackDriverState r?padenot

There is no change in behavior here because the default constructor is zero initialized
https://searchfox.org/mozilla-central/rev/bc3600def806859c31b2c7ac06e3d69271052a89/mfbt/Atomics.h#287
and None is zero.
https://searchfox.org/mozilla-central/rev/bc3600def806859c31b2c7ac06e3d69271052a89/dom/media/GraphDriver.h#763

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1285290 use strong pointers for tracks in ApplyAudioContextOperation() r?padenot

This will be helpful when delaying resumption of tracks until there is an AudioCallbackDriver.

The memory for the array is also transferred to the callee to save an allocation.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: introduce PendingResumeOperation

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1644647 rewrite AudioContextOperation handling using PendingResumeOperation r?padenot

The key change here is that AudioContextOperation promises for the same
AudioContext are resolved in order as long as the graph is running.
There is one remaining case where AudioContextOperation promises can be
resolved out of order, which is when
AudioContextOperationControlMessage::RunDuringShutdown() resolves a Close
operation shortly after Run() has queued its update via mUpdateRunnables.
mUpdateRunnables are run after controlMessagesToRunDuringShutdown.
https://searchfox.org/mozilla-central/rev/ea7f70dac1c5fd18400f6d2a92679777d4b21492/dom/media/MediaTrackGraph.cpp#1793,1803

Pending resume operations are now stored on the graph instead of on
mNextDriver. This means there is no need to move operations between drivers
when switching from one mNextDriver to another, the code for which was
previously missing.

Suspend operations are performed immediately, because even a callback driver
can render nothing. Tracks are not resumed until either there is an
AudioCallbackDriver to deliver the rendered audio or the Resume operation is
canceled by a subsequent Suspend.

While the graph is running, DispatchToMainThreadStableState() is used
consistently, whereas previously this was mixed with direct Dispatch() to the
main thread, which could have the runnable Run() before main thread state had
been updated for rendering up to the time of a Suspend.

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1644647 use the same mechanism for the first suspended->running transition as for others r?padenot

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: Bug 1644647 use AudioContextOperation::Resume to start realtime AudioNodeTracks r?padenot

This provides that a new track does not start processing before existing
tracks in the AudioContext, which may not be resumed until an
AudioCallbackDriver is running.

Depends on D79048

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: Bug 1644647 test that processing is consistent whether nodes are created before or after resume r?padenot

Depends on D79049

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: Bug 1644647 reject promises for Close AudioContextOperations run after forced graph shutdown r?padenot

and re-enable OnStateChanged() transition assertions.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: Bug 1644647 test AudioContext state updates with suspend() shortly after construction r?padenot

Comment 11

[Karl Tomlinson (:karlt)]

Attachment: remove waits for suspend and resume completion added to workaround bug 1198386

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39f13a31bf2e
Bug 1285290 use strong pointers for tracks in ApplyAudioContextOperation() r=padenot
https://hg.mozilla.org/integration/autoland/rev/680987aac530
explicitly initialize mFallbackDriverState r=padenot

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71426ce4b138
use the same mechanism for the first suspended->running transition as for others r=padenot
https://hg.mozilla.org/integration/autoland/rev/fdcd2a267d75
use AudioContextOperation::Resume to start realtime AudioNodeTracks r=padenot
https://hg.mozilla.org/integration/autoland/rev/0edf554a7996
test that processing is consistent whether nodes are created before or after resume r=padenot
https://hg.mozilla.org/integration/autoland/rev/ce7f146ddc9f
introduce PendingResumeOperation r=padenot
https://hg.mozilla.org/integration/autoland/rev/6316b2fbd3b9
rewrite AudioContextOperation handling using PendingResumeOperation r=padenot

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24129 for changes under testing/web-platform/tests

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de7e71fc0ea9
remove waits for suspend and resume completion added to workaround bug 1198386 r=padenot
https://hg.mozilla.org/integration/autoland/rev/cbbe8b775137
test AudioContext state updates with suspend() shortly after construction r=padenot
https://hg.mozilla.org/integration/autoland/rev/7c5fe3b1bec5
reject promises for Close AudioContextOperations run after forced graph shutdown r=padenot

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/24129
* Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/OQw1_K9fRSSgLYJA8sprLA)

Comment 18

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/39f13a31bf2e
https://hg.mozilla.org/mozilla-central/rev/680987aac530
https://hg.mozilla.org/mozilla-central/rev/71426ce4b138
https://hg.mozilla.org/mozilla-central/rev/fdcd2a267d75
https://hg.mozilla.org/mozilla-central/rev/0edf554a7996
https://hg.mozilla.org/mozilla-central/rev/ce7f146ddc9f
https://hg.mozilla.org/mozilla-central/rev/6316b2fbd3b9

Comment 19

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/de7e71fc0ea9
https://hg.mozilla.org/mozilla-central/rev/cbbe8b775137
https://hg.mozilla.org/mozilla-central/rev/7c5fe3b1bec5

Comment 20

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by jgraham