1801190 - Resume MediaTrackGraph audio when device is replugged after setSinkId()

Status: RESOLVED FIXED

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

Description

[Karl Tomlinson (:karlt)]

https://w3c.github.io/mediacapture-output/#algorithms-new-sink-available

New audio devices may become available to the user agent, or an audio device (identified by a media element's sinkId attribute) that had previously become unavailable may become available again, for example, if it is unplugged and later plugged back in.

In this scenario, the user agent must run the following steps:

1. Let sinkId be the identifier for the newly available device.
2. For each media element whose sinkId attribute is equal to sinkId:
3. If the media element's paused attribute is false, start rendering this object's audio out of the device represented by the sinkId attribute.

STR:

1. Set pref media.setsinkid.enabled true (and media.cubeb.sandbox_v2 false to workaround bug 1801049).
2. Load https://bugzilla.mozilla.org/attachment.cgi?id=9303828
3. Click "Go!"
4. Click "Allow". (Need microphone connected.)
5. Click "Default system output..."
6. Select removable speaker device.
7. Click "Allow".
8. Remove and re-plug removable speaker device.
9. Tap microphone from 4.

Expect: audio in speaker from 6.
Actual: no audio.

Comment 1

[Karl Tomlinson (:karlt)]

This trace is not of the STR in comment 0, but indicates that there is an extra cubeb_stream_stop() on destroy in the audio ipc server:
https://pernos.co/debug/m2NrySYapwiHSN-w0EuxNA/index.html#f{m[BkB4,COc_,t[kw,HQce_,f{e[BkB4,CFA_,s{af7ouQAAA,bAYw,uLrCcUg,oLrDtvA___,v[{wiLg,v[{f'list',q'stdouterr',p{_,xAYag_,{f'container',q'stack',p{_,xAYag_,{f'notebook',q'notebook',p.,xAYag___,{w/eg,v[{f'source',q'source',p{_,xAh/8_,{f'list',q'execution',p{'symbol''mozilla:3A%3AAudioStream%3A%3AStateCallback','print'':24time'_,xAYag_,{f'list',q'callees',p{_,x+mM_,{f'list',q'execution',p{'symbol''audioipc2_server:3A%3Aserver%3A%3Astate_cb_c'_,xAYag_,{f'list',q'execution',p{'symbol''cubeb_stream_stop','print'':24tid'_,x0Tk_,{f'container',q'current-tasks',p{_,xAjwG____/

This generates an extra CUBEB_STATE_STOPPED, which would reset the Pending mAudioStreamState before (cubeb_stream_destroy()) it can switch to Running.

AudioCallbackDriver is interested in state changes from only the new cubeb_stream, not from the previous cubeb_stream that has passed through CUBEB_STATE_ERROR.

Comment 2

[Karl Tomlinson (:karlt)]

The extra cubeb_stream_stop() was added to avoid UaF from callbacks after cubeb_stream_destroy(). This notifies CUBEB_STATE_STOPPED when done.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: Bug 1801190 do not reset mAudioStreamState to None on state changes from an old cubeb_stream r?pehrsons

Such resets were preventing transitions to Running for new streams and allowing
MaybeStartAudioStream() to start another stream.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: Bug 1801190 merge mStarted into mAudioStreamState r?pehrsons

Depends on D173920

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: Bug 1801190 reset mAudioStreamState after cubeb_stream_stop() instead of on CUBEB_STATE_STOPPED r?padenot

Depends on D173921

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1801190 assert on mAudioStreamState in DataCallback() r?pehrsons

Depends on D173922

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: Bug 1801190 remove racy mAudioStream warning r?pehrsons

mAudioStream may be replaced on the CubebOperation thread while the fallback
driver hands over the graph to a ThreadedDriver or main thread that may create
a SHUTDOWN AsyncCubebTask.

The new assertion matches one in AudioCallbackDriver::Init() but is tested
earlier.

Depends on D173923

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: Bug 1801190 clarify that MockCubebStream needs the same memory layout as cubeb_stream r?padenot

to save problems if/when further members are added.

cubeb_stream_user_ptr() is not used/tested right now, but this change will
make that behave as expected.

Depends on D173924

Comment 9

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/jobs?repo=try&revision=da938aac37c4831c988e1d15891d87483159af9e

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f81e1b1615b3
do not reset mAudioStreamState to None on state changes from an old cubeb_stream r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/2eea0baf77ab
merge mStarted into mAudioStreamState r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/16ac83dc9ac5
reset mAudioStreamState after cubeb_stream_stop() instead of on CUBEB_STATE_STOPPED r=padenot
https://hg.mozilla.org/integration/autoland/rev/2fafc3764c75
remove racy mAudioStream warning r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/3894874ab98c
clarify that MockCubebStream needs the same memory layout as cubeb_stream r=padenot

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/f81e1b1615b3
https://hg.mozilla.org/mozilla-central/rev/2eea0baf77ab
https://hg.mozilla.org/mozilla-central/rev/16ac83dc9ac5
https://hg.mozilla.org/mozilla-central/rev/2fafc3764c75
https://hg.mozilla.org/mozilla-central/rev/3894874ab98c