[FFmpeg-devel] [PATCH v8 2/2] fftools/ffmpeg: add exif orientation support per frame's metadata

[Jun Li]

On Fri, Jun 7, 2019 at 11:54 AM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Thu, Jun 06, 2019 at 08:28:15PM -0700, Jun Li wrote:
> > Fix #6945
> > Rotate or/and flip frame according to frame's metadata orientation
> > ---
> >  fftools/ffmpeg.c        |  5 +++--
> >  fftools/ffmpeg.h        |  8 ++++++++
> >  fftools/ffmpeg_filter.c | 36 +++++++++++++++++++++++++++++++-----
> >  3 files changed, 42 insertions(+), 7 deletions(-)
> >
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 01f04103cf..bc0cece59d 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -2141,8 +2141,9 @@ static int ifilter_send_frame(InputFilter
> *ifilter, AVFrame *frame)
> >                         ifilter->channel_layout != frame->channel_layout;
> >          break;
> >      case AVMEDIA_TYPE_VIDEO:
> > -        need_reinit |= ifilter->width  != frame->width ||
> > -                       ifilter->height != frame->height;
> > +        need_reinit |= ifilter->width       != frame->width ||
> > +                       ifilter->height      != frame->height ||
> > +                       ifilter->orientation !=
> get_frame_orientation(frame);
> >          break;
> >      }
> >
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index 7b6f802082..7324813ce3 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -232,6 +232,12 @@ typedef struct OptionsContext {
> >      int        nb_enc_time_bases;
> >  } OptionsContext;
> >
> > +enum OrientationType {
> > +    ORIENTATION_NONE,
> > +    ORIENTATION_AUTO_FLIP,
> > +    ORIENTATION_AUTO_TRANSPOSE
> > +};
> > +
> >  typedef struct InputFilter {
> >      AVFilterContext    *filter;
> >      struct InputStream *ist;
> > @@ -245,6 +251,7 @@ typedef struct InputFilter {
> >      int format;
> >
> >      int width, height;
> > +    enum OrientationType orientation;
> >      AVRational sample_aspect_ratio;
> >
> >      int sample_rate;
> > @@ -649,6 +656,7 @@ int init_complex_filtergraph(FilterGraph *fg);
> >  void sub2video_update(InputStream *ist, AVSubtitle *sub);
> >
> >  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame
> *frame);
> > +enum OrientationType get_frame_orientation(const AVFrame* frame);
> >
> >  int ffmpeg_parse_options(int argc, char **argv);
> >
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index 72838de1e2..790751f47f 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -743,6 +743,28 @@ static int sub2video_prepare(InputStream *ist,
> InputFilter *ifilter)
> >      return 0;
> >  }
> >
> > +enum OrientationType get_frame_orientation(const AVFrame *frame)
> > +{
> > +    AVDictionaryEntry *entry = NULL;
> > +    int orientation = 0;
> > +
> > +    // read exif orientation data
> > +    entry = av_dict_get(frame->metadata, "Orientation", NULL, 0);
> > +    if (entry && entry->value)
> > +        orientation = atoi(entry->value);
>
> what if entry->value equals "The cow says moo" ?
> This is of course a silly example but the following code would not detect
> it as an error
> as atoi would return 0
> or is there something that prevents this case ?
>

Hi Michael,
The question is like how to validate a integer number, or maybe partially
validate a number.
In this case, I can do a simple check before call atoi,  but need to cover
the following case:
  "      +2"   // start with space,  with or without sign

Something might be like:
  int i = 0;
  char *pos = NULL;
  if (entry && entry->value) {
      while(entry->value[i] && entry->value[i] == ' ') // eat white-space
           i++;
      if (entry->value[i]) {
          if (entry->value[i] == '+')   // eat '+'
             i++;

          if (entry->value[i] && entry->value[i] >= '0' && entry->value <=
'8')
              pos = entry->value + i;
      }
  }

  if (pos) {
     orientation = atoi(pos);
  }

But the validation is *NOT COMPLETE* although it covers the case your
mentioned.
Like "3.4" is valid and converted to orientation 3, even "3     4" is a
valid orientation.
I think a complete validation should be creating a function like "atodigit"
, string to digit, instead of using atoi,

If you prefer a complete solution, I can send a new version to validate the
value.
What do you think ?

Best Regards,
Jun


> > +
> > +    if (orientation > 8 || orientation < 0) {
> > +        av_log(NULL, AV_LOG_WARNING, "Invalid frame orientation: %i,
> skip it.\n", orientation);
> > +        return ORIENTATION_NONE;
> > +    } else if (orientation <= 1) {
> > +        return ORIENTATION_NONE;
> > +    } else if (orientation <= 4) {
> > +        return ORIENTATION_AUTO_FLIP;
> > +    } else {
> > +        return ORIENTATION_AUTO_TRANSPOSE;
> > +    }
> > +}
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> "Nothing to hide" only works if the folks in power share the values of
> you and everyone you know entirely and always will -- Tom Scott
>
> _______________________________________________
> 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".