Following orientation allows keeping the camera capturing the scene even if rotating the device.
Created attachment 308150 [details] Patch
(In reply to youenn fablet from comment #1) > Created attachment 308150 [details] > Patch WIP doing also some refactoring in the capture factories.
Created attachment 308154 [details] Patch
Created attachment 308161 [details] Patch
Attachment 308161 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:247: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:248: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 308271 [details] Fixing GTK build
(In reply to youenn fablet from comment #6) > Created attachment 308271 [details] > Fixing GTK build orientation adaptation in AVVideoCaptureSource is untested and probably broken now. Will do some manual testing. Interested in feedback on the overall design and refactoring.
Attachment 308271 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:247: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:248: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 2 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.
<rdar://problem/31671907>
Created attachment 308470 [details] Patch
Created attachment 308472 [details] Patch
Attachment 308472 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:467: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:247: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:248: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 308475 [details] Patch
Attachment 308475 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:467: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:247: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:248: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 308475 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308475&action=review > Source/WebCore/platform/mediastream/RealtimeMediaSource.h:250 > + operator bool() const { return !!captureSource; } Is the "!!" necessary? > Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:167 > + // FIXME: Do something with m_deviceOrientation Nit: you could not that this is https://bugs.webkit.org/show_bug.cgi?id=169822. > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:140 > + if (!!audioSource) Why is the "!!" necessary since CaptureSourceOrError has an operator bool()? > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:149 > + if (!!videoSource) Ditto. > Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:91 > + if (!!sourceOrError && sourceOrError.captureSource->supportsConstraints(audioConstraints, invalidConstraint)) Ditto. > Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:139 > + if (!!sourceOrError) Ditto. > Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:143 > + if (!!sourceOrError) Ditto. > Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:152 > + if (!!sourceOrError) Ditto. > Source/WebCore/platform/mock/MockRealtimeMediaSourceCenter.cpp:156 > + if (!!sourceOrError) Ditto. > Source/WebKit2/UIProcess/Cocoa/UserMediaCaptureManagerProxy.cpp:144 > + succeeded = !!sourceOrError; Ditto. > Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.cpp:101 > - ASSERT(!idToHistoryItemMap().contains(itemID)); > +// ASSERT(!idToHistoryItemMap().contains(itemID)); Oops!
Created attachment 308597 [details] Patch for landing
Attachment 308597 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:467: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:247: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:248: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 38 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 308597 [details] Patch for landing Clearing flags on attachment: 308597 Committed r215955: <http://trac.webkit.org/changeset/215955>
All reviewed patches have been landed. Closing bug.
Thanks for the review. I took all of it in https://bugs.webkit.org/show_bug.cgi?id=171466 > > Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:167 > > + // FIXME: Do something with m_deviceOrientation > > Nit: you could not that this is > https://bugs.webkit.org/show_bug.cgi?id=169822. Done > > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:140 > > + if (!!audioSource) > > Why is the "!!" necessary since CaptureSourceOrError has an operator bool()? Done for all these. > > Source/WebKit2/WebProcess/WebPage/WebBackForwardListProxy.cpp:101 > > - ASSERT(!idToHistoryItemMap().contains(itemID)); > > +// ASSERT(!idToHistoryItemMap().contains(itemID)); > > Oops! Fixed.