Closed Bug 1651049 Opened 4 years ago Closed 4 years ago

Stream becomes unmuted if sink id set before the stream

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla80
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox78 --- unaffected
firefox79 --- fixed
firefox80 --- fixed

People

(Reporter: achronop, Assigned: achronop)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

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.

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.

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.

Severity: N/A → S4

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

Pushed by achronopoulos@mozilla.com: 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
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

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.

Flags: needinfo?(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:
Flags: needinfo?(achronop)
Attachment #9161873 - Flags: approval-mozilla-beta?
Attachment #9162417 - Flags: approval-mozilla-beta?

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.

Attachment #9161873 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9162417 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: