<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><font size="2">On 18 April 2016 at 21:49, Jean-Baptiste Kempf <span dir="ltr"><<a href="mailto:jb@videolan.org" target="_blank">jb@videolan.org</a>></span> wrote:<br></font><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><font size="2">I am very sorry, but I cannot apply this patch. It is corrupted for me<br>
everytime, because of wrapping and line endings.<br></font></blockquote><div><font size="2"><br></font></div><div><font size="2">My apologies. I will try very hard to get this right. <br><br></font></div><div><font size="2">Changes in this patch:<br></font></div><div><div><font size="2"><br>a) Made orientation automatic, with no option to disable<br></font></div><div><font size="2">to address your mild concerns and <span style="font-family:monospace,monospace"><span class="im">Rémi</span>'s</span> firm objections.<br></font></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><font size="2"><span class=""><br></span>Well, I'm surprised GetWLE and GetWBE are not enough for you, and<br>
hton16|32|64, who are actually swapping or not depending on the machine.<br>
We have many other formats, so I'm surprised we need something new<br>
here...<br></font>
<div class=""><div class="h5"><font size="2"><br></font></div></div></blockquote><div><font size="2">b) I spent an hour looking at this, and can't see the existing functions will apply.<br></font></div><div><font size="2">So someone smarter than me will have to address this. I have made detailed<br></font></div><font size="2">comments.<br><br></font></div><div><font size="2">c) Amended the citation to LGPL code I modified, basing it on other VLC<br></font></div><div><font size="2">code citations<br></font><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<font size="2">define i_otag in the scope where you do need it.<br></font>
<font size="2"><br></font></blockquote><div>d) Have moved definitions, CONST and DEFINEs to be near their relevant code<br> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><font size="2">
The rest starts looking really good... (mention it in NEWS?)<br></font>
<div class=""><div class="h5"><font size="2"><br></font></div></div></blockquote><div>e) included patch for NEWS<br> <br></div>[PATCH Automatically orient JPEG image based on orientation tag, if present<br><br>---<br> NEWS | 1 +<br> modules/codec/jpeg.c | 225 ++++++++++++++++++++++++++++++++++++++++++++++++++-<br> 2 files changed, 225 insertions(+), 1 deletion(-)<br><br>diff --git a/NEWS b/NEWS<br>index 5233b81..dd37984 100644<br>--- a/NEWS<br>+++ b/NEWS<br>@@ -67,6 +67,7 @@ Decoder:<br> supporting H.263, H.264/MPEG-4 AVC, MPEG-4 Part 2, and DV depending on device<br> and OS version<br> * Support for the OggSpots video codec<br>+ * JPEG images correctly oriented using embedded orientation tag, if present<br> <br> Demuxers:<br> * Support HD-DVD .evo (H.264, VC-1, MPEG-2, PCM, AC-3, E-AC3, MLP, DTS)<br>diff --git a/modules/codec/jpeg.c b/modules/codec/jpeg.c<br>index fdb91c7..69c7a69 100644<br>--- a/modules/codec/jpeg.c<br>+++ b/modules/codec/jpeg.c<br>@@ -1,7 +1,7 @@<br> /*****************************************************************************<br> * jpeg.c: jpeg decoder module making use of libjpeg.<br> *****************************************************************************<br>- * Copyright (C) 2013-2014 VLC authors and VideoLAN<br>+ * Copyright (C) 2013-2014,2016 VLC authors and VideoLAN<br> *<br> * Authors: Maxim Bublis <<a href="mailto:b@codemonkey.ru">b@codemonkey.ru</a>><br> *<br>@@ -182,6 +182,220 @@ static int OpenDecoder(vlc_object_t *p_this)<br> }<br> <br> /*<br>+ * The following two functions are used to return 16 and 32 bit values from<br>+ * the EXIF tag structure. That structure is borrowed from TIFF files and may be<br>+ * in big endian or little endian format. The endian parameter tells us which.<br>+ * Case Little Endian EXIF tag / Little Endian machine<br>+ * - just memcpy the tag structure into the value to return<br>+ * Case Little Endian EXIF tag / Big Endian machine<br>+ * - memcpy the tag structure into value, and bswap it<br>+ * Case Little Endian EXIF tag / Big Endian machine<br>+ * - memcpy the tag structure into value, and bswap it<br>+ * Case Big Endian EXIF tag / Big Endian machine<br>+ * - just memcpy the tag structure into the value to return<br>+ *<br>+ * While there are byte manipulation functions in vlc_common.h, none of them<br>+ * address that format of the data supplied may be either big or little endian.<br>+ *<br>+ * The claim is made that this is the best way to do it. Can you do better?<br>+*/<br>+<br>+#define G_LITTLE_ENDIAN 1234<br>+#define G_BIG_ENDIAN 4321<br>+<br>+LOCAL( unsigned short )<br>+de_get16( void * ptr, uint endian ) {<br>+ unsigned short val;<br>+<br>+ memcpy( &val, ptr, sizeof( val ) );<br>+ if ( endian == G_BIG_ENDIAN )<br>+ {<br>+ #ifndef WORDS_BIGENDIAN<br>+ val = bswap16( val );<br>+ #endif<br>+ }<br>+ else<br>+ {<br>+ #ifdef WORDS_BIGENDIAN<br>+ val = bswap16( val );<br>+ #endif<br>+ }<br>+ return val;<br>+}<br>+<br>+LOCAL( unsigned int )<br>+de_get32( void * ptr, uint endian ) {<br>+ unsigned int val;<br>+<br>+ memcpy( &val, ptr, sizeof( val ) );<br>+ if ( endian == G_BIG_ENDIAN )<br>+ {<br>+ #ifndef WORDS_BIGENDIAN<br>+ val = bswap32( val );<br>+ #endif<br>+ }<br>+ else<br>+ {<br>+ #ifdef WORDS_BIGENDIAN<br>+ val = bswap32( val );<br>+ #endif<br>+ }<br>+ return val;<br>+}<br>+<br>+/*<br>+ * Look through the meta data in the libjpeg decompress structure to determine<br>+ * if an EXIF Orientation tag is present. If so return its value (1-8),<br>+ * otherwise return 0<br>+ *<br>+ * This function is based on the function get_orientation in io-jpeg.c, part of<br>+ * the GdkPixbuf library, licensed under LGPLv2+.<br>+ * Copyright (C) 1999 Michael Zucchi, The Free Software Foundation<br>+*/<br>+LOCAL( int )<br>+jpeg_GetOrientation( j_decompress_ptr cinfo )<br>+{<br>+<br>+ uint i; /* index into working buffer */<br>+ ushort tag_type; /* endianed tag type extracted from tiff header */<br>+ uint ret; /* Return value */<br>+ uint offset; /* de-endianed offset in various situations */<br>+ uint tags; /* number of tags in current ifd */<br>+ uint type; /* de-endianed type of tag */<br>+ uint count; /* de-endianed count of elements in a tag */<br>+ uint tiff = 0; /* offset to active tiff header */<br>+ uint endian = 0; /* detected endian of data */<br>+<br>+ jpeg_saved_marker_ptr exif_marker; /* Location of the Exif APP1 marker */<br>+ jpeg_saved_marker_ptr cmarker; /* Location to check for Exif APP1 marker */<br>+<br>+ const char leth[] = { 0x49, 0x49, 0x2a, 0x00 }; /* Little endian TIFF header */<br>+ const char beth[] = { 0x4d, 0x4d, 0x00, 0x2a }; /* Big endian TIFF header */<br>+<br>+ #define EXIF_JPEG_MARKER 0xE1<br>+ #define EXIF_IDENT_STRING "Exif\000\000"<br>+ #define EXIF_ORIENT_TAGID 0x112<br>+<br>+ /* check for Exif marker (also called the APP1 marker) */<br>+ exif_marker = NULL;<br>+ cmarker = cinfo->marker_list;<br>+<br>+ while ( cmarker )<br>+ {<br>+ if ( cmarker->marker == EXIF_JPEG_MARKER )<br>+ {<br>+ /* The Exif APP1 marker should contain a unique<br>+ identification string ("Exif\0\0"). Check for it. */<br>+ if ( !memcmp( cmarker->data, EXIF_IDENT_STRING, 6 ) )<br>+ {<br>+ exif_marker = cmarker;<br>+ }<br>+ }<br>+ cmarker = cmarker->next;<br>+ }<br>+<br>+ /* Did we find the Exif APP1 marker? */<br>+ if ( exif_marker == NULL )<br>+ return 0;<br>+<br>+ /* Do we have enough data? */<br>+ if ( exif_marker->data_length < 32 )<br>+ return 0;<br>+<br>+ /* Check for TIFF header and catch endianess */<br>+ i = 0;<br>+<br>+ /* Just skip data until TIFF header - it should be within 16 bytes from marker start.<br>+ Normal structure relative to APP1 marker -<br>+ 0x0000: APP1 marker entry = 2 bytes<br>+ 0x0002: APP1 length entry = 2 bytes<br>+ 0x0004: Exif Identifier entry = 6 bytes<br>+ 0x000A: Start of TIFF header (Byte order entry) - 4 bytes<br>+ - This is what we look for, to determine endianess.<br>+ 0x000E: 0th IFD offset pointer - 4 bytes<br>+<br>+ exif_marker->data points to the first data after the APP1 marker<br>+ and length entries, which is the exif identification string.<br>+ The TIFF header should thus normally be found at i=6, below,<br>+ and the pointer to IFD0 will be at 6+4 = 10.<br>+ */<br>+<br>+ while ( i < 16 )<br>+ {<br>+ /* Little endian TIFF header */<br>+ if ( memcmp( &exif_marker->data[i], leth, 4 ) == 0 )<br>+ {<br>+ endian = G_LITTLE_ENDIAN;<br>+ }<br>+ /* Big endian TIFF header */<br>+ else<br>+ if ( memcmp( &exif_marker->data[i], beth, 4 ) == 0 )<br>+ {<br>+ endian = G_BIG_ENDIAN;<br>+ }<br>+ /* Keep looking through buffer */<br>+ else<br>+ {<br>+ i++;<br>+ continue;<br>+ }<br>+ /* We have found either big or little endian TIFF header */<br>+ tiff = i;<br>+ break;<br>+ }<br>+<br>+ /* So did we find a TIFF header or did we just hit end of buffer? */<br>+ if ( tiff == 0 )<br>+ return 0;<br>+<br>+ /* Read out the offset pointer to IFD0 */<br>+ offset = de_get32( &exif_marker->data[i] + 4, endian );<br>+ i = i + offset;<br>+<br>+ /* Check that we still are within the buffer and can read the tag count */<br>+<br>+ if ( ( i + 2 ) > exif_marker->data_length )<br>+ return 0;<br>+<br>+ /* Find out how many tags we have in IFD0. As per the TIFF spec, the first<br>+ two bytes of the IFD contain a count of the number of tags. */<br>+ tags = de_get16( &exif_marker->data[i], endian );<br>+ i = i + 2;<br>+<br>+ /* Check that we still have enough data for all tags to check. The tags<br>+ are listed in consecutive 12-byte blocks. The tag ID, type, size, and<br>+ a pointer to the actual value, are packed into these 12 byte entries. */<br>+ if ( ( i + tags * 12 ) > exif_marker->data_length )<br>+ return 0;<br>+<br>+ /* Check through IFD0 for tags of interest */<br>+ while ( tags-- )<br>+ {<br>+ tag_type = de_get16( &exif_marker->data[i], endian );<br>+ /* Is this the orientation tag? */<br>+ if ( tag_type == EXIF_ORIENT_TAGID )<br>+ {<br>+ type = de_get16( &exif_marker->data[i + 2], endian );<br>+ count = de_get32( &exif_marker->data[i + 4], endian );<br>+<br>+ /* Check that type and count fields are OK. The orientation field<br>+ will consist of a single (count=1) 2-byte integer (type=3). */<br>+ if ( type != 3 || count != 1 )<br>+ return 0;<br>+<br>+ /* Return the orientation value. Within the 12-byte block, the<br>+ pointer to the actual data is at offset 8. */<br>+ ret = de_get16( &exif_marker->data[i + 8], endian );<br>+ return ( ret <= 8 ) ? ret : 0;<br>+ }<br>+ /* move the pointer to the next 12-byte tag field. */<br>+ i = i + 12;<br>+ }<br>+<br>+ return 0; /* No EXIF Orientation tag found */<br>+}<br>+<br>+/*<br> * This function must be fed with a complete compressed frame.<br> */<br> static picture_t *DecodeBlock(decoder_t *p_dec, block_t **pp_block)<br>@@ -214,6 +428,7 @@ static picture_t *DecodeBlock(decoder_t *p_dec, block_t **pp_block)<br> <br> jpeg_create_decompress(&p_sys->p_jpeg);<br> jpeg_mem_src(&p_sys->p_jpeg, p_block->p_buffer, p_block->i_buffer);<br>+ jpeg_save_markers( &p_sys->p_jpeg, EXIF_JPEG_MARKER, 0xffff );<br> jpeg_read_header(&p_sys->p_jpeg, TRUE);<br> <br> p_sys->p_jpeg.out_color_space = JCS_RGB;<br>@@ -230,6 +445,14 @@ static picture_t *DecodeBlock(decoder_t *p_dec, block_t **pp_block)<br> p_dec->fmt_out.video.i_gmask = 0x0000ff00;<br> p_dec->fmt_out.video.i_bmask = 0x00ff0000;<br> <br>+ int i_otag; /* Orientation tag has valid range of 1-8. 1 is normal orientation, 0 = unspecified = normal */<br>+ i_otag = jpeg_GetOrientation( &p_sys->p_jpeg );<br>+ if ( i_otag > 1 )<br>+ {<br>+ msg_Info( p_dec, "Orientation is %d", i_otag );<br>+ p_dec->fmt_out.video.orientation = ORIENT_FROM_EXIF( i_otag );<br>+ }<br>+<br> /* Get a new picture */<br> p_pic = decoder_NewPicture(p_dec);<br> if (!p_pic)<br>-- <br>2.7.4<br><br></div></div></div>