Bug 148524

Summary: [GStreamer] video orientation support
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, slomo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Philippe Normand 2015-08-27 04:37:04 PDT
We should handle the GST_TAG_IMAGE_ORIENTATION tag and update the orientation of the video texture accordingly.

Another option would be to use videoflip as a playbin video-filter if GL isn't available.

See also https://bugzilla.gnome.org/show_bug.cgi?id=679522
Comment 1 Miguel Gomez 2016-06-17 09:22:51 PDT
Created attachment 281565 [details]
Patch
Comment 2 Philippe Normand 2016-06-20 00:25:43 PDT
Comment on attachment 281565 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=281565&action=review

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2003
> +        // the image-orientation tag

Missing full stop

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:2004
> +        GstElement* videoFlip = gst_element_factory_make("videoflip", 0);

nullptr :)

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:294
> +    // When using accelerated compositing, if the video is tagged as rotated 90 or 270 degrees, swap width and height

Missing full stop

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:711
> +    switch (m_videoSourceRotation) {

The switch seems useful only when USE(TEXTURE_MAPPER_GL) is on. So maybe use a single ifdef englobing the switch ?

> LayoutTests/media/video-orientation-expected.txt:18
> +EXPECTED (video.videoWidth == '352') OK
> +EXPECTED (video.videoHeight == '288') OK

This doesn't match with the rotation-90.mp4 it seems?

> LayoutTests/media/video-orientation-expected.txt:32
> +EXPECTED (video.videoWidth == '352') OK
> +EXPECTED (video.videoHeight == '288') OK

Ditto
Comment 3 Miguel Gomez 2016-06-20 01:15:12 PDT
Created attachment 281638 [details]
Patch
Comment 4 Philippe Normand 2016-06-20 01:30:41 PDT
Comment on attachment 281638 [details]
Patch

Looks good! Let's wait EWS finishes to land this.
Comment 5 WebKit Commit Bot 2016-06-20 03:03:40 PDT
Comment on attachment 281638 [details]
Patch

Rejecting attachment 281638 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 281638, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
LayoutTests
:040000 040000 105f7e2febfadc8d1f550aa805ccd215e8f27c27 f3f1d4fa1265a983a518943e7e3a0f794477e6bd M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.webkit.org/results/1535702
Comment 6 Philippe Normand 2016-06-21 01:11:42 PDT
Comment on attachment 281638 [details]
Patch

Let's try cq again.
Comment 7 WebKit Commit Bot 2016-06-21 01:32:16 PDT
Comment on attachment 281638 [details]
Patch

Clearing flags on attachment: 281638

Committed r202272: <http://trac.webkit.org/changeset/202272>
Comment 8 WebKit Commit Bot 2016-06-21 01:32:20 PDT
All reviewed patches have been landed.  Closing bug.