Refactor enumerateDevices to allow dseparate CaptureDeviceManagers for audio and video.
rdar://problem/31575382
Created attachment 306921 [details] Preliminary Patch.
Comment on attachment 306921 [details] Preliminary Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=306921&action=review > Source/WebCore/platform/mediastream/CaptureDeviceManager.cpp:50 > for (auto captureDevice : captureDevices()) { Nit: auto& > Source/WebCore/platform/mediastream/CaptureDeviceManager.cpp:63 > + for (auto captureDevice : captureDevices()) { Ditto. > Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:131 > + CaptureDevice existingCaptureDevice; Nit: this should be declare just above where it is used to avoid initializing it if !hasMatchingType. > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDevice.cpp:42 > + Nit: extra blank line. > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:80 > + bool haveDeviceChanges = false; Nit: you can declare this just above the loop where it is used. > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:109 > + for (auto& device : m_coreAudioCaptureDevices) { > + if (device->deviceID() == deviceID) { > + found = true; > + break; > + } > + } This will be more succinct if you use std::any_of. > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:139 > + Nit: extra blank. > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:89 > This should ASSERT(m_captureDeviceID)
Comment on attachment 306921 [details] Preliminary Patch. View in context: https://bugs.webkit.org/attachment.cgi?id=306921&action=review >> Source/WebCore/platform/mediastream/CaptureDeviceManager.cpp:50 >> for (auto captureDevice : captureDevices()) { > > Nit: auto& done >> Source/WebCore/platform/mediastream/CaptureDeviceManager.cpp:63 >> + for (auto captureDevice : captureDevices()) { > > Ditto. done >> Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:131 >> + CaptureDevice existingCaptureDevice; > > Nit: this should be declare just above where it is used to avoid initializing it if !hasMatchingType. done. >> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDevice.cpp:42 >> + > > Nit: extra blank line. Done >> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:80 >> + bool haveDeviceChanges = false; > > Nit: you can declare this just above the loop where it is used. Done. >> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:109 >> + } > > This will be more succinct if you use std::any_of. Done. >> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureDeviceManager.cpp:139 >> + > > Nit: extra blank. done. >> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:89 >> > > This should ASSERT(m_captureDeviceID) Done.
Created attachment 307017 [details] Patch
Created attachment 307040 [details] Patch
Comment on attachment 307040 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307040&action=review > Source/WebCore/ChangeLog:3 > + Refactor enumerateDevices to allow dseparate CaptureDeviceManagers for audio and video. Nit: dseparate -> separate > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:63 > +#if PLATFORM(MAC) > + RealtimeMediaSourceCenter::singleton().setAudioCaptureDeviceManager(CoreAudioCaptureDeviceManager::singleton()); > +#else > + RealtimeMediaSourceCenter::singleton().setAudioCaptureDeviceManager(AVCaptureDeviceManager::singleton()); > +#endif Please add a FIXME with a bug number about changing this for iOS. > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:97 > +#if PLATFORM(MAC) > + m_audioCaptureDeviceManager = &CoreAudioCaptureDeviceManager::singleton(); > +#else > + m_audioCaptureDeviceManager = &AVCaptureDeviceManager::singleton(); > +#endif Ditto.
Comment on attachment 307040 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307040&action=review >> Source/WebCore/ChangeLog:3 >> + Refactor enumerateDevices to allow dseparate CaptureDeviceManagers for audio and video. > > Nit: dseparate -> separate done. >> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:63 >> +#endif > > Please add a FIXME with a bug number about changing this for iOS. Done. >> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:97 >> +#endif > > Ditto. Done.
Created attachment 307155 [details] Patch for landing.
Comment on attachment 307155 [details] Patch for landing. Clearing flags on attachment: 307155 Committed r215418: <http://trac.webkit.org/changeset/215418>