[webkit-reviews] review granted: [Bug 191587] [MediaStream] Observer AVCaptureDevice "suspended" property : [Attachment 354675] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 13 10:35:12 PST 2018
youenn fablet <youennf at gmail.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 191587: [MediaStream] Observer AVCaptureDevice "suspended" property
https://bugs.webkit.org/show_bug.cgi?id=191587
Attachment 354675: Patch
https://bugs.webkit.org/attachment.cgi?id=354675&action=review
--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 354675
--> https://bugs.webkit.org/attachment.cgi?id=354675
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=354675&action=review
> Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.h:72
> + RetainPtr<NSMutableArray> m_AVCaptureDevices;
Should it be named m_avCaptureDevices?
> Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:145
> + stopObservingDevices(scratchArray.get());
I would tend to move the computation of removed devices in
stopObservingDevices.
That way, it is clear that we are doing the right thing in terms of begin/stop
observing.
It seems we might iterate twice over currentDevices but that might not be a big
deal.
s/beginObserving/startObserving?
> Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:159
> + deviceList.append(captureDevice);
WTFMove(captureDevice).
> Source/WebCore/platform/mediastream/mac/AVCaptureDeviceManager.mm:171
> + m_devices = deviceList;
WTFMove(deviceList);
> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:556
> + notifyMutedChange(isSuspended);
notifyMutedChange is checking whether muted == m_muted, so we might be able to
just write;
notifyMutedChange([m_device isSuspended]);
More information about the webkit-reviews
mailing list