Doesn't switch the mic automatically when unplugging the mic
Categories
(Core :: WebRTC: Audio/Video, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: chunmin, Assigned: achronop)
References
Details
Attachments
(5 files)
Step to reproduce
- Make sure there are at least two input devices in the system
- Make a WebRTC call via https://appear.in/ and choose the system default input device as the mic
- Use another device to join the WebRTC call
- Unplug the mic
Expect
The mic is switched to another input device
Actual
No mic is active, and there is a warning message:
Looks like you unplugged your microphone.
To allow others to hear you, plugin again or select a different input source.
Note
- If the mic choose in the call is a non-default device, then the mic will be switched to the default input device automatically
- In Windows, the mic will be switched automatically no matter the mic is default or not.
Reporter | ||
Comment 1•5 years ago
|
||
This problem cannot be reproduced when hitting bug 1572273.
Reporter | ||
Comment 2•5 years ago
•
|
||
It looks like there is a racing issue here on my macOS.
There are two sources that can notify the devices-changed event:
InputObserver::OnDeviceChange()
inCamerasParent
, by listeningAVCaptureDeviceWasDisconnectedNotification
CubebDeviceEnumerator::InputAudioDeviceListChanged_s
for input,CubebDeviceEnumerator::OutputAudioDeviceListChanged_s
for output, by listeningkAudioHardwarePropertyDevices
onkAudioObjectSystemObject
The listeners in (2) are registered as the callbacks for devices-changed events in the content process directly. When the listener is fired, it will clear the cached input or output devices in CubebDeviceEnumerator
(specifically, mInputDevices
and mOutputDevices
).
On the other hand, the notification by (1) will end up getting to MediaManager::OnDeviceChange
via an IPC channel from CamerasParent
(parent process) to CamerasChild
(content process). Then MediaManager::OnDeviceChange
will create a task to check if there are missing devices (code path below). To check if there are missing devices, MediaManager
will get the current devices by CubebDeviceEnumerator
and then compare the current devices with the its cached devices (mDeviceIDs
specifically). The missing device is the device that is in the cached device list but there is not the current device list. In the end, the media sources for the missing devices will be stopped. Therefore, it looks like we intend to close the media source if the device tied to the source is unplugged. Thus, the microphone shouldn't be switched automatically.
However, the interesting thing is that the mic can be switched automatically when the device is not the default input device in the system.
When the mic used in WebRTC is system default, the listeners in (2) are always run before the CubebDeviceEnumerator::EnumerateAudioDevices
triggered by (1). Therefore, the cached devices in the CubebDeviceEnumerator
will be cleared before MediaManager
asks for the current devices. If there is no cached device, the CubebDeviceEnumerator
will get the devices from cubeb. Currently, if the mic is closed properly, MediaStreamGraph::mInputDeviceID
will be set to nullptr
and the AudioCallbackDriver::mInputChannelCount
will become 0
, so the AudioCallbackDriver
for next iteration will be a output-only stream if there is an output device.
In contrast, when the mic used in WebRTC is not system default, the listeners in (2) are always run after the CubebDeviceEnumerator::EnumerateAudioDevices
triggered by (1). That is, at the time CubebDeviceEnumerator::EnumerateAudioDevices
is called, CubebDeviceEnumerator
will return its cached devices directly without querying from cubeb. As a result, the media source tied to the unplugged mic won't be stopped. Since cubeb can reinitialize stream silently, the WebRTC works fine.
I am not sure if we intend to switch the mic automatically when the active mic is unplugged. It looks like we try stopping the media source stream tied to the missing mic. If we need to make sure the media stream for the missing device will be close, maybe we could try:
- Use only one listener to monitor the audio device change
- Make sure callbacks in (2) run before
CubebDeviceEnumerator::EnumerateAudioDevices
triggered by (1) - Add a parameter
bool aManualInvalidation
toCubebDeviceEnumerator::EnumerateAudioDevices
, if it's true then querying devices from cubeb - Always querying devices from cubeb
Devices enumeration triggered by device changed events from parent process
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/media/webrtc/trunk/webrtc/modules/video_capture/objc/device_info_objc.mm#127
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/media/webrtc/trunk/webrtc/modules/video_capture/video_capture.h#85
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/systemservices/CamerasParent.cpp#113
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/systemservices/CamerasChild.cpp#583
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#2165
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#1689
- https://searchfox.org/mozilla-central/source/dom/media/MediaManager.cpp#1324
- https://searchfox.org/mozilla-central/source/dom/media/webrtc/MediaEngineWebRTC.cpp#233
- https://searchfox.org/mozilla-central/source/dom/media/webrtc/MediaEngineWebRTC.cpp#148
- https://searchfox.org/mozilla-central/source/dom/media/webrtc/CubebDeviceEnumerator.cpp#75
- https://searchfox.org/mozilla-central/source/dom/media/webrtc/CubebDeviceEnumerator.cpp#177
Stop media sources for the missing devices
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#2206
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#4562
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#4211
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#1052
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/webrtc/MediaEngineWebRTCAudio.cpp#582
- https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaStreamGraph.cpp#2413
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/MediaStreamGraph.cpp#841
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/MediaStreamGraph.cpp#785
Close input device when there is an active output device
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/MediaStreamGraph.cpp#813,828-830
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#65,81
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#125,138
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#977
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#109
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#722
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#438
- https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#653
Comment 3•5 years ago
|
||
Regardless of the disconnect-policy we decide on for audio devices, I think the "devicechange" callback from CamerasParent should regard camera devices only. I have a hunch we implemented audio devicechange in CamerasParent before we had the support in cubeb, so the feature could be shipped, and then somehow this was forgotten.
I don't have cycles to look into this right now, but I'm happy to help should there be any questions on cameras or MediaManager.
Assignee | ||
Comment 4•5 years ago
|
||
I agree with Andreas that the audio device changes should be handled from the cubeb notification and leave the devicechange callback from CamerasParent to handle the camera devices only. I have also mentioned the same thing in Bug 1522488#c10 which is a different side effect of the same problem. At that time that solution was being blocked by https://github.com/djg/audioipc-2/issues/50 but now this issue has been solved and we can go on with the proper fix. My plan is to create a listener in CubebDeviceEnumerator
, any component which is interested in those events will be able to register itself on that listener and get the notifications. That way we can control better when the list of devices is invalidated. I could start looking at it.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 5•5 years ago
|
||
Assignee | ||
Comment 6•5 years ago
|
||
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
Video capture used to provide device change notifications for audio and video devices. From now on, CubebDeviceEnumerator will provide audio device change notifications thus video capture is updated to notify only changes of the video device. This is the windows part.
Assignee | ||
Comment 10•5 years ago
|
||
Video capture used to provide device change notifications for audio and video devices. From now on, CubebDeviceEnumerator will provide audio device change notifications thus video capture is updated to notify only changes of the video device. This is the OSX part.
Assignee | ||
Comment 11•5 years ago
|
||
Video capture used to provide device change notifications for audio and video devices. From now on, CubebDeviceEnumerator will provide audio device change notifications thus video capture is updated to notify only changes of the video device. This is the Linux part.
Assignee | ||
Comment 12•5 years ago
|
||
CubebDeviceEnumerator already knows when an audio device changes. It is enhanced to allow listeners/observers registration and to create notifications when that happens. Also, it is hooked to the existing notification path.
On a minor note, it has been revisited the way the enumerator is touched in MediaEngineWebRTC class.
Assignee | ||
Comment 13•5 years ago
|
||
DeviceChangeCallback class implements the observer pattern. However, the role of the observer and the subject is integrated into the same class which makes use of virtual methods to allow a separation of the roles. This makes code reading difficult. Also, it does not allow from a class to inherit only the observer role or the subject role. This patch breaks the DeviceChangeCallback class into two classes according to the observer or subject functionality.
Comment 14•5 years ago
|
||
Comment 15•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/90286c5d1ef1
https://hg.mozilla.org/mozilla-central/rev/9f1562768496
https://hg.mozilla.org/mozilla-central/rev/7bf7263db30b
https://hg.mozilla.org/mozilla-central/rev/3bd2c4d6811b
https://hg.mozilla.org/mozilla-central/rev/a5c23245837e
Updated•5 years ago
|
Description
•