[MacOS] getUserMedia called multiple times should better handle camera resolution When calling getUserMedia once with a high resolution and second with a low resolution, the first stream will be captured with a low resolution and then upscaled.
<rdar://problem/51718990>
Created attachment 372801 [details] Patch
Attachment 372801 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:50: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:54: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:62: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:67: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 4 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 372808 [details] Patch
Attachment 372808 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:50: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:54: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:62: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:66: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 4 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 372809 [details] Patch
Attachment 372809 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:50: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:54: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:62: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:66: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 4 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 372813 [details] Patch
Attachment 372813 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:50: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:54: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:62: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:66: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 4 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 372813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372813&action=review > Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.cpp:63 > + auto result = source->applyConstraints(*constraints); Nit: should we special case width and height here to avoid configuring the capture device to capture at less than the maximum needed? > Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h:78 > + virtual CaptureSourceOrError createVideoCaptureSource(const CaptureDevice&, String&&, const MediaConstraints*)= 0; Nit: missing space before "=" > Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h:96 > + virtual RefPtr<RealtimeVideoCaptureSource> newSource(const CaptureDevice&, String&&) = 0; Nit: Something like "createNewSource" might be clearer.
Thanks for the review. (In reply to Eric Carlson from comment #10) > Comment on attachment 372813 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372813&action=review > > > Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.cpp:63 > > + auto result = source->applyConstraints(*constraints); > > Nit: should we special case width and height here to avoid configuring the > capture device to capture at less than the maximum needed? applyConstraints will bump the capture resolution if needs be. > > Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h:78 > > + virtual CaptureSourceOrError createVideoCaptureSource(const CaptureDevice&, String&&, const MediaConstraints*)= 0; > > Nit: missing space before "=" OK > > Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h:96 > > + virtual RefPtr<RealtimeVideoCaptureSource> newSource(const CaptureDevice&, String&&) = 0; > > Nit: Something like "createNewSource" might be clearer. Will do
Created attachment 373058 [details] Patch
Attachment 373058 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:50: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:54: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:62: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:66: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 4 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 373058 [details] Patch Attachment 373058 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12598843 New failing tests: svg/text/textpath-reference-update.html js/dom/try-catch-crash.html
Created attachment 373085 [details] Archive of layout-test-results from ews211 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews211 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 373085 [details] Archive of layout-test-results from ews211 for win-future Win error unrelated
Comment on attachment 373058 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373058&action=review > Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:129 > +#if PLATFORM(MAC) > + AVCaptureDeviceManager::singleton().addVideoCaptureSource(*this); > +#endif We should try to minimize the differences between real and mock sources as much as possible, so even though iOS can currently only have one camera active at a time it would be better to have this and VideoCaptureSourceFactoryMac::existingSource be the same on both platforms. > Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:131 > + case CaptureDevice::DeviceType::Window: { > + auto source = MockRealtimeVideoSource::create(String { device.persistentId() }, String { device.label() }, String { }); > + if (constraints) { > + auto result = source->applyConstraints(*constraints); > + if (result) > + return WTFMove(result.value().badConstraint); > + } > + return CaptureSourceOrError { RealtimeVideoSource::create(source.releaseNonNull()) }; > + } > break; Nit: the "break" isn't needed.
(In reply to Eric Carlson from comment #17) > Comment on attachment 373058 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=373058&action=review > > > Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:129 > > +#if PLATFORM(MAC) > > + AVCaptureDeviceManager::singleton().addVideoCaptureSource(*this); > > +#endif > > We should try to minimize the differences between real and mock sources as > much as possible, so even though iOS can currently only have one camera > active at a time it would be better to have this and > VideoCaptureSourceFactoryMac::existingSource be the same on both platforms. Right, it might be better for the device manager to actually mandate for iOS that there is only one active source at any given time. I'll update the patch accordingly and will try to see how to remove SimpleSourceFactory. > > > Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:131 > > + case CaptureDevice::DeviceType::Window: { > > + auto source = MockRealtimeVideoSource::create(String { device.persistentId() }, String { device.label() }, String { }); > > + if (constraints) { > > + auto result = source->applyConstraints(*constraints); > > + if (result) > > + return WTFMove(result.value().badConstraint); > > + } > > + return CaptureSourceOrError { RealtimeVideoSource::create(source.releaseNonNull()) }; > > + } > > break; > > Nit: the "break" isn't needed. OK
Created attachment 375414 [details] Patch for landing
Created attachment 375416 [details] Patch for landing
Attachment 375416 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:50: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:54: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:62: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] ERROR: Source/WebCore/platform/mediastream/CaptureDeviceManager.h:66: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 4 in 17 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 375416 [details] Patch for landing >Subversion Revision: 248104 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 455ca5f5b884c5d99683e2257ffe452463a94440..77843175b856a02f75f922fd301deae91fc10880 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,50 @@ >+2019-08-02 Youenn Fablet <youenn@apple.com> >+ >+ [MacOS] getUserMedia called multiple times should better handle camera resolution >+ https://bugs.webkit.org/show_bug.cgi?id=198839 >+ <rdar://problem/51718990>from Mel fuck i >+ >+ Reviewed by Eric Carlson. >+ >+ Store the list of camera sources in a map of the camera manager. >+ Whenever we create a new camera source, we look into this map for an existing source. >+ If a source exists, we directly create a RealtimeVideoSource from it and we apply constraints on the RealtimeVideoSource. >+ This is the same code path as clone/applyConstraints. >+ >+ This patch does some refactoring to share that code in the camera manager and camera source factory. >+ >+ Covered by existing tests and updated iOS test. >+ >+ * platform/mediastream/CaptureDeviceManager.h: >+ * platform/mediastream/RealtimeMediaSourceFactory.cpp: >+ (WebCore::VideoCaptureFactory::createVideoCaptureSource): >+ * platform/mediastream/RealtimeMediaSourceFactory.h: >+ * platform/mediastream/gstreamer/MockGStreamerVideoCaptureSource.cpp: >+ (WebCore::MockRealtimeVideoSource::create): >+ * platform/mediastream/mac/AVCaptureDeviceManager.h: >+ * platform/mediastream/mac/AVVideoCaptureSource.h: >+ * platform/mediastream/mac/AVVideoCaptureSource.mm: >+ (WebCore::AVVideoCaptureSource::create): >+ (WebCore::AVVideoCaptureSource::AVVideoCaptureSource): >+ (WebCore::AVVideoCaptureSource::~AVVideoCaptureSource): >+ * platform/mediastream/mac/MockRealtimeVideoSourceMac.mm: >+ (WebCore::MockRealtimeVideoSource::create): >+ * platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp: >+ * platform/mock/MockRealtimeMediaSourceCenter.cpp: >+ * platform/mock/MockRealtimeMediaSourceCenter.h: >+ (WebCore::MockRealtimeMediaSourceCenter::videoCaptureDeviceManager): >+ (WebCore::MockRealtimeMediaSourceCenter::audioCaptureDeviceManager): >+ * platform/mock/MockRealtimeVideoSource.cpp: >+ (WebCore::MockRealtimeVideoSource::create): >+ (WebCore::MockRealtimeVideoSource::MockRealtimeVideoSource): >+ (WebCore::MockRealtimeVideoSource::~MockRealtimeVideoSource): >+ (WebCore::MockRealtimeVideoSource::deviceType const): >+ * platform/mock/MockRealtimeVideoSource.h: >+ * testing/Internals.cpp: >+ (WebCore::Internals::resetToConsistentState): >+ Now that we have sources kept in a map, we can reuse them from one test to another. >+ We clear this state to ensure consistent init step of a test. >+ > 2019-08-01 Youenn Fablet <youenn@apple.com> > > [iOS] Directly use RealtimeMediaSourceCenter to compute the media capture state >diff --git a/Source/WebCore/platform/mediastream/CaptureDeviceManager.h b/Source/WebCore/platform/mediastream/CaptureDeviceManager.h >index 20a93973c1e3addada837ed06770803dfdbecd0e..05f8e1dbba8448db4cb248abee2fc76cebdccfa9 100644 >--- a/Source/WebCore/platform/mediastream/CaptureDeviceManager.h >+++ b/Source/WebCore/platform/mediastream/CaptureDeviceManager.h >@@ -28,7 +28,7 @@ > #if ENABLE(MEDIA_STREAM) > > #include "CaptureDevice.h" >-#include "RealtimeMediaSource.h" >+#include "RealtimeVideoCaptureSource.h" > #include <wtf/WeakPtr.h> > > namespace WebCore { >@@ -44,6 +44,33 @@ protected: > void deviceChanged(); > }; > >+class WEBCORE_EXPORT CaptureCameraManager : public CaptureDeviceManager { >+public: >+ RealtimeVideoCaptureSource* videoCaptureSource(const String& id) >+ { >+ return m_deviceSources.get(id); >+ } >+ void addVideoCaptureSource(RealtimeVideoCaptureSource& source) >+ { >+#if !ASSERT_DISABLED >+ auto* existing = m_deviceSources.get(source.persistentID()); >+ ASSERT(!existing || existing->muted() || existing->interrupted() || existing->deviceIDHashSalt() != source.deviceIDHashSalt()); >+#endif >+ m_deviceSources.set(source.persistentID(), &source); >+ } >+ void removeVideoCaptureSource(RealtimeVideoCaptureSource& source) >+ { >+ m_deviceSources.remove(source.persistentID()); >+ } >+ void clearSources() >+ { >+ m_deviceSources.clear(); >+ } >+ >+private: >+ HashMap<String, RealtimeVideoCaptureSource*> m_deviceSources; >+}; >+ > } // namespace WebCore > > #endif // ENABLE(MEDIA_STREAM) >diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.cpp b/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.cpp >index e8e55df27f4a3f6858f13b586a48e9655afe61fd..fe60bc6d85f8c854e99f3a8675583a8c917c6d6d 100644 >--- a/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.cpp >+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.cpp >@@ -32,7 +32,7 @@ > #include "CaptureDeviceManager.h" > #include "Logging.h" > #include "MediaStreamPrivate.h" >-#include "RealtimeMediaSource.h" >+#include "RealtimeVideoSource.h" > #include <wtf/SHA1.h> > > namespace WebCore { >@@ -52,6 +52,34 @@ void SingleSourceFactory::unsetActiveSource(RealtimeMediaSource& source) > m_activeSource = nullptr; > } > >+CaptureSourceOrError CameraCaptureFactory::createVideoCaptureSource(const CaptureDevice& device, String&& hashSalt, const MediaConstraints* constraints) >+{ >+ ASSERT(device.type() == CaptureDevice::DeviceType::Camera); >+ >+ auto* deviceSource = existingSource(device.persistentId()); >+ if (deviceSource && !deviceSource->muted() && !deviceSource->interrupted() && hashSalt == deviceSource->deviceIDHashSalt()) { >+ auto source = RealtimeVideoSource::create(*deviceSource); >+ if (constraints) { >+ auto result = source->applyConstraints(*constraints); >+ if (result) >+ return WTFMove(result.value().badConstraint); >+ } >+ return CaptureSourceOrError { WTFMove(source) }; >+ } >+ >+ auto newDeviceSource = createNewSource(device, WTFMove(hashSalt)); >+ if (!newDeviceSource) >+ return { }; >+ >+ if (constraints) { >+ auto result = newDeviceSource->applyConstraints(*constraints); >+ if (result) >+ return WTFMove(result.value().badConstraint); >+ } >+ >+ return CaptureSourceOrError { RealtimeVideoSource::create(newDeviceSource.releaseNonNull()) }; >+} >+ > } // namespace WebCore > > #endif // ENABLE(MEDIA_STREAM) >diff --git a/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h b/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h >index a3d52f0fcb773634cbf453ff94acea5a1a2fc562..3f76ee6e4063e49685eca253abe0ff7c24584532 100644 >--- a/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h >+++ b/Source/WebCore/platform/mediastream/RealtimeMediaSourceFactory.h >@@ -34,6 +34,7 @@ namespace WebCore { > class CaptureDevice; > class CaptureDeviceManager; > class RealtimeMediaSource; >+class RealtimeVideoCaptureSource; > > struct CaptureSourceOrError; > struct MediaConstraints; >@@ -69,12 +70,13 @@ protected: > > class VideoCaptureFactory > #if PLATFORM(IOS_FAMILY) >- : public SingleSourceFactory >+: public SingleSourceFactory > #endif > { > public: > virtual ~VideoCaptureFactory() = default; > virtual CaptureSourceOrError createVideoCaptureSource(const CaptureDevice&, String&&, const MediaConstraints*) = 0; >+ > virtual CaptureDeviceManager& videoCaptureDeviceManager() = 0; > virtual void setVideoCapturePageState(bool interrupted, bool pageMuted) > { >@@ -86,6 +88,14 @@ protected: > VideoCaptureFactory() = default; > }; > >+class WEBCORE_EXPORT CameraCaptureFactory : public VideoCaptureFactory { >+private: >+ CaptureSourceOrError createVideoCaptureSource(const CaptureDevice&, String&&, const MediaConstraints*) final; >+ >+ virtual RealtimeVideoCaptureSource* existingSource(const String&) = 0; >+ virtual RefPtr<RealtimeVideoCaptureSource> createNewSource(const CaptureDevice&, String&&) = 0; >+}; >+ > class DisplayCaptureFactory { > public: > virtual ~DisplayCaptureFactory() = default; >diff --git a/Source/WebCore/platform/mediastream/gstreamer/MockGStreamerVideoCaptureSource.cpp b/Source/WebCore/platform/mediastream/gstreamer/MockGStreamerVideoCaptureSource.cpp >index a48fcc8f6f642d66f9fcb89885badbebc2c0cf13..6d50dbad72d154ba9f31292859f5c022cea90ef1 100644 >--- a/Source/WebCore/platform/mediastream/gstreamer/MockGStreamerVideoCaptureSource.cpp >+++ b/Source/WebCore/platform/mediastream/gstreamer/MockGStreamerVideoCaptureSource.cpp >@@ -72,15 +72,9 @@ private: > } > }; > >-CaptureSourceOrError MockRealtimeVideoSource::create(String&& deviceID, >- String&& name, String&& hashSalt, const MediaConstraints* constraints) >+RefPtr<RealtimeVideoCaptureSource> MockRealtimeVideoSource::create(String&& deviceID, String&& name, String&& hashSalt) > { >- auto source = adoptRef(*new MockGStreamerVideoCaptureSource(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))); >- >- if (constraints && source->applyConstraints(*constraints)) >- return { }; >- >- return CaptureSourceOrError(WTFMove(source)); >+ return adoptRef(*new MockGStreamerVideoCaptureSource(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))); > } > > void MockGStreamerVideoCaptureSource::startProducingData() >diff --git a/Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.h b/Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.h >index 5ff4a3a3103ab2074c49f3ab7a88aee730568afd..62318e230f5d18f4fa1b8b763db2640ccf9fe195 100644 >--- a/Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.h >+++ b/Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.h >@@ -43,8 +43,9 @@ OBJC_CLASS NSString; > OBJC_CLASS WebCoreAVCaptureDeviceManagerObserver; > > namespace WebCore { >+class AVVideoCaptureSource; > >-class AVCaptureDeviceManager final : public CaptureDeviceManager { >+class AVCaptureDeviceManager final : public CaptureCameraManager { > friend class NeverDestroyed<AVCaptureDeviceManager>; > public: > const Vector<CaptureDevice>& captureDevices() final; >diff --git a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h >index 2a85ad44609a9d35464cde81088895ee2a7cdb73..87fb5339cd2590cffbf603f9e83d78e03dd9f2c1 100644 >--- a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h >+++ b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h >@@ -53,9 +53,7 @@ class ImageTransferSessionVT; > > class AVVideoCaptureSource : public RealtimeVideoCaptureSource, private OrientationNotifier::Observer { > public: >- static CaptureSourceOrError create(String&& id, String&& hashSalt, const MediaConstraints*); >- >- WEBCORE_EXPORT static VideoCaptureFactory& factory(); >+ static RefPtr<AVVideoCaptureSource> create(String&& id, String&& hashSalt); > > enum class InterruptionReason { None, VideoNotAllowedInBackground, AudioInUse, VideoInUse, VideoNotAllowedInSideBySide }; > void captureSessionBeginInterruption(RetainPtr<NSNotification>); >diff --git a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm >index 5c612edb323bf2e926377ed127d68781209970bf..db42d26a33762a9e9ec01d15f31a895dfb9ea519 100644 >--- a/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm >+++ b/Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm >@@ -28,6 +28,7 @@ > > #if ENABLE(MEDIA_STREAM) && USE(AVFOUNDATION) > >+#import "AVCaptureDeviceManager.h" > #import "ImageBuffer.h" > #import "ImageTransferSessionVT.h" > #import "IntRect.h" >@@ -109,20 +110,13 @@ public: > RetainPtr<AVCaptureDeviceFormat> format; > }; > >-CaptureSourceOrError AVVideoCaptureSource::create(String&& id, String&& hashSalt, const MediaConstraints* constraints) >+RefPtr<AVVideoCaptureSource> AVVideoCaptureSource::create(String&& id, String&& hashSalt) > { > AVCaptureDevice *device = [PAL::getAVCaptureDeviceClass() deviceWithUniqueID:id]; > if (!device) > return { }; > >- auto source = adoptRef(*new AVVideoCaptureSource(device, WTFMove(id), WTFMove(hashSalt))); >- if (constraints) { >- auto result = source->applyConstraints(*constraints); >- if (result) >- return WTFMove(result.value().badConstraint); >- } >- >- return CaptureSourceOrError(RealtimeVideoSource::create(WTFMove(source))); >+ return adoptRef(*new AVVideoCaptureSource(device, WTFMove(id), WTFMove(hashSalt))); > } > > AVVideoCaptureSource::AVVideoCaptureSource(AVCaptureDevice* device, String&& id, String&& hashSalt) >@@ -130,6 +124,7 @@ AVVideoCaptureSource::AVVideoCaptureSource(AVCaptureDevice* device, String&& id, > , m_objcObserver(adoptNS([[WebCoreAVVideoCaptureSourceObserver alloc] initWithCallback:this])) > , m_device(device) > { >+ AVCaptureDeviceManager::singleton().addVideoCaptureSource(*this); > #if PLATFORM(IOS_FAMILY) > static_assert(static_cast<int>(InterruptionReason::VideoNotAllowedInBackground) == static_cast<int>(AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableInBackground), "InterruptionReason::VideoNotAllowedInBackground is not AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableInBackground as expected"); > static_assert(static_cast<int>(InterruptionReason::VideoNotAllowedInSideBySide) == AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableWithMultipleForegroundApps, "InterruptionReason::VideoNotAllowedInSideBySide is not AVCaptureSessionInterruptionReasonVideoDeviceNotAvailableWithMultipleForegroundApps as expected"); >@@ -142,6 +137,7 @@ AVVideoCaptureSource::AVVideoCaptureSource(AVCaptureDevice* device, String&& id, > > AVVideoCaptureSource::~AVVideoCaptureSource() > { >+ AVCaptureDeviceManager::singleton().removeVideoCaptureSource(*this); > #if PLATFORM(IOS_FAMILY) > RealtimeMediaSourceCenter::singleton().videoCaptureFactory().unsetActiveSource(*this); > #endif >diff --git a/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm b/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm >index 9ddff40c1c523ba21e4dc46db6911d811513c635..f41399bbbf3ba29d137445a5b630f8634a8835c8 100644 >--- a/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm >+++ b/Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm >@@ -53,7 +53,7 @@ > namespace WebCore { > using namespace PAL; > >-CaptureSourceOrError MockRealtimeVideoSource::create(String&& deviceID, String&& name, String&& hashSalt, const MediaConstraints* constraints) >+RefPtr<RealtimeVideoCaptureSource> MockRealtimeVideoSource::create(String&& deviceID, String&& name, String&& hashSalt) > { > #ifndef NDEBUG > auto device = MockRealtimeMediaSourceCenter::mockDeviceWithPersistentID(deviceID); >@@ -61,13 +61,7 @@ CaptureSourceOrError MockRealtimeVideoSource::create(String&& deviceID, String&& > if (!device) > return { }; > #endif >- >- auto source = adoptRef(*new MockRealtimeVideoSourceMac(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))); >- // FIXME: We should report error messages >- if (constraints && source->applyConstraints(*constraints)) >- return { }; >- >- return CaptureSourceOrError(RealtimeVideoSource::create(WTFMove(source))); >+ return adoptRef(*new MockRealtimeVideoSourceMac(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))); > } > > MockRealtimeVideoSourceMac::MockRealtimeVideoSourceMac(String&& deviceID, String&& name, String&& hashSalt) >diff --git a/Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp b/Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp >index a328cd3a83ecbfca049e54abf70de9d78faf331b..9842b1e9cccff7be353fb7569777c7a88beed97e 100644 >--- a/Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp >+++ b/Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp >@@ -38,6 +38,7 @@ > #include "DisplayCaptureManagerCocoa.h" > #include "Logging.h" > #include "MediaStreamPrivate.h" >+#include "RealtimeVideoSource.h" > #include "RuntimeEnabledFeatures.h" > #include "ScreenDisplayCaptureSourceMac.h" > #include "WindowDisplayCaptureSourceMac.h" >@@ -45,14 +46,7 @@ > > namespace WebCore { > >-class VideoCaptureSourceFactoryMac final : public VideoCaptureFactory { >-public: >- CaptureSourceOrError createVideoCaptureSource(const CaptureDevice& device, String&& hashSalt, const MediaConstraints* constraints) final >- { >- ASSERT(device.type() == CaptureDevice::DeviceType::Camera); >- return AVVideoCaptureSource::create(String { device.persistentId() }, WTFMove(hashSalt), constraints); >- } >- >+class VideoCaptureSourceFactoryMac final : public CameraCaptureFactory { > private: > #if PLATFORM(IOS_FAMILY) > void setVideoCapturePageState(bool interrupted, bool pageMuted) >@@ -62,7 +56,21 @@ private: > } > #endif > >- CaptureDeviceManager& videoCaptureDeviceManager() { return AVCaptureDeviceManager::singleton(); } >+ RealtimeVideoCaptureSource* existingSource(const String& persistentID) final >+ { >+#if PLATFORM(IOS_FAMILY) >+ return (activeSource() && activeSource()->persistentID() == persistentID) ? static_cast<RealtimeVideoCaptureSource*>(activeSource()) : nullptr; >+#else >+ return AVCaptureDeviceManager::singleton().videoCaptureSource(persistentID); >+#endif >+ } >+ >+ RefPtr<RealtimeVideoCaptureSource> createNewSource(const CaptureDevice& device, String&& hashSalt) final >+ { >+ return AVVideoCaptureSource::create(String { device.persistentId() }, WTFMove(hashSalt)); >+ } >+ >+ CaptureDeviceManager& videoCaptureDeviceManager() final { return AVCaptureDeviceManager::singleton(); } > }; > > class DisplayCaptureSourceFactoryMac final : public DisplayCaptureFactory { >diff --git a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp >index d43168350af5a032c107db74bfbd3a4229f91605..06087df3d9a184204c229690ca0c7c7d4db88876 100644 >--- a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp >+++ b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp >@@ -84,17 +84,7 @@ static inline Vector<MockMediaDevice> defaultDevices() > }; > } > >-class MockRealtimeVideoSourceFactory : public VideoCaptureFactory { >-public: >- CaptureSourceOrError createVideoCaptureSource(const CaptureDevice& device, String&& hashSalt, const MediaConstraints* constraints) final >- { >- ASSERT(device.type() == CaptureDevice::DeviceType::Camera); >- if (!MockRealtimeMediaSourceCenter::captureDeviceWithPersistentID(CaptureDevice::DeviceType::Camera, device.persistentId())) >- return { }; >- >- return MockRealtimeVideoSource::create(String { device.persistentId() }, String { device.label() }, WTFMove(hashSalt), constraints); >- } >- >+class MockRealtimeVideoSourceFactory : public CameraCaptureFactory { > private: > #if PLATFORM(IOS_FAMILY) > void setVideoCapturePageState(bool interrupted, bool pageMuted) final >@@ -103,6 +93,21 @@ private: > activeSource()->setInterrupted(interrupted, pageMuted); > } > #endif >+ >+ RealtimeVideoCaptureSource* existingSource(const String& persistentID) final >+ { >+#if PLATFORM(IOS_FAMILY) >+ return (activeSource() && activeSource()->persistentID() == persistentID) ? static_cast<RealtimeVideoCaptureSource*>(activeSource()) : nullptr; >+#else >+ return MockRealtimeMediaSourceCenter::singleton().videoCaptureDeviceManager().videoCaptureSource(persistentID); >+#endif >+ } >+ >+ RefPtr<RealtimeVideoCaptureSource> createNewSource(const CaptureDevice& device, String&& hashSalt) final >+ { >+ return MockRealtimeVideoSource::create(String { device.persistentId() }, String { device.label() }, WTFMove(hashSalt)); >+ } >+ > CaptureDeviceManager& videoCaptureDeviceManager() final { return MockRealtimeMediaSourceCenter::singleton().videoCaptureDeviceManager(); } > }; > >@@ -115,9 +120,15 @@ public: > > switch (device.type()) { > case CaptureDevice::DeviceType::Screen: >- case CaptureDevice::DeviceType::Window: >- return MockRealtimeVideoSource::create(String { device.persistentId() }, String { device.label() }, String { }, constraints); >- break; >+ case CaptureDevice::DeviceType::Window: { >+ auto source = MockRealtimeVideoSource::create(String { device.persistentId() }, String { device.label() }, String { }); >+ if (constraints) { >+ auto result = source->applyConstraints(*constraints); >+ if (result) >+ return WTFMove(result.value().badConstraint); >+ } >+ return CaptureSourceOrError { RealtimeVideoSource::create(source.releaseNonNull()) }; >+ } > case CaptureDevice::DeviceType::Microphone: > case CaptureDevice::DeviceType::Camera: > case CaptureDevice::DeviceType::Unknown: >diff --git a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h >index 6d16fe77a6cebe3b551381b6b9aabb0eec934dd3..87741666db14ae2dfd82231a425ae2bd84330908 100644 >--- a/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h >+++ b/Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.h >@@ -59,8 +59,8 @@ public: > static Optional<MockMediaDevice> mockDeviceWithPersistentID(const String&); > static Optional<CaptureDevice> captureDeviceWithPersistentID(CaptureDevice::DeviceType, const String&); > >+ CaptureCameraManager& videoCaptureDeviceManager() { return m_videoCaptureDeviceManager; } > CaptureDeviceManager& audioCaptureDeviceManager() { return m_audioCaptureDeviceManager; } >- CaptureDeviceManager& videoCaptureDeviceManager() { return m_videoCaptureDeviceManager; } > CaptureDeviceManager& displayCaptureDeviceManager() { return m_displayCaptureDeviceManager; } > > private: >@@ -76,14 +76,14 @@ private: > const Vector<CaptureDevice>& captureDevices() final { return MockRealtimeMediaSourceCenter::audioDevices(); } > Optional<CaptureDevice> captureDeviceWithPersistentID(CaptureDevice::DeviceType type, const String& id) final { return MockRealtimeMediaSourceCenter::captureDeviceWithPersistentID(type, id); } > }; >- class MockVideoCaptureDeviceManager final : public CaptureDeviceManager { >+ class MockDisplayCaptureDeviceManager final : public CaptureDeviceManager { > private: >- const Vector<CaptureDevice>& captureDevices() final { return MockRealtimeMediaSourceCenter::videoDevices(); } >+ const Vector<CaptureDevice>& captureDevices() final { return MockRealtimeMediaSourceCenter::displayDevices(); } > Optional<CaptureDevice> captureDeviceWithPersistentID(CaptureDevice::DeviceType type, const String& id) final { return MockRealtimeMediaSourceCenter::captureDeviceWithPersistentID(type, id); } > }; >- class MockDisplayCaptureDeviceManager final : public CaptureDeviceManager { >+ class MockVideoCaptureDeviceManager final : public CaptureCameraManager { > private: >- const Vector<CaptureDevice>& captureDevices() final { return MockRealtimeMediaSourceCenter::displayDevices(); } >+ const Vector<CaptureDevice>& captureDevices() final { return MockRealtimeMediaSourceCenter::videoDevices(); } > Optional<CaptureDevice> captureDeviceWithPersistentID(CaptureDevice::DeviceType type, const String& id) final { return MockRealtimeMediaSourceCenter::captureDeviceWithPersistentID(type, id); } > }; > >diff --git a/Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp b/Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp >index 04ee3b4a9062b8ae9bda16218113f1e367f676a2..2b39dbe4b07dad2dd262a531f9ba9c77b91031f8 100644 >--- a/Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp >+++ b/Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp >@@ -50,7 +50,7 @@ > namespace WebCore { > > #if !PLATFORM(MAC) && !PLATFORM(IOS_FAMILY) && !(USE(GSTREAMER) && USE(LIBWEBRTC)) >-CaptureSourceOrError MockRealtimeVideoSource::create(String&& deviceID, String&& name, String&& hashSalt, const MediaConstraints* constraints) >+RefPtr<RealtimeVideoCaptureSource> MockRealtimeVideoSource::create(String&& deviceID, String&& name, String&& hashSalt) > { > #ifndef NDEBUG > auto device = MockRealtimeMediaSourceCenter::mockDeviceWithPersistentID(deviceID); >@@ -58,12 +58,7 @@ CaptureSourceOrError MockRealtimeVideoSource::create(String&& deviceID, String&& > if (!device) > return { }; > #endif >- >- auto source = adoptRef(*new MockRealtimeVideoSource(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))); >- if (constraints && source->applyConstraints(*constraints)) >- return { }; >- >- return CaptureSourceOrError(RealtimeVideoSource::create(WTFMove(source))); >+ return adoptRef(*new MockRealtimeVideoSource(WTFMove(deviceID), WTFMove(name), WTFMove(hashSalt))); > } > #endif > >@@ -91,6 +86,28 @@ MockRealtimeVideoSource::MockRealtimeVideoSource(String&& deviceID, String&& nam > setFrameRate(properties.defaultFrameRate); > setFacingMode(properties.facingMode); > m_fillColor = properties.fillColor; >+ >+ MockRealtimeMediaSourceCenter::singleton().videoCaptureDeviceManager().addVideoCaptureSource(*this); >+} >+ >+MockRealtimeVideoSource::~MockRealtimeVideoSource() >+{ >+ if (mockDisplay()) >+ return; >+ MockRealtimeMediaSourceCenter::singleton().videoCaptureDeviceManager().removeVideoCaptureSource(*this); >+} >+ >+CaptureDevice::DeviceType MockRealtimeVideoSource::deviceType() const >+{ >+ if (mockCamera()) >+ return CaptureDevice::DeviceType::Camera; >+ if (mockScreen()) >+ return CaptureDevice::DeviceType::Screen; >+ if (mockWindow()) >+ return CaptureDevice::DeviceType::Window; >+ >+ ASSERT_NOT_REACHED(); >+ return CaptureDevice::DeviceType::Unknown; > } > > bool MockRealtimeVideoSource::supportsSizeAndFrameRate(Optional<int> width, Optional<int> height, Optional<double> rate) >diff --git a/Source/WebCore/platform/mock/MockRealtimeVideoSource.h b/Source/WebCore/platform/mock/MockRealtimeVideoSource.h >index 91c7daeaa3c499e5c950fdd6b9b52c91efa1deee..19a3cde00de681cb96a22458caea168dfcce7586 100644 >--- a/Source/WebCore/platform/mock/MockRealtimeVideoSource.h >+++ b/Source/WebCore/platform/mock/MockRealtimeVideoSource.h >@@ -48,11 +48,11 @@ class GraphicsContext; > > class MockRealtimeVideoSource : public RealtimeVideoCaptureSource, private OrientationNotifier::Observer { > public: >- >- static CaptureSourceOrError create(String&& deviceID, String&& name, String&& hashSalt, const MediaConstraints*); >+ static RefPtr<RealtimeVideoCaptureSource> create(String&& deviceID, String&& name, String&& hashSalt); > > protected: > MockRealtimeVideoSource(String&& deviceID, String&& name, String&& hashSalt); >+ ~MockRealtimeVideoSource(); > > virtual void updateSampleBuffer() = 0; > >@@ -70,7 +70,7 @@ private: > void startProducingData() final; > void stopProducingData() final; > bool isCaptureSource() const final { return true; } >- CaptureDevice::DeviceType deviceType() const final { return CaptureDevice::DeviceType::Camera; } >+ CaptureDevice::DeviceType deviceType() const final; > bool supportsSizeAndFrameRate(Optional<int> width, Optional<int> height, Optional<double>) final; > void setSizeAndFrameRate(Optional<int> width, Optional<int> height, Optional<double>) final; > void setFrameRateWithPreset(double, RefPtr<VideoPreset>) final; >diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp >index cdf1694ee6b4dfc82194b116bf3a24130ac31716..af59f6f0fa6df5e271675cbbb6d14a4b5285ffad 100644 >--- a/Source/WebCore/testing/Internals.cpp >+++ b/Source/WebCore/testing/Internals.cpp >@@ -521,6 +521,9 @@ void Internals::resetToConsistentState(Page& page) > MockPreviewLoaderClient::singleton().setPassword(""); > PreviewLoader::setClientForTesting(nullptr); > #endif >+#if ENABLE(MEDIA_STREAM) >+ MockRealtimeMediaSourceCenter::singleton().videoCaptureDeviceManager().clearSources(); >+#endif > > printContextForTesting() = nullptr; > >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index 0720871a206e4c3032c0298ff1fe56938a63d8dc..117e0ba84ee986566afb3c45ffef46a5ecd25763 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,14 @@ >+2019-08-02 Youenn Fablet <youenn@apple.com> >+ >+ [MacOS] getUserMedia called multiple times should better handle camera resolution >+ https://bugs.webkit.org/show_bug.cgi?id=198839 >+ <rdar://problem/51718990> >+ >+ Reviewed by Eric Carlson. >+ >+ Update test to cover the case of successive calls to getUserMedia selecting the same camera. >+ * platform/ios/mediastream/getUserMedia-single-capture.html: >+ > 2019-08-01 Youenn Fablet <youenn@apple.com> > > UserMediaPermissionRequestManagerProxy should not use audio+video denied requests to deny audio-only or video-only requests >diff --git a/LayoutTests/platform/ios/mediastream/getUserMedia-single-capture.html b/LayoutTests/platform/ios/mediastream/getUserMedia-single-capture.html >index 689f41d0ba8a156e68a1bc9590e3a8ce58120fc0..0357200f9ad35ed8b952f1be20b24f73ab248f9f 100644 >--- a/LayoutTests/platform/ios/mediastream/getUserMedia-single-capture.html >+++ b/LayoutTests/platform/ios/mediastream/getUserMedia-single-capture.html >@@ -8,31 +8,40 @@ > </head> > <body> > <script> >-promise_test((test) => { >+promise_test(async (test) => { > if (window.testRunner) > testRunner.setUserMediaPermission(true); > >- var firstStream; >- var audioTrack; >- var videoTrack; >- return navigator.mediaDevices.getUserMedia({ audio: true, video: true }).then((stream) => { >- firstStream = stream; >- audioTrack = firstStream.getAudioTracks()[0]; >- videoTrack = firstStream.getVideoTracks()[0]; >+ const firstStream = await navigator.mediaDevices.getUserMedia({ audio: true, video: true }); >+ const audioTrack = firstStream.getAudioTracks()[0]; >+ const videoTrack = firstStream.getVideoTracks()[0]; > >- assert_false(audioTrack.muted, "audio track is active"); >- assert_false(videoTrack.muted, "video track is active"); >+ assert_false(audioTrack.muted, "audio track is active"); >+ assert_false(videoTrack.muted, "video track is active"); > >- return navigator.mediaDevices.getUserMedia({ audio: true}); >- }).then((stream) => { >- assert_true(audioTrack.muted, "audio track is muted"); >- assert_false(videoTrack.muted, "video track is active"); >+ await navigator.mediaDevices.getUserMedia({ audio: true}); >+ assert_true(audioTrack.muted, "audio track is muted"); >+ assert_false(videoTrack.muted, "video track is active"); > >- return navigator.mediaDevices.getUserMedia({ video: true}); >- }).then((stream) => { >- assert_true(audioTrack.muted, "audio track is muted"); >- assert_true(videoTrack.muted, "video track is muted"); >+ await navigator.mediaDevices.getUserMedia({ video: true}); >+ assert_true(audioTrack.muted, "audio track is still muted"); >+ assert_false(videoTrack.muted, "video track is not muted"); >+ >+ const devices = await navigator.mediaDevices.enumerateDevices(); >+ let deviceId; >+ let counter = 0; >+ devices.forEach(device => { >+ if (device.kind != "videoinput") >+ return; >+ if (!counter++) >+ return; >+ deviceId = device.deviceId; > }); >+ >+ await navigator.mediaDevices.getUserMedia({ video: { deviceId : {exact : deviceId }}}); >+ assert_true(audioTrack.muted, "audio track is still muted"); >+ assert_true(videoTrack.muted, "video track is muted"); >+ > }, "Testing successive getUserMedia calls"); > </script> > </body>
What happened here. Did this land?
This was fixed by https://bugs.webkit.org/show_bug.cgi?id=198840