[FFmpeg-devel] [PATCH v4 2/2] fftools/ffmpeg: Add stream metadata from first frame's metadata

[Jun Li]

On Tue, May 7, 2019 at 11:40 PM Gyan <ffmpeg at gyani.pro> wrote:

>
>
> On 08-05-2019 11:54 AM, 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.
>
> Suggest commit msg should be
>
> "
>
> 'Orientation' field from EXIF tags in first decoded frame is extracted
>   as stream side data so that ffmpeg can apply auto-rotation.
>
> "
>

Sure, will address that in next iteration.


>
> > ---
> >   fftools/ffmpeg.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 01f04103cf..98ccaf0732 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -2341,6 +2341,58 @@ static int decode_audio(InputStream *ist,
> AVPacket *pkt, int *got_output,
> >       return err < 0 ? err : ret;
> >   }
> >
> > +static int set_metadata_from_1stframe(InputStream* ist, AVFrame* frame)
> > +{
> > +    // read exif Orientation data
> > +    AVDictionaryEntry *entry = av_dict_get(frame->metadata,
> "Orientation", NULL, 0);
> > +    int orientation = 0;
> > +    int32_t* sd = NULL;
> > +    if (entry) {
> > +        orientation = atoi(entry->value);
> > +        sd = (int32_t*)av_stream_new_side_data(ist->st,
> AV_PKT_DATA_DISPLAYMATRIX, 4 * 9);
> > +        if (!sd)
> > +            return AVERROR(ENOMEM);
> > +        memset(sd, 0, 4 * 9);
> > +        switch (orientation)
> > +        {
> > +            case 1: // horizontal (normal)
> > +                av_display_rotation_set(sd, 0.0);
> > +                break;
> > +            case 2: // mirror horizontal
> > +                av_display_rotation_set(sd, 0.0);
> > +                av_display_matrix_flip(sd, 1, 0);
> > +                break;
> > +            case 3: // rotate 180
> > +                av_display_rotation_set(sd, 180.0);
> > +                break;
> > +            case 4: // mirror vertical
> > +                av_display_rotation_set(sd, 0.0);
> > +                av_display_matrix_flip(sd, 0, 1);
> > +                break;
> > +            case 5: // mirror horizontal and rotate 270 CW
> > +                av_display_rotation_set(sd, 270.0);
> > +                av_display_matrix_flip(sd, 0, 1);
> > +                break;
> > +            case 6: // rotate 90 CW
> > +                av_display_rotation_set(sd, 90.0);
> > +                break;
> > +            case 7: // mirror horizontal and rotate 90 CW
> > +                av_display_rotation_set(sd, 90.0);
> > +                av_display_matrix_flip(sd, 0, 1);
> > +                break;
> > +            case 8: // rotate 270 CW
> > +                av_display_rotation_set(sd, 270.0);
> > +                break;
> > +            default:
> > +                av_display_rotation_set(sd, 0.0);
> > +                av_log(ist->dec_ctx, AV_LOG_WARNING,
> > +                    "Exif orientation data out of range: %i. Set to
> default value: horizontal(normal).\n", orientation);
> > +                break;
> > +        }
> > +    }
> > +    return 0;
> > +}
> > +
> >   static int decode_video(InputStream *ist, AVPacket *pkt, int
> *got_output, int64_t *duration_pts, int eof,
> >                           int *decode_failed)
> >   {
> > @@ -2423,7 +2475,10 @@ 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 &&
> > +        ((err = set_metadata_from_1stframe(ist, decoded_frame)) < 0))
> > +        goto fail;
> > +
> >       if (ist->hwaccel_retrieve_data && decoded_frame->format ==
> ist->hwaccel_pix_fmt) {
> >           err = ist->hwaccel_retrieve_data(ist->dec_ctx, decoded_frame);
> >           if (err < 0)
>
> Also, is there a chance that there may be multiple sources for
> orientation data available for a given stream? If yes, what's the
> interaction? It looks like you append a new SD element.
>

Thanks Gyan for review !
Nicolas George asked the same question before. :)

Yes, this patch can't handle the case every frame has its own orientation.
>From a technical perspective, it is absolutely possible, for example a
motion jpeg stream with different orientation value
on every frame. I think an ideal solution for this case is a filter doing
transformation per orientation every frame.

But based on my limited experience, I think this kind of content is rare.
What do you think ?
Maybe someone in the community met this kind of content before ?

Gyan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".