We currently send a list of devices, no matter what, to Safari for requesting user media. There should be an easy way to create a list of only devices that can satisfy constraints.
<rdar://problem/21931121>
Created attachment 257218 [details] Patch
Comment on attachment 257218 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257218&action=review Doesn’t build because you forgot to update the other implementations of MediaStreamCreationClient, specifically MockRealtimeMediaSourceCenter. > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:126 > + for (auto audioTrack : audioTracks) This will churn reference counts unnecessarily as we iterate each item and copy it into a RefPtr. To fix that, use auto& instead of auto. > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:128 > + for (auto videoTrack : videoTracks) Ditto. > Source/WebCore/Modules/mediastream/UserMediaRequest.h:74 > + Vector<String> filteredListUIDsVideo() const { return m_filteredListUIDsVideo; } > + Vector<String> filteredListUIDsAudio() const { return m_filteredListUIDsAudio; } It’s inefficient to copy these vectors just to return them. Please consider returning const Vector<String>& instead. > Source/WebCore/platform/mediastream/MediaStreamCreationClient.h:41 > + virtual void constraintsValidated(Vector<RefPtr<RealtimeMediaSource>>, Vector<RefPtr<RealtimeMediaSource>>) = 0; It’s inefficient to copy these vectors as part of calling this function. Please consider const Vector&. Also, it’s not clear what these two vectors represent, so these arguments need names. > Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:383 > + Vector<RefPtr<RealtimeMediaSource>> bestSourcesList = Vector<RefPtr<RealtimeMediaSource>>(); No need to do the "=" part here. Vectors are empty by default without any explicit initialization. > Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:389 > Vector<CaptureDevice>& devices = captureDeviceList(); > size_t count = devices.size(); This code should really use a modern for loop, like this: for (auto& device : captureDeviceList()) { Then below use “device” wherever the old code used “devices[i]”. Only issue is that we’d need to rename the local variables named device below. > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:102 > + // FIXME: consider the constraints when choosing among multiple devices. For now just select the first available Please capitalize the sentence in a comment like this. The word "consider". > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:117 > + // FIXME: consider the constraints when choosing among multiple devices. For now just select the first available Please capitalize the sentence in a comment like this. The word "consider".
Created attachment 257221 [details] Patch
Comment on attachment 257221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257221&action=review Looks good overall, but it obviously needs to be rebased so it applies to trunk. > Source/WebCore/Modules/mediastream/UserMediaRequest.h:74 > + const Vector<String>& deviceUIDsVideo() const { return m_deviceUIDsVideo; } > + const Vector<String>& deviceUIDsAudio() const { return m_deviceUIDsAudio; } Nit: I think audioDeviceUIDs and videoDeviceUIDs would be better. Also for the instance variables.
Comment on attachment 257221 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257221&action=review >> Source/WebCore/Modules/mediastream/UserMediaRequest.h:74 >> + const Vector<String>& deviceUIDsAudio() const { return m_deviceUIDsAudio; } > > Nit: I think audioDeviceUIDs and videoDeviceUIDs would be better. Also for the instance variables. Fixed.
Created attachment 257231 [details] Patch
Created attachment 257236 [details] Patch
Comment on attachment 257236 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257236&action=review > Source/WebCore/Modules/mediastream/UserMediaRequest.h:80 > + virtual void constraintsValidated(const Vector<RefPtr<RealtimeMediaSource>>&, const Vector<RefPtr<RealtimeMediaSource>>&) override final; As Darin noted earlier, it is not clear what these two vectors represent so these arguments need names.
Comment on attachment 257236 [details] Patch Clearing flags on attachment: 257236 Committed r187168: <http://trac.webkit.org/changeset/187168>
All reviewed patches have been landed. Closing bug.