1651049 - Stream becomes unmuted if sink id set before the stream

Status: RESOLVED FIXED

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

Description

[Alex Chronopoulos [:achronop]]

STR:
On an empty HTMLMediaElement.

1. Mute the element.
2. Change the sinkid to a device (any device)
3. Create a MediaStream with audio data. Either through WebAudio or through and gUM.
4. Attach the stream to the element (element.srcObject = stream).

Error:
The audio data of the stream is audible.

Expected result:
The stream will be rendered but it will be muted (not audible). Unmuting the stream will make it audible.

Comment 1

[Alex Chronopoulos [:achronop]]

The problem is that AudioOutput is set twice for the same key&track. First, is set when the element setups the MediaStream for Playback and a second time in MediaStreamRenderer when the MediaStream's tracks are added.

The sink change follows the logic of AudioOuptut so it should be included in the MediaStreamRenderer. Then renderer has to handle correctly the rendering and non-rendering case.

A side problem is that the AudioOutput logic allows for the same key for the same track to be registered more than once. This not possible with that patch. However, I will revisit the details of the AudioOuptut in a following bug.

Comment 2

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1651049 - Move the sink change logic in MediaStreamRenderer. r?padenot

The MediaStreamRenderer handles the AudioOutput of an HTMLMediaElement. it register/unregister the AudioOutput according to the rendering status. The sink-change follows the logic of AudioOutput thus it has been moved in it.

The previous way created an error when the element had been muted and the sink had been changed before the source MediaStream was attached to the element. The error occured because it was possible, the same entry to be registered more than once in the AudioOutput's list, which resulted in the track to be unmuted when it should not.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1493990

Comment 4

[Alex Chronopoulos [:achronop]]

Added a mochitest: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2c7eef610623958e388e2869f3a3994e7c5e407

Comment 5

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1651049 - Mochitest to verify the failed scenario. r?padenot

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a226a8d23ce
Move the sink change logic in MediaStreamRenderer. r=padenot
https://hg.mozilla.org/integration/autoland/rev/6468f2d9b8ae
Mochitest to verify the failed scenario. r=padenot

Comment 7

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1a226a8d23ce
https://hg.mozilla.org/mozilla-central/rev/6468f2d9b8ae

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:achronop, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 9

[Alex Chronopoulos [:achronop]]

Comment on attachment 9161873 [details]
Bug 1651049 - Move the sink change logic in MediaStreamRenderer. r?padenot

Beta/Release Uplift Approval Request

• User impact if declined: A conference scenario fails. The impact is that the media element will be unmuted when it is expected muted. The functionality that exposes that error is still preffed off in Nightly and Beta.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: See the description of the Bug.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Has been tested in Beta for some time.
• String changes made/needed:

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9161873 [details]
Bug 1651049 - Move the sink change logic in MediaStreamRenderer. r?padenot

Approved for 79.0b8. Thanks for including a test.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/06308497afcc
https://hg.mozilla.org/releases/mozilla-beta/rev/dab7fc014ffe