Closed Bug 1572281 Opened 5 years ago Closed 5 years ago

Doesn't switch the mic automatically when unplugging the mic

Categories

(Core :: WebRTC: Audio/Video, enhancement, P2)

Product:
Core
Component:
WebRTC: Audio/Video
Platform:
Unspecified
macOS
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox71 --- fixed

People

(Reporter: chunmin, Assigned: achronop)

References

Details

Attachments

(5 files)

Bug 1572281 - Remove audio device change notifications from video capture in Windows. r?dminor
47 bytes, text/x-phabricator-request
Details | Review
Bug 1572281 - Remove audio device change notifications from video capture in OSX. r?dminor
47 bytes, text/x-phabricator-request
Details | Review
Bug 1572281 - Remove audio device change notifications from video capture in Linux. r?dminor
47 bytes, text/x-phabricator-request
Details | Review
Bug 1572281 - Enhance CubebDeviceEnumerator to accept listeners and signal notifications when an audio device changes. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1572281 - In DeviceChangeCallback class separate the observer from the subject functionality. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review

Step to reproduce

  1. Make sure there are at least two input devices in the system
  2. Make a WebRTC call via https://appear.in/ and choose the system default input device as the mic
  3. Use another device to join the WebRTC call
  4. 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.

This problem cannot be reproduced when hitting bug 1572273.

It looks like there is a racing issue here on my macOS.

There are two sources that can notify the devices-changed event:

  1. InputObserver::OnDeviceChange() in CamerasParent, by listening AVCaptureDeviceWasDisconnectedNotification
  2. CubebDeviceEnumerator::InputAudioDeviceListChanged_s for input, CubebDeviceEnumerator::OutputAudioDeviceListChanged_s for output, by listening kAudioHardwarePropertyDevices on kAudioObjectSystemObject

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:

  1. Use only one listener to monitor the audio device change
  2. Make sure callbacks in (2) run before CubebDeviceEnumerator::EnumerateAudioDevices triggered by (1)
  3. Add a parameter bool aManualInvalidation to CubebDeviceEnumerator::EnumerateAudioDevices, if it's true then querying devices from cubeb
  4. Always querying devices from cubeb

Devices enumeration triggered by device changed events from parent process

  1. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/media/webrtc/trunk/webrtc/modules/video_capture/objc/device_info_objc.mm#127
  2. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/media/webrtc/trunk/webrtc/modules/video_capture/video_capture.h#85
  3. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/systemservices/CamerasParent.cpp#113
  4. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/systemservices/CamerasChild.cpp#583
  5. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#2165
  6. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#1689
  7. https://searchfox.org/mozilla-central/source/dom/media/MediaManager.cpp#1324
  8. https://searchfox.org/mozilla-central/source/dom/media/webrtc/MediaEngineWebRTC.cpp#233
  9. https://searchfox.org/mozilla-central/source/dom/media/webrtc/MediaEngineWebRTC.cpp#148
  10. https://searchfox.org/mozilla-central/source/dom/media/webrtc/CubebDeviceEnumerator.cpp#75
  11. https://searchfox.org/mozilla-central/source/dom/media/webrtc/CubebDeviceEnumerator.cpp#177

Stop media sources for the missing devices

  1. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#2206
  2. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#4562
  3. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#4211
  4. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaManager.cpp#1052
  5. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/webrtc/MediaEngineWebRTCAudio.cpp#582
  6. https://searchfox.org/mozilla-central/rev/325c1a707819602feff736f129cb36055ba6d94f/dom/media/MediaStreamGraph.cpp#2413
  7. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/MediaStreamGraph.cpp#841
  8. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/MediaStreamGraph.cpp#785

Close input device when there is an active output device

  1. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/MediaStreamGraph.cpp#813,828-830
  2. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#65,81
  3. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#125,138
  4. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#977
  5. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#109
  6. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#722
  7. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#438
  8. https://searchfox.org/mozilla-central/rev/7088fc958db5935eba24b413b1f16d6ab7bd13ea/dom/media/GraphDriver.cpp#653

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.

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: nobody → achronop
Priority: P3 → P2
Depends on: 1581806

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.

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.

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.

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.

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.

Pushed by achronopoulos@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/90286c5d1ef1 In DeviceChangeCallback class separate the observer from the subject functionality. r=pehrsons https://hg.mozilla.org/integration/autoland/rev/9f1562768496 Enhance CubebDeviceEnumerator to accept listeners and signal notifications when an audio device changes. r=pehrsons https://hg.mozilla.org/integration/autoland/rev/7bf7263db30b Remove audio device change notifications from video capture in Linux. r=dminor https://hg.mozilla.org/integration/autoland/rev/3bd2c4d6811b Remove audio device change notifications from video capture in OSX. r=dminor https://hg.mozilla.org/integration/autoland/rev/a5c23245837e Remove audio device change notifications from video capture in Windows. r=dminor
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: