Audio output device cannot be changed when using WebRTC

VERIFIED FIXED in Firefox 56

Status

()

Core
WebRTC: Audio/Video
P1
normal
Rank:
15
VERIFIED FIXED
3 months ago
2 months ago

People

(Reporter: chunmin, Assigned: chunmin)

Tracking

unspecified
mozilla57
x86_64
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 verified, firefox57 verified, firefox58 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Assignee)

Description

3 months ago
Firefox version: 56.0b3 (Firefox Developer Edition)

Steps to reproduce:
1. Open https://webrtc.github.io/samples/src/content/getusermedia/audio/
2. Check the audio works in current default microphone and speaker
3. Switch the default audio output device

Expected result:
Audio should come out from the new default output device

Actual result:
Audio comes out from the original default output device
(Assignee)

Comment 1

3 months ago
I believe the root cause is same as 1361336. We should reuse the AudioNotificationReceiver/Sender in GraphDriver or MediaStreamGraph to pass the default-device-changed event.
Depends on: 1361336
I thought the switch will happen automatically inside cubeb. cubeb on OSX does that. Is the behavior different on Windows?
Rank: 15
Flags: needinfo?(padenot)
Priority: -- → P1
Yes, sandboxing broke the automatic switch. Our sandbox level is higher on Windows and the event that tells us that the default device changed is never received.
Flags: needinfo?(padenot)
Can you try to repro with sandbox disabled?
Flags: needinfo?(cchang)
(Assignee)

Comment 5

3 months ago
(In reply to Alex Chronopoulos [:achronop] from comment #4)
> Can you try to repro with sandbox disabled?

If I change the security.sandbox.content.level to 0 or disable e10s, then the device changing works well as usual. It's same in bug 1361336 comment 5, bug 1361336 comment 9, bug 1361336 comment 10

You can try opening a stream on https://janus.conf.meetecho.com/streamingtest.html and switch the output device to test it.
Flags: needinfo?(cchang)
(Assignee)

Comment 6

3 months ago
Created attachment 8901747 [details] [diff] [review]
[WIP] Using  AudioNotificationReceiver in GraphDriver to get device-changed notification
Assignee: nobody → cchang
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

3 months ago
mozreview-review
Comment on attachment 8902172 [details]
Bug 1392930 - part 3: Make AudioCallbackDriver inherit from DeviceChangeListener;

https://reviewboard.mozilla.org/r/173632/#review178950

Thanks!
Attachment #8902172 - Flags: review?(padenot) → review+

Comment 11

3 months ago
mozreview-review
Comment on attachment 8902170 [details]
Bug 1392930 - part 1: Replace AudioStream by DeviceChangeListener in AudioNotificationReceiver;

https://reviewboard.mozilla.org/r/173628/#review178956

::: dom/media/AudioNotificationReceiver.h:70
(Diff revision 1)
>  namespace audio {
>  
> +// The base class that provides a ResetDefaultDevice interface that
> +// will be called in AudioNotificationReceiver::NotifyDefaultDeviceChanged
> +// when it receives device-changed notification from the chrome process.
> +class StreamListener

StreamListener is a too general name to convey what it actually does. Please call it 'DeviceChangeListener'.
Attachment #8902170 - Flags: review?(jwwang) → review+

Comment 12

3 months ago
mozreview-review
Comment on attachment 8902171 [details]
Bug 1392930 - part 2: Make AudioStream inherit from DeviceChangeListener;

https://reviewboard.mozilla.org/r/173630/#review178958

::: dom/media/AudioStream.h:20
(Diff revision 1)
>  #include "mozilla/TimeStamp.h"
>  #include "mozilla/UniquePtr.h"
>  #include "CubebUtils.h"
>  #include "soundtouch/SoundTouchFactory.h"
>  
> +#if defined(XP_WIN)

Why do we need this ifdef?
(Assignee)

Comment 13

3 months ago
mozreview-review-reply
Comment on attachment 8902171 [details]
Bug 1392930 - part 2: Make AudioStream inherit from DeviceChangeListener;

https://reviewboard.mozilla.org/r/173630/#review178958

> Why do we need this ifdef?

AudioNotificationReceiver is only used on Windows platform since the sandbox only blocks the device-changed notification in Windows on content side for now. The device-changing will be handled in Cubeb on other platforms.
https://searchfox.org/mozilla-central/rev/cd82cacec2cf734768827ff85ba2dba90a534c5e/dom/media/moz.build#269
(Assignee)

Comment 14

3 months ago
mozreview-review-reply
Comment on attachment 8902170 [details]
Bug 1392930 - part 1: Replace AudioStream by DeviceChangeListener in AudioNotificationReceiver;

https://reviewboard.mozilla.org/r/173628/#review178956

> StreamListener is a too general name to convey what it actually does. Please call it 'DeviceChangeListener'.

Sure.

Comment 15

3 months ago
mozreview-review
Comment on attachment 8902171 [details]
Bug 1392930 - part 2: Make AudioStream inherit from DeviceChangeListener;

https://reviewboard.mozilla.org/r/173630/#review178986
Attachment #8902171 - Flags: review?(jwwang) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Updated

3 months ago
Blocks: 1384805
(Assignee)

Updated

3 months ago
Keywords: checkin-needed

Comment 19

3 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/131c054cb9f8
part 1: Replace AudioStream by DeviceChangeListener in AudioNotificationReceiver; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/ef53c534fcc2
part 2: Make AudioStream inherit from DeviceChangeListener; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/46e178724afe
part 3: Make AudioCallbackDriver inherit from DeviceChangeListener; r=padenot
Keywords: checkin-needed

Comment 20

3 months ago
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/674670bfea13
part 1: Replace AudioStream by DeviceChangeListener in AudioNotificationReceiver; r=jwwang
https://hg.mozilla.org/mozilla-central/rev/02bf9496d1c8
part 2: Make AudioStream inherit from DeviceChangeListener; r=jwwang
https://hg.mozilla.org/mozilla-central/rev/89d020630629
part 3: Make AudioCallbackDriver inherit from DeviceChangeListener; r=padenot

Comment 21

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/674670bfea13
https://hg.mozilla.org/mozilla-central/rev/02bf9496d1c8
https://hg.mozilla.org/mozilla-central/rev/89d020630629
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Updated

3 months ago
Duplicate of this bug: 1384805

Comment 23

2 months ago
will the modification merged into firefox 56?

Updated

2 months ago
Duplicate of this bug: 1346676
Chun-Min, can you request approval for beta 56 ?
Flags: needinfo?(cchang)
(Assignee)

Comment 26

2 months ago
Comment on attachment 8902172 [details]
Bug 1392930 - part 3: Make AudioCallbackDriver inherit from DeviceChangeListener;

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: When running WebRTC on Firefox Windows, the audio devices can not be changed.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes. I tested it manually and some users have confirmed it's fixed on FF57 (bug 1346676 comment 13)
[Needs manual test from QE? If yes, steps to reproduce]: Yes. STR is in comment 0.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: No.
[Why is the change risky/not risky?]: The patch is simple and users have confirmed it's fixed.
[String changes made/needed]: None.
Flags: needinfo?(cchang)
Attachment #8902172 - Flags: approval-mozilla-beta?
(In reply to Chun-Min Chang[:chunmin] from comment #26)
> [User impact if declined]: When running WebRTC on Firefox Windows, the audio
> devices can not be changed.

I'd like to elaborate on this: this will break Web Audio API or WebRTC (media will freeze) in the event of an audio device change (physically plug/unplug, device error, etc.). This regression is caused by making the sandbox stricter on Windows, and we ca't really ship with a regression like that.

A number of people have verified the fix, there has been a number of dupes already.

Updated

2 months ago
status-firefox56: --- → affected
Comment on attachment 8902172 [details]
Bug 1392930 - part 3: Make AudioCallbackDriver inherit from DeviceChangeListener;

Fix a WebRTC issue and was verified. Beta56+.
Attachment #8902172 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 29

2 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/93101738c6f9
https://hg.mozilla.org/releases/mozilla-beta/rev/4302ede51ff6
https://hg.mozilla.org/releases/mozilla-beta/rev/a566b0235ef9
status-firefox56: affected → fixed
Flags: qe-verify+

Comment 30

2 months ago
the issue reproduced in the firefox nightly 57.0a1(2017-09-11).    

it works fine in nightly version 57.0a1(2017-09-07)
you can reproduce the issue using the demo: https://webrtc.github.io/samples/src/content/devices/input-output/
select the micphone and camera , and then plugout the micphone in use, the video frame will be paused.

Comment 31

2 months ago
firefox nightly 57.0a1(2017-09-07)  :  Pass
firefox nightly 57.0a1(2017-09-11) : fail
firefox nightly 57.0a1(2017-09-12)  :  Pass
firefox nightly 57.0a1(2017-09-13) : fail
xpeng,
Thanks for your comments. 
Can you help use mozregression[1] to the regression?

[1]http://mozilla.github.io/mozregression/
Flags: needinfo?(xpeng)
(In reply to Blake Wu [:bwu][:blakewu] from comment #32)
> xpeng,
> Thanks for your comments. 
> Can you help use mozregression[1] to the regression?
to find the regression :-)

Comment 34

2 months ago
(In reply to Blake Wu [:bwu][:blakewu] from comment #33)
> (In reply to Blake Wu [:bwu][:blakewu] from comment #32)
> > xpeng,
> > Thanks for your comments. 
> > Can you help use mozregression[1] to the regression?
> to find the regression :-)

1. I hava download the mozregression-gui and what should i do? 
2. the issue was reproduced in the firefox 56.0b12 beta .
Flags: needinfo?(xpeng)
(In reply to xpeng from comment #34)
> (In reply to Blake Wu [:bwu][:blakewu] from comment #33)
> > (In reply to Blake Wu [:bwu][:blakewu] from comment #32)
> > > xpeng,
> > > Thanks for your comments. 
> > > Can you help use mozregression[1] to the regression?
> > to find the regression :-)
> 
> 1. I hava download the mozregression-gui and what should i do? 
Just follow the instructions in http://mozilla.github.io/mozregression/.
Cornel,
Per comment 31 and comment 34, may we have your help to double check if this is really fixed or not?
Flags: needinfo?(cornel.ionce)

Comment 37

2 months ago
I managed to reproduce the bug on Windows 10 x32 and Windows 10 x64 using an old version of Nightly (2017-08-23).
I retested everything on the same platforms using Beta 56.0b12 and latest Nightly, but I couldn't reproduce the bug anymore. As soon as I switched the default audio output device, the sound was played from the new default device.

Comment 38

2 months ago
the issue what i reproduced was just like the bug : https://bugzilla.mozilla.org/show_bug.cgi?id=1346676  which was marked as a duplicate issue. 
    
Step 1: use the demo to select a usb device (micphone) and a camera device , the camera display normally 
https://webrtc.github.io/samples/src/content/devices/input-output/
Step 2: unplug the usb device(micphone)

Result : the camera is paused, the image do not change anymore.

Comment 39

2 months ago
I retested everything on Latest Nightly using Windows 10 x32, Windows 10 x64, Mac OS x10.11 and Ubuntu 16.04 x 64 and the bug is partially not reproducing. When you pull out the microphone the video freezes for a short moment then it continues to play. The exception being Ubuntu, where the transaction is really smooth.

On the other hand, if you unplug the webcam, the sound is not working anymore. (I checked so that the microphone used wasn't the one from the webcam.) This issue appears on all platforms and is not reproducing on Chrome. Is that something that can be related to this bug or is it a different issue?
(In reply to Oana Botisan from comment #39)
> I retested everything on Latest Nightly using Windows 10 x32, Windows 10
> x64, Mac OS x10.11 and Ubuntu 16.04 x 64 and the bug is partially not
> reproducing. When you pull out the microphone the video freezes for a short
> moment then it continues to play. The exception being Ubuntu, where the
> transaction is really smooth.

Thanks. The short freeze is expected for now, unfortunately.

> On the other hand, if you unplug the webcam, the sound is not working
> anymore. (I checked so that the microphone used wasn't the one from the
> webcam.) This issue appears on all platforms and is not reproducing on
> Chrome. Is that something that can be related to this bug or is it a
> different issue?

This is a different one. jib, can you investigate?
Flags: needinfo?(jib)
About the second problem I see in the debugger that we get a close audio message from main thread in the middle of the iteration:

  * frame #0: 0x0000000105aa89d4 XUL`mozilla::GraphDriver::SwitchAtNextIteration(this=0x00000001334d9980, aNextDriver=0x00000001274512f0) + 20 at GraphDriver.cpp:68
    frame #1: 0x0000000105c5402c XUL`mozilla::MediaStreamGraphImpl::CloseAudioInputImpl(this=0x000000012783b800, aListener=0x000000013cd05b80) + 604 at MediaStreamGraph.cpp:1071
    frame #2: 0x0000000105c76d9d XUL`mozilla::MediaStreamGraphImpl::CloseAudioInput(this=0x00000001245244e0)::Message::Run() + 45 at MediaStreamGraph.cpp:1100
    frame #3: 0x0000000105c5578a XUL`mozilla::MediaStreamGraphImpl::RunMessagesInQueue(this=0x000000012783b800) + 170 at MediaStreamGraph.cpp:1281
    frame #4: 0x0000000105c564f0 XUL`mozilla::MediaStreamGraphImpl::OneIteration(this=0x000000012783b800, aStateEnd=381056) + 48 at MediaStreamGraph.cpp:1451
    frame #5: 0x0000000105aad60b XUL`mozilla::AudioCallbackDriver::DataCallback(this=0x00000001334d9980, aInputBuffer=0x000000013112a000, aOutputBuffer=0x000000012bda7000, aFrames=512) + 1099 at GraphDriver.cpp:1028
    frame #6: 0x0000000105aac7b4 XUL`mozilla::AudioCallbackDriver::DataCallback_s(aStream=0x0000000132061400, aUser=0x00000001334d9980, aInputBuffer=0x000000013112a000, aOutputBuffer=0x000000012bda7000, aFrames=512) + 68 at GraphDriver.cpp:897
    frame #7: 0x0000000107ca968e XUL`passthrough_resampler<float>::fill(this=0x0000000125f5f640, input_buffer=0x0000000131b67000, input_frames_count=0x0000700003fa97e8, output_buffer=0x000000012bda7000, output_frames=512) + 462 at cubeb_resampler.cpp:69
    frame #8: 0x0000000107ca5174 XUL`::cubeb_resampler_fill(resampler=0x0000000125f5f640, input_buffer=0x0000000131b67000, input_frames_count=0x0000700003fa97e8, output_buffer=0x000000012bda7000, output_frames_needed=512) + 84 at cubeb_resampler.cpp:300
    frame #9: 0x0000000107c9c891 XUL`audiounit_output_callback(user_ptr=0x0000000132061400, (null)=0x0000700003fa9c78, tstamp=0x0000700003fa9c80, bus=0, output_frames=512, outBufferList=0x0000000127127e60) + 2385 at cubeb_audiounit.cpp:492
Clearing my ni? since Oana already covered the verification.
Flags: needinfo?(cornel.ionce)
I filed bug 1402377 about 40.
Flags: needinfo?(jib)

Comment 44

2 months ago
I retested everything just to be sure on the latest Nightly using Windows 10x64, Ubuntu 16.04x64 and Mac OS 10.11 but the bug is not reproducing anymore. 

Based on this result and the ones from comment 39 and comment 43, I think that this bug is fixed.
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified
status-firefox57: fixed → verified
status-firefox58: --- → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.