Closed
Bug 1392930
Opened 7 years ago
Closed 7 years ago
Audio output device cannot be changed when using WebRTC
Categories
(Core :: WebRTC: Audio/Video, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla57
People
(Reporter: chunmin, Assigned: chunmin)
References
Details
Attachments
(4 files)
14.58 KB,
patch
|
Details | Diff | Splinter Review | |
59 bytes,
text/x-review-board-request
|
jwwang
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
jwwang
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
padenot
:
review+
gchang
:
approval-mozilla-beta+
|
Details |
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•7 years 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
Comment 2•7 years ago
|
||
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
Comment 3•7 years ago
|
||
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)
Assignee | ||
Comment 5•7 years 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•7 years ago
|
||
Assignee: nobody → cchang
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•7 years 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•7 years 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•7 years 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•7 years 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•7 years 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•7 years 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) |
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 19•7 years 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•7 years 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•7 years 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
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 23•7 years ago
|
||
will the modification merged into firefox 56?
Assignee | ||
Comment 26•7 years 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?
Comment 27•7 years ago
|
||
(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•7 years ago
|
status-firefox56:
--- → affected
Comment 28•7 years ago
|
||
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•7 years ago
|
||
bugherder uplift |
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
Updated•7 years ago
|
Flags: qe-verify+
Comment 30•7 years 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•7 years 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
Comment 32•7 years ago
|
||
xpeng, Thanks for your comments. Can you help use mozregression[1] to the regression? [1]http://mozilla.github.io/mozregression/
Flags: needinfo?(xpeng)
Comment 33•7 years ago
|
||
(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•7 years 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)
Comment 35•7 years ago
|
||
(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/.
Comment 36•7 years ago
|
||
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•7 years 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•7 years 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•7 years 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?
Comment 40•7 years ago
|
||
(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)
Comment 41•7 years ago
|
||
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
Comment 42•7 years ago
|
||
Clearing my ni? since Oana already covered the verification.
Flags: needinfo?(cornel.ionce)
Comment 44•7 years 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.
You need to log in
before you can comment on or make changes to this bug.
Description
•