GNOME Bugzilla – Bug 619508
[tag] Add image orientation tag
Last modified: 2010-06-23 15:19:23 UTC
This is the "Orientation" tag description from exif's spec: Orientation The image orientation viewed in terms of rows and columns. Tag = 274 (112.H) Type = SHORT Count = 1 Default = 1 1 = The 0th row is at the visual top of the image, and the 0th column is the visual left-hand side. 2 = The 0th row is at the visual top of the image, and the 0th column is the visual right-hand side. 3 = The 0th row is at the visual bottom of the image, and the 0th column is the visual right-hand side. 4 = The 0th row is at the visual bottom of the image, and the 0th column is the visual left-hand side. 5 = The 0th row is the visual left-hand side of the image, and the 0th column is the visual top. 6 = The 0th row is the visual right-hand side of the image, and the 0th column is the visual top. 7 = The 0th row is the visual right-hand side of the image, and the 0th column is the visual bottom. 8 = The 0th row is the visual left-hand side of the image, and the 0th column is the visual bottom. Other = reserved Shortly, it tells us how should we mirror or rotate the picture before displaying it to users. I know some applications handle this properly (gimp and windows image viewer, for example), we could make gstreamer handle it, too. Adding a tag is trivial, I'm just asking here how should we represent this one as we don't have enum tags so far. One option is to use strings as 'mirror-vertical', 'rotate-90-cw'... The strings have intuitive names, but I think this is error prone unless we have macros for each one. But if we are going to use macros, I'd prefer them to represent the integers from the spec. Keep in mind that there are some other tags from exif that are enums as well, so we might want to take a longer term and generic decision here.
Any ideas?
Maybe: 0 - rotate-0 3 - rotate-180 6 - rotate-270 8 - rotate-90 and either: 5 - transpose-rotate-0 2 - transpose-rotate-90 7 - transpose-rotate-180 4 - transpose-rotate-270 or: 2 - flip-rotate-0 7 - flip-rotate-90 4 - flip-rotate-180 5 - flip-rotate-270 (what 'flip' means exactly would then need to be documented). Inspired by: http://www.impulseadventure.com/photo/exif-orientation.html http://sylvana.net/jpegcrop/exif_orientation.html The latter also refers to a naming scheme (for a command line tool). I don't think we should be using an enum for this, since we'd need to register a type for that plus get_type function for bindings etc. and that causes all kinds of additional problems (thwarting other masterplans, possibly). I'd just make it a string and document the possible values. Alternatively, split the tag into two tags, one for the rotation value (double or int) and an additional one for the transpose/flip operation.
Created attachment 163736 [details] [review] tag: Adds GST_TAG_IMAGE_ORIENTATION tag Adds GST_TAG_IMAGE_ORIENTATION and its possible values as macros
Hrm, feel free to accuse me of bikeshedding, but ... I don't particularly like adding defines for all the values - if we do that, we may just as well add a proper enum and see what happens, since bindings would want to bind the values explicitly as well if they're in the API. Then the names: "image-orientation-flip-horizontal-rotate-90" - this seems a lot less elegant / more convoluted to me than e.g. my suggestions in comment #2.
Created attachment 163812 [details] [review] tag: Adds GST_TAG_IMAGE_ORIENTATION tag Updated patch removing the macros and shortening the tag valid strings.
Created attachment 163935 [details] [review] tag: Adds GST_TAG_IMAGE_ORIENTATION tag Simplifies valid input names and changes rotations to be in CW direction
FYI, the use case here is to add this to xmp/exif helper libraries, allowing it to be used when writing jpeg files so that image viewers can properly show those images (if they are rotated/flipped somehow). It should also be possible to write a flip/rotate element to correct the orientation in gstreamer pipelines.
Review of attachment 163935 [details] [review]: Overall, this seems ok to me. There is a part of me that's not entirely happy about the covert enum / predefined string values thing, but that part is equally reluctant to start adding enum tags now, with all the API and bindings hassle this would entail, especially for a tag like this where there's a clear noop transform/value. So while it's not the perfect solution, I can't think of better solutions either, nor imagine a scenario where someone really misses the possibility to introspect the possible settings here. ::: gst/gsttaglist.c @@ +353,3 @@ + _("image orientation"), + _("Represents how the image should be rotated or flipped before " + "ehxibition"), NULL); ehxibition => exhibition (or just 'display'?) ::: gst/gsttaglist.h @@ +945,3 @@ + * + * Represents the 'Orientation' tag from EXIF. Defining how the image + * should be rotated and mirrored for ehxibition. (string) ehxibition => exhibition @@ +947,3 @@ + * should be rotated and mirrored for ehxibition. (string) + * + * This tag has a predefined set of possible inputs: possible inputs => allowed values maybe ? @@ +951,3 @@ + * "flip-rotate-0" + * "rotate-180" + * "flip-rotate-180" Can we put these in order now that the original relation to the number value is gone? ie. 0/90/180/270, etc.
Created attachment 164399 [details] [review] tag: Adds GST_TAG_IMAGE_ORIENTATION tag Updated patch with Tim's comments.
Fixed. commit 7a34c1cd189ba27fd1ba56f180475c13fe080cc3 Author: Thiago Santos <thiago.sousa.santos@collabora.co.uk> Date: Tue Jun 15 18:26:01 2010 -0300 tag: Adds GST_TAG_IMAGE_ORIENTATION tag Adds a new tag to inform about the image orientation and how to rotate and flip it before display. Note that this tag is a string with a predefined set of possible values. API: GST_TAG_IMAGE_ORIENTATION Fixes #619508