[FFmpeg-devel,v1,2/2] fftools/ffmpeg Add stream metadata from first frame's metadata

Submitted by Jun Li on May 5, 2019, 2:13 a.m.

Details

Message ID 20190505021334.465-2-junli1026@gmail.com
State New
Headers show

Commit Message

Jun Li May 5, 2019, 2:13 a.m.
Fix #6945
Exif extension has 'Orientaion' field for image flip and rotation.
This change is to add the first frame's exif into stream so that
autorotation would use the info to adjust the frames.
---
 fftools/ffmpeg.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

Comments

Michael Niedermayer May 6, 2019, 12:07 a.m.
On Sat, May 04, 2019 at 07:13:34PM -0700, Jun Li wrote:
> Fix #6945
> Exif extension has 'Orientaion' field for image flip and rotation.
> This change is to add the first frame's exif into stream so that
> autorotation would use the info to adjust the frames.
> ---
>  fftools/ffmpeg.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)

This breaks converting jpegs

try this 
(file shoudl be here: https://trac.ffmpeg.org/raw-attachment/ticket/3424/bug.jpg)

./ffmpeg -i ~/tickets/3424/bug.jpg new.jpg
./ffmpeg -i ~/tickets/3424/bug.jpg new.bmp

./ffplay ~/tickets/3424/bug.jpg
./ffplay new.jpg
./ffplay new.bmp

All 3 above should produce a image with the same and correct orientation
they do before these 2 patches, they do not after

Thanks

[...]
Jun Li May 6, 2019, 3:47 a.m.
On Sun, May 5, 2019 at 6:06 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Sat, May 04, 2019 at 07:13:34PM -0700, Jun Li wrote:
> > Fix #6945
> > Exif extension has 'Orientaion' field for image flip and rotation.
> > This change is to add the first frame's exif into stream so that
> > autorotation would use the info to adjust the frames.
> > ---
> >  fftools/ffmpeg.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
>
> This breaks converting jpegs
>
> try this
> (file shoudl be here:
> https://trac.ffmpeg.org/raw-attachment/ticket/3424/bug.jpg)
>
> ./ffmpeg -i ~/tickets/3424/bug.jpg new.jpg
> ./ffmpeg -i ~/tickets/3424/bug.jpg new.bmp
>
> ./ffplay ~/tickets/3424/bug.jpg
> ./ffplay new.jpg
> ./ffplay new.bmp
>
> All 3 above should produce a image with the same and correct orientation
> they do before these 2 patches, they do not after
>
> Thanks
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Into a blind darkness they enter who follow after the Ignorance,
> they as if into a greater darkness enter who devote themselves
> to the Knowledge alone. -- Isha Upanishad
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".


Thanks Michael for review and testing !
Yes, this is a bug in the patch. I missed the case when "Orientation" is 1,
which is horizontal(normal) .
Now the issue has been addressed in v2 as follows:
https://patchwork.ffmpeg.org/patch/13000/
https://patchwork.ffmpeg.org/patch/13001/

Best Regards,
Jun

Patch hide | download patch | download mbox

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 01f04103cf..1b59542e6c 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2341,6 +2341,49 @@  static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output,
     return err < 0 ? err : ret;
 }
 
+static void set_metadata_from_1stframe(AVStream* stream, AVFrame* frame) 
+{
+    // read exif Orientation data
+    AVDictionaryEntry *entry = av_dict_get(frame->metadata, "Orientation", NULL, 0);
+    int orientation = 0;
+    uint8_t* sd = NULL;
+    if (entry) {
+        orientation = atoi(entry->value);
+        sd = av_stream_new_side_data(stream, AV_PKT_DATA_DISPLAYMATRIX, 
+                                                sizeof(int32_t) * 9);
+        switch (orientation)
+        {
+            case 2: // mirror horizontal
+                av_display_rotation_set((int32_t*)sd, 0.0);
+                av_display_matrix_flip(sd, 1, 0);
+                break;
+            case 3: // rotate 180
+                av_display_rotation_set((int32_t*)sd, 180.0);
+                break;
+            case 4: // mirror vertical
+                av_display_rotation_set((int32_t*)sd, 0.0);
+                av_display_matrix_flip(sd, 0, 1);
+                break;
+            case 5: // mirror horizontal and rotate 270 CW
+                av_display_rotation_set((int32_t*)sd, 270.0);
+                av_display_matrix_flip(sd, 0, 1);
+                break;
+            case 6: // rotate 90 CW
+                av_display_rotation_set((int32_t*)sd, 90.0);
+                break;
+            case 7: // mirror horizontal and rotate 90 CW
+                av_display_rotation_set((int32_t*)sd, 90.0);
+                av_display_matrix_flip(sd, 0, 1);
+                break;
+            case 8: // rotate 270 CW
+                av_display_rotation_set((int32_t*)sd, 270.0);
+                break;
+            default:
+                break;
+        }
+    }
+}
+
 static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_t *duration_pts, int eof,
                         int *decode_failed)
 {
@@ -2423,6 +2466,8 @@  static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_
         decoded_frame->top_field_first = ist->top_field_first;
 
     ist->frames_decoded++;
+    if (ist->frames_decoded == 1)
+        set_metadata_from_1stframe(ist->st, decoded_frame);
 
     if (ist->hwaccel_retrieve_data && decoded_frame->format == ist->hwaccel_pix_fmt) {
         err = ist->hwaccel_retrieve_data(ist->dec_ctx, decoded_frame);