After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 709312 - videoflip: Add an automatic method that flip base on image-orientation tag
videoflip: Add an automatic method that flip base on image-orientation tag
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-10-02 19:53 UTC by Nicolas Dufresne (ndufresne)
Modified: 2013-10-04 21:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videoflip: Add automatic flip mode driven by image-orientation tag (7.15 KB, patch)
2013-10-02 19:53 UTC, Nicolas Dufresne (ndufresne)
needs-work Details | Review
[PATCH] videoflip: Add automatic flip mode driven by image-orientation tag (10.12 KB, patch)
2013-10-03 20:42 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2013-10-02 19:53:00 UTC
Created attachment 256319 [details] [review]
videoflip: Add automatic flip mode driven by image-orientation tag

It's not particularly useful right now, as we don't have any ranked jpeg parser, though I wanted to be able to display jpegs with the corrected orientation. So I added an automatic method to videoflip.

The general idea is that when we receive the GST_TAG_IMAGE_ORIENTATION, we automatically choose the right transformation. The value of this tag is a perfect match with the videoflip methods.
Comment 1 Nicolas Dufresne (ndufresne) 2013-10-02 19:56:23 UTC
Here is a simple pipeline to test:
gst-launch-1.0 -m filesrc location=12080002.jpg ! queue ! jpegparse ! jpegdec ! imagefreeze ! videoconvert ! videoscale ! videoflip method=0 ! xvimagesink

The property handling might not be completly right, maybe we should keep reporting automatic on g_object_get() ? Also I'm missing a test, if review is mostly positive I'll add the tests.
Comment 2 Nicolas Dufresne (ndufresne) 2013-10-02 19:56:43 UTC
Sorry, method=automatic obviously
Comment 3 Sebastian Dröge (slomo) 2013-10-03 09:57:15 UTC
Review of attachment 256319 [details] [review]:

::: gst/videofilter/gstvideoflip.c
@@ +1057,3 @@
+        video_flip_methods[method].value_nick);
+
+    videoflip->method = method;

The handling of the properties does not make sense to me. You will now return non-auto from get_property.

I think you should just store the currently in use method separate from the set property. So the former can never be auto, but the latter can be auto, and in that case the former would change on tag events.
Comment 4 Nicolas Dufresne (ndufresne) 2013-10-03 20:42:22 UTC
Created attachment 256421 [details] [review]
[PATCH] videoflip: Add automatic flip mode driven by image-orientation tag


https://bugzilla.gnome.org/show_bug.cgi?id=709312
---
 gst/videofilter/gstvideoflip.c | 128 ++++++++++++++++++++++++++++++-----------
 gst/videofilter/gstvideoflip.h |   6 +-
 2 files changed, 101 insertions(+), 33 deletions(-)
Comment 5 Nicolas Dufresne (ndufresne) 2013-10-03 20:45:27 UTC
Comment on attachment 256421 [details] [review]
[PATCH] videoflip: Add automatic flip mode driven by image-orientation tag

I've gone a little further by keeping both the active and the tag method, in a way that if you switch to automatic at anymoment, the method will always reflect that last seen tag. Hope that's better now. I've looked at the tests, but there is nothing there very useful, would have to rething the test a bit to test this. Let me know if you feel this is absolutly required.
Comment 6 Nicolas Dufresne (ndufresne) 2013-10-04 21:00:29 UTC
commit ed77b22f2b5dc05caad844f6c35a815aed1ce863
Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Date:   Thu Oct 3 16:39:26 2013 -0400

    videoflip: Add automatic flip mode driven by image-orientation tag
    
    https://bugzilla.gnome.org/show_bug.cgi?id=709312