Bug 171284 – getUserMedia video streams should follow device orientation

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[youenn fablet]

Following orientation allows keeping the camera capturing the scene even if rotating the device.

[youenn fablet]

Created attachment 308150 [details]
Patch

[youenn fablet]

(In reply to youenn fablet from comment #1)
> Created attachment 308150 [details]
> Patch

WIP doing also some refactoring in the capture factories.

[youenn fablet]

Created attachment 308154 [details]
Patch

[youenn fablet]

Created attachment 308161 [details]
Patch

[Build Bot]

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.

[youenn fablet]

Created attachment 308271 [details]
Fixing GTK build

[youenn fablet]

(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.

[Build Bot]

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.

[Jon Lee]

<rdar://problem/31671907>

[youenn fablet]

Created attachment 308470 [details]
Patch

[youenn fablet]

Created attachment 308472 [details]
Patch

[Build Bot]

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.

[youenn fablet]

Created attachment 308475 [details]
Patch

[Build Bot]

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.

[Eric Carlson]

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!

[youenn fablet]

Created attachment 308597 [details]
Patch for landing

[Build Bot]

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.

[WebKit Commit Bot]

Comment on attachment 308597 [details]
Patch for landing

Clearing flags on attachment: 308597

Committed r215955: <http://trac.webkit.org/changeset/215955>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[youenn fablet]

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.