diff mbox

[FFmpeg-devel,v2,2/2] fftools/ffmpeg: add support for per frame rotation and flip

Message ID 20190515053645.21432-2-junli1026@gmail.com
State Superseded
Headers show

Commit Message

Jun Li May 15, 2019, 5:36 a.m. UTC 
Fix #6945
Current implementaion for autorotate works fine for stream
level rotataion but no support for frame level operation
and frame flip. This patch is for adding flip support and
per frame operations.
---
 fftools/cmdutils.c      |  9 ++---
 fftools/cmdutils.h      |  2 +-
 fftools/ffmpeg.c        | 21 ++++++++++-
 fftools/ffmpeg.h        |  2 +
 fftools/ffmpeg_filter.c | 81 ++++++++++++++++++++++++++++++++++++++---
 fftools/ffplay.c        | 28 +++++++++++---
 6 files changed, 126 insertions(+), 17 deletions(-)

Comments

Andriy Gelman May 15, 2019, 12:12 p.m. UTC | #1 
On Tue, 14. May 22:36, Jun Li wrote:
> Fix #6945
> Current implementaion for autorotate works fine for stream
> level rotataion but no support for frame level operation
> and frame flip. This patch is for adding flip support and
> per frame operations.
> ---
>  fftools/cmdutils.c      |  9 ++---
>  fftools/cmdutils.h      |  2 +-
>  fftools/ffmpeg.c        | 21 ++++++++++-
>  fftools/ffmpeg.h        |  2 +
>  fftools/ffmpeg_filter.c | 81 ++++++++++++++++++++++++++++++++++++++---
>  fftools/ffplay.c        | 28 +++++++++++---
>  6 files changed, 126 insertions(+), 17 deletions(-)
> 
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 9cfbc45c2b..b8bb904419 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -2172,13 +2172,12 @@ void *grow_array(void *array, int elem_size, int *size, int new_size)
>      return array;
>  }
>  
> -double get_rotation(AVStream *st)
> +double get_rotation_hflip(int32_t display_matrix[9], int* hflip)
>  {
> -    uint8_t* displaymatrix = av_stream_get_side_data(st,
> -                                                     AV_PKT_DATA_DISPLAYMATRIX, NULL);
>      double theta = 0;
> -    if (displaymatrix)
> -        theta = -av_display_rotation_get((int32_t*) displaymatrix);
> +
> +    if (display_matrix)
> +        theta = -av_display_rotation_hflip_get(display_matrix, hflip);
>  
>      theta -= 360*floor(theta/360 + 0.9/360);
>  
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index 6e2e0a2acb..dce44ed6e1 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -643,6 +643,6 @@ void *grow_array(void *array, int elem_size, int *size, int new_size);
>      char name[128];\
>      av_get_channel_layout_string(name, sizeof(name), 0, ch_layout);
>  
> -double get_rotation(AVStream *st);
> +double get_rotation_hflip(int32_t display_matrix[9], int* hflip);
>  
>  #endif /* FFTOOLS_CMDUTILS_H */
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 01f04103cf..9ea1aaa930 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -2126,6 +2126,24 @@ static int ifilter_has_all_input_formats(FilterGraph *fg)
>      return 1;
>  }
>  
> +static int ifilter_display_matrix_need_from_frame(InputFilter *ifilter, AVFrame* frame)
> +{
> +    int32_t* stream_matrix = (int32_t*)av_stream_get_side_data(ifilter->ist->st, 
> +                                                AV_PKT_DATA_DISPLAYMATRIX, NULL);
> +    // if we already have display matrix from stream, use it instead of extracting
> +    // from frame.
> +    if (stream_matrix) {
> +        memcpy(ifilter->display_matrix, stream_matrix, 4 * 9);

would it be better to use sizeof? 
memcpy(ifilter->display_matrix, stream_matrix, sizeof(int32_t) * 9); 

> +        return 0;
> +    }
> +
> +    // cleanup the display matrix, may be from last frame
> +    memset(ifilter->display_matrix, 0, 4 * 9);

memset(ifilter->display_matrix, 0, sizeof(int32_t) * 9);

> +    av_display_rotation_set(ifilter->display_matrix, 0);
> +
> +    return !!av_dict_get(frame->metadata, "Orientation", NULL, 0);
> +}
> +
>  static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame)
>  {
>      FilterGraph *fg = ifilter->graph;
> @@ -2141,7 +2159,8 @@ 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 ||
> +        need_reinit |= ifilter_display_matrix_need_from_frame(ifilter, frame) ||
> +                       ifilter->width  != frame->width ||
>                         ifilter->height != frame->height;
>          break;
>      }
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index eb1eaf6363..8331720663 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -251,6 +251,8 @@ typedef struct InputFilter {
>      int channels;
>      uint64_t channel_layout;
>  
> +    int32_t display_matrix[9];
> +    
>      AVBufferRef *hw_frames_ctx;
>  
>      int eof;
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 72838de1e2..1894f30561 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -328,6 +328,7 @@ static void init_input_filter(FilterGraph *fg, AVFilterInOut *in)
>      fg->inputs[fg->nb_inputs - 1]->format = -1;
>      fg->inputs[fg->nb_inputs - 1]->type = ist->st->codecpar->codec_type;
>      fg->inputs[fg->nb_inputs - 1]->name = describe_filter_link(fg, in, 1);
> +    av_display_rotation_set(fg->inputs[fg->nb_inputs - 1]->display_matrix, 0);
>  
>      fg->inputs[fg->nb_inputs - 1]->frame_queue = av_fifo_alloc(8 * sizeof(AVFrame*));
>      if (!fg->inputs[fg->nb_inputs - 1]->frame_queue)
> @@ -807,22 +808,43 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
>      last_filter = ifilter->filter;
>  
>      if (ist->autorotate) {
> -        double theta = get_rotation(ist->st);
> +        int hflip = 0;
> +        double theta = get_rotation_hflip(ifilter->display_matrix, &hflip);
>  
> -        if (fabs(theta - 90) < 1.0) {
> +        if (fabs(theta) < 1.0) {
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> +        } else if (fabs(theta - 90) < 1.0) {
>              ret = insert_filter(&last_filter, &pad_idx, "transpose", "clock");
> -        } else if (fabs(theta - 180) < 1.0) {
> -            ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
>              if (ret < 0)
>                  return ret;
> -            ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> +        } else if (fabs(theta - 180) < 1.0) {
> +            if (hflip) { // rotate 180 and hflip equals vflip
> +                ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> +            } else {
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> +                if (ret < 0)
> +                    return ret;
> +                ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> +            }
>          } else if (fabs(theta - 270) < 1.0) {
>              ret = insert_filter(&last_filter, &pad_idx, "transpose", "cclock");
> +            if (ret < 0)
> +                return ret;
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
>          } else if (fabs(theta) > 1.0) {
>              char rotate_buf[64];
>              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
>              ret = insert_filter(&last_filter, &pad_idx, "rotate", rotate_buf);
> +            if (ret < 0)
> +                return ret;
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
>          }
> +
>          if (ret < 0)
>              return ret;
>      }
> @@ -1182,6 +1204,53 @@ fail:
>      return ret;
>  }
>  
> +static void set_display_matrix_from_frame(const AVFrame *frame, int32_t m[9])
> +{
> +    AVDictionaryEntry *entry = NULL;
> +    int orientation = 0;
> +    
> +    // read exif orientation data
> +    entry = av_dict_get(frame->metadata, "Orientation", NULL, 0);
> +    if (entry) {
> +        orientation = atoi(entry->value);
> +        memset(m, 0, 4 * 9);
> +        switch (orientation)
> +        {
> +            case 1: // horizontal (normal)
> +                av_display_rotation_set(m, 0.0);
> +                break;
> +            case 2: // mirror horizontal
> +                av_display_rotation_set(m, 0.0);
> +                av_display_matrix_flip(m, 1, 0);
> +                break;
> +            case 3: // rotate 180
> +                av_display_rotation_set(m, 180.0);
> +                break;
> +            case 4: // mirror vertical
> +                av_display_rotation_set(m, 0.0);
> +                av_display_matrix_flip(m, 0, 1);
> +                break;
> +            case 5: // mirror horizontal and rotate 270 CW
> +                av_display_rotation_set(m, 270.0);
> +                av_display_matrix_flip(m, 0, 1);
> +                break;
> +            case 6: // rotate 90 CW
> +                av_display_rotation_set(m, 90.0);
> +                break;
> +            case 7: // mirror horizontal and rotate 90 CW
> +                av_display_rotation_set(m, 90.0);
> +                av_display_matrix_flip(m, 0, 1);
> +                break;
> +            case 8: // rotate 270 CW
> +                av_display_rotation_set(m, 270.0);
> +                break;
> +            default:
> +                av_display_rotation_set(m, 0.0);
> +                break;
> +        }
> +    }
> +}
> +
>  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
>  {
>      av_buffer_unref(&ifilter->hw_frames_ctx);
> @@ -1202,6 +1271,8 @@ int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
>              return AVERROR(ENOMEM);
>      }
>  
> +    set_display_matrix_from_frame(frame, ifilter->display_matrix);
> +
>      return 0;
>  }
>  
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 8f050e16e6..717a9a830c 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1914,19 +1914,37 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>  } while (0)
>  
>      if (autorotate) {
> -        double theta  = get_rotation(is->video_st);
> -
> -        if (fabs(theta - 90) < 1.0) {
> +        int hflip = 0;
> +        double theta = 0.0;
> +        int32_t* display_matrix = (int32_t*)av_stream_get_side_data(is->video_st, 
> +                                                AV_PKT_DATA_DISPLAYMATRIX, NULL);
> +        if (display_matrix)
> +             theta  = get_rotation_hflip(display_matrix, &hflip);
> +
> +        if (fabs(theta) < 1.0) {
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
> +        } else if (fabs(theta - 90) < 1.0) {
>              INSERT_FILT("transpose", "clock");
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
>          } else if (fabs(theta - 180) < 1.0) {
> -            INSERT_FILT("hflip", NULL);
> -            INSERT_FILT("vflip", NULL);
> +            if (hflip) { // rotate 180 and hflip equals vflip
> +                INSERT_FILT("vflip", NULL);
> +            } else {
> +                INSERT_FILT("hflip", NULL);
> +                INSERT_FILT("vflip", NULL);
> +            }
>          } else if (fabs(theta - 270) < 1.0) {
>              INSERT_FILT("transpose", "cclock");
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
>          } else if (fabs(theta) > 1.0) {
>              char rotate_buf[64];
>              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
>              INSERT_FILT("rotate", rotate_buf);
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
>          }
>      }
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> 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".
Andriy Gelman May 15, 2019, 6:45 p.m. UTC | #2 
On Tue, 14. May 22:36, Jun Li wrote:
> Fix #6945
> Current implementaion for autorotate works fine for stream
> level rotataion but no support for frame level operation
> and frame flip. This patch is for adding flip support and
> per frame operations.
> ---
>  fftools/cmdutils.c      |  9 ++---
>  fftools/cmdutils.h      |  2 +-
>  fftools/ffmpeg.c        | 21 ++++++++++-
>  fftools/ffmpeg.h        |  2 +
>  fftools/ffmpeg_filter.c | 81 ++++++++++++++++++++++++++++++++++++++---
>  fftools/ffplay.c        | 28 +++++++++++---
>  6 files changed, 126 insertions(+), 17 deletions(-)
> 
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 9cfbc45c2b..b8bb904419 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -2172,13 +2172,12 @@ void *grow_array(void *array, int elem_size, int *size, int new_size)
>      return array;
>  }
>  
> -double get_rotation(AVStream *st)
> +double get_rotation_hflip(int32_t display_matrix[9], int* hflip)
>  {
> -    uint8_t* displaymatrix = av_stream_get_side_data(st,
> -                                                     AV_PKT_DATA_DISPLAYMATRIX, NULL);
>      double theta = 0;
> -    if (displaymatrix)
> -        theta = -av_display_rotation_get((int32_t*) displaymatrix);
> +
> +    if (display_matrix)
> +        theta = -av_display_rotation_hflip_get(display_matrix, hflip);
>  
>      theta -= 360*floor(theta/360 + 0.9/360);
>  
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index 6e2e0a2acb..dce44ed6e1 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -643,6 +643,6 @@ void *grow_array(void *array, int elem_size, int *size, int new_size);
>      char name[128];\
>      av_get_channel_layout_string(name, sizeof(name), 0, ch_layout);
>  
> -double get_rotation(AVStream *st);
> +double get_rotation_hflip(int32_t display_matrix[9], int* hflip);
>  
>  #endif /* FFTOOLS_CMDUTILS_H */
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 01f04103cf..9ea1aaa930 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -2126,6 +2126,24 @@ static int ifilter_has_all_input_formats(FilterGraph *fg)
>      return 1;
>  }
>  
> +static int ifilter_display_matrix_need_from_frame(InputFilter *ifilter, AVFrame* frame)
> +{
> +    int32_t* stream_matrix = (int32_t*)av_stream_get_side_data(ifilter->ist->st, 
> +                                                AV_PKT_DATA_DISPLAYMATRIX, NULL);
> +    // if we already have display matrix from stream, use it instead of extracting
> +    // from frame.
> +    if (stream_matrix) {
> +        memcpy(ifilter->display_matrix, stream_matrix, 4 * 9);
> +        return 0;
> +    }
> +
> +    // cleanup the display matrix, may be from last frame
> +    memset(ifilter->display_matrix, 0, 4 * 9);
> +    av_display_rotation_set(ifilter->display_matrix, 0);
> +
> +    return !!av_dict_get(frame->metadata, "Orientation", NULL, 0);
> +}
> +
>  static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame)
>  {
>      FilterGraph *fg = ifilter->graph;
> @@ -2141,7 +2159,8 @@ 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 ||
> +        need_reinit |= ifilter_display_matrix_need_from_frame(ifilter, frame) ||
> +                       ifilter->width  != frame->width ||
>                         ifilter->height != frame->height;
>          break;
>      }
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index eb1eaf6363..8331720663 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -251,6 +251,8 @@ typedef struct InputFilter {
>      int channels;
>      uint64_t channel_layout;
>  
> +    int32_t display_matrix[9];
> +    
>      AVBufferRef *hw_frames_ctx;
>  
>      int eof;
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 72838de1e2..1894f30561 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -328,6 +328,7 @@ static void init_input_filter(FilterGraph *fg, AVFilterInOut *in)
>      fg->inputs[fg->nb_inputs - 1]->format = -1;
>      fg->inputs[fg->nb_inputs - 1]->type = ist->st->codecpar->codec_type;
>      fg->inputs[fg->nb_inputs - 1]->name = describe_filter_link(fg, in, 1);
> +    av_display_rotation_set(fg->inputs[fg->nb_inputs - 1]->display_matrix, 0);
>  
>      fg->inputs[fg->nb_inputs - 1]->frame_queue = av_fifo_alloc(8 * sizeof(AVFrame*));
>      if (!fg->inputs[fg->nb_inputs - 1]->frame_queue)
> @@ -807,22 +808,43 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
>      last_filter = ifilter->filter;
>  
>      if (ist->autorotate) {
> -        double theta = get_rotation(ist->st);
> +        int hflip = 0;
> +        double theta = get_rotation_hflip(ifilter->display_matrix, &hflip);
>  
> -        if (fabs(theta - 90) < 1.0) {
> +        if (fabs(theta) < 1.0) {
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> +        } else if (fabs(theta - 90) < 1.0) {
>              ret = insert_filter(&last_filter, &pad_idx, "transpose", "clock");
> -        } else if (fabs(theta - 180) < 1.0) {
> -            ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
>              if (ret < 0)
>                  return ret;
> -            ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> +        } else if (fabs(theta - 180) < 1.0) {
> +            if (hflip) { // rotate 180 and hflip equals vflip
> +                ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> +            } else {
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> +                if (ret < 0)
> +                    return ret;
> +                ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> +            }
>          } else if (fabs(theta - 270) < 1.0) {
>              ret = insert_filter(&last_filter, &pad_idx, "transpose", "cclock");
> +            if (ret < 0)
> +                return ret;
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
>          } else if (fabs(theta) > 1.0) {
>              char rotate_buf[64];
>              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
>              ret = insert_filter(&last_filter, &pad_idx, "rotate", rotate_buf);
> +            if (ret < 0)
> +                return ret;
> +            if (hflip)
> +                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
>          }

Why do you need the complicated logic that uses horizontal
flip and theta? 
Looking at the code before, it looks that all the cases were handled using only theta. 

> +
>          if (ret < 0)
>              return ret;
>      }
> @@ -1182,6 +1204,53 @@ fail:
>      return ret;
>  }
>  
> +static void set_display_matrix_from_frame(const AVFrame *frame, int32_t m[9])
> +{
> +    AVDictionaryEntry *entry = NULL;
> +    int orientation = 0;
> +    
> +    // read exif orientation data
> +    entry = av_dict_get(frame->metadata, "Orientation", NULL, 0);
> +    if (entry) {
> +        orientation = atoi(entry->value);
> +        memset(m, 0, 4 * 9);
> +        switch (orientation)
> +        {
> +            case 1: // horizontal (normal)
> +                av_display_rotation_set(m, 0.0);
> +                break;
> +            case 2: // mirror horizontal
> +                av_display_rotation_set(m, 0.0);
> +                av_display_matrix_flip(m, 1, 0);
> +                break;
> +            case 3: // rotate 180
> +                av_display_rotation_set(m, 180.0);
> +                break;
> +            case 4: // mirror vertical
> +                av_display_rotation_set(m, 0.0);
> +                av_display_matrix_flip(m, 0, 1);
> +                break;
> +            case 5: // mirror horizontal and rotate 270 CW
> +                av_display_rotation_set(m, 270.0);
> +                av_display_matrix_flip(m, 0, 1);
> +                break;
> +            case 6: // rotate 90 CW
> +                av_display_rotation_set(m, 90.0);
> +                break;
> +            case 7: // mirror horizontal and rotate 90 CW
> +                av_display_rotation_set(m, 90.0);
> +                av_display_matrix_flip(m, 0, 1);
> +                break;
> +            case 8: // rotate 270 CW
> +                av_display_rotation_set(m, 270.0);
> +                break;
> +            default:
> +                av_display_rotation_set(m, 0.0);
> +                break;
> +        }
> +    }
> +}
> +
>  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
>  {
>      av_buffer_unref(&ifilter->hw_frames_ctx);
> @@ -1202,6 +1271,8 @@ int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
>              return AVERROR(ENOMEM);
>      }
>  
> +    set_display_matrix_from_frame(frame, ifilter->display_matrix);
> +
>      return 0;
>  }
>  
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 8f050e16e6..717a9a830c 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1914,19 +1914,37 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>  } while (0)
>  
>      if (autorotate) {
> -        double theta  = get_rotation(is->video_st);
> -
> -        if (fabs(theta - 90) < 1.0) {
> +        int hflip = 0;
> +        double theta = 0.0;
> +        int32_t* display_matrix = (int32_t*)av_stream_get_side_data(is->video_st, 
> +                                                AV_PKT_DATA_DISPLAYMATRIX, NULL);
> +        if (display_matrix)
> +             theta  = get_rotation_hflip(display_matrix, &hflip);
> +
> +        if (fabs(theta) < 1.0) {
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
> +        } else if (fabs(theta - 90) < 1.0) {
>              INSERT_FILT("transpose", "clock");
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
>          } else if (fabs(theta - 180) < 1.0) {
> -            INSERT_FILT("hflip", NULL);
> -            INSERT_FILT("vflip", NULL);
> +            if (hflip) { // rotate 180 and hflip equals vflip
> +                INSERT_FILT("vflip", NULL);
> +            } else {
> +                INSERT_FILT("hflip", NULL);
> +                INSERT_FILT("vflip", NULL);
> +            }
>          } else if (fabs(theta - 270) < 1.0) {
>              INSERT_FILT("transpose", "cclock");
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
>          } else if (fabs(theta) > 1.0) {
>              char rotate_buf[64];
>              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
>              INSERT_FILT("rotate", rotate_buf);
> +            if (hflip)
> +                INSERT_FILT("hflip", NULL);
>          }
>      }
Same question about theta and hflip. 

>  
> -- 
> 2.17.1
> 
> _______________________________________________
> 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,
Andriy
Jun Li May 15, 2019, 6:48 p.m. UTC | #3 
On Wed, May 15, 2019 at 5:18 AM Andriy Gelman <andriy.gelman@gmail.com>
wrote:

> On Tue, 14. May 22:36, Jun Li wrote:
> > Fix #6945
> > Current implementaion for autorotate works fine for stream
> > level rotataion but no support for frame level operation
> > and frame flip. This patch is for adding flip support and
> > per frame operations.
> > ---
> >  fftools/cmdutils.c      |  9 ++---
> >  fftools/cmdutils.h      |  2 +-
> >  fftools/ffmpeg.c        | 21 ++++++++++-
> >  fftools/ffmpeg.h        |  2 +
> >  fftools/ffmpeg_filter.c | 81 ++++++++++++++++++++++++++++++++++++++---
> >  fftools/ffplay.c        | 28 +++++++++++---
> >  6 files changed, 126 insertions(+), 17 deletions(-)
> >
> > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> > index 9cfbc45c2b..b8bb904419 100644
> > --- a/fftools/cmdutils.c
> > +++ b/fftools/cmdutils.c
> > @@ -2172,13 +2172,12 @@ void *grow_array(void *array, int elem_size, int
> *size, int new_size)
> >      return array;
> >  }
> >
> > -double get_rotation(AVStream *st)
> > +double get_rotation_hflip(int32_t display_matrix[9], int* hflip)
> >  {
> > -    uint8_t* displaymatrix = av_stream_get_side_data(st,
> > -
>  AV_PKT_DATA_DISPLAYMATRIX, NULL);
> >      double theta = 0;
> > -    if (displaymatrix)
> > -        theta = -av_display_rotation_get((int32_t*) displaymatrix);
> > +
> > +    if (display_matrix)
> > +        theta = -av_display_rotation_hflip_get(display_matrix, hflip);
> >
> >      theta -= 360*floor(theta/360 + 0.9/360);
> >
> > diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> > index 6e2e0a2acb..dce44ed6e1 100644
> > --- a/fftools/cmdutils.h
> > +++ b/fftools/cmdutils.h
> > @@ -643,6 +643,6 @@ void *grow_array(void *array, int elem_size, int
> *size, int new_size);
> >      char name[128];\
> >      av_get_channel_layout_string(name, sizeof(name), 0, ch_layout);
> >
> > -double get_rotation(AVStream *st);
> > +double get_rotation_hflip(int32_t display_matrix[9], int* hflip);
> >
> >  #endif /* FFTOOLS_CMDUTILS_H */
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 01f04103cf..9ea1aaa930 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -2126,6 +2126,24 @@ static int
> ifilter_has_all_input_formats(FilterGraph *fg)
> >      return 1;
> >  }
> >
> > +static int ifilter_display_matrix_need_from_frame(InputFilter *ifilter,
> AVFrame* frame)
> > +{
> > +    int32_t* stream_matrix =
> (int32_t*)av_stream_get_side_data(ifilter->ist->st,
> > +
> AV_PKT_DATA_DISPLAYMATRIX, NULL);
> > +    // if we already have display matrix from stream, use it instead of
> extracting
> > +    // from frame.
> > +    if (stream_matrix) {
> > +        memcpy(ifilter->display_matrix, stream_matrix, 4 * 9);
>
> would it be better to use sizeof?
> memcpy(ifilter->display_matrix, stream_matrix, sizeof(int32_t) * 9);
>

Hi Andriy,
Thanks for the review. I was suggested to use constant instead of
sizeof(type) in previous patch:
https://patchwork.ffmpeg.org/patch/13000/
I cannot think of any case that sizeof(int32_t) is not '4', so they mean
the same for me from code correctness perspective.
Of course from coding habit or design perspective, there might have some
different opinions and discussion.
So I am going to keep the constant '4' here unless it is not a correct
value in any case.

Thanks,
-Jun


> > +        return 0;
> > +    }
> > +
> > +    // cleanup the display matrix, may be from last frame
> > +    memset(ifilter->display_matrix, 0, 4 * 9);
>
> memset(ifilter->display_matrix, 0, sizeof(int32_t) * 9);
>
> > +    av_display_rotation_set(ifilter->display_matrix, 0);
> > +
> > +    return !!av_dict_get(frame->metadata, "Orientation", NULL, 0);
> > +}
> > +
> >  static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame)
> >  {
> >      FilterGraph *fg = ifilter->graph;
> > @@ -2141,7 +2159,8 @@ 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 ||
> > +        need_reinit |= ifilter_display_matrix_need_from_frame(ifilter,
> frame) ||
> > +                       ifilter->width  != frame->width ||
> >                         ifilter->height != frame->height;
> >          break;
> >      }
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index eb1eaf6363..8331720663 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -251,6 +251,8 @@ typedef struct InputFilter {
> >      int channels;
> >      uint64_t channel_layout;
> >
> > +    int32_t display_matrix[9];
> > +
> >      AVBufferRef *hw_frames_ctx;
> >
> >      int eof;
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index 72838de1e2..1894f30561 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -328,6 +328,7 @@ static void init_input_filter(FilterGraph *fg,
> AVFilterInOut *in)
> >      fg->inputs[fg->nb_inputs - 1]->format = -1;
> >      fg->inputs[fg->nb_inputs - 1]->type = ist->st->codecpar->codec_type;
> >      fg->inputs[fg->nb_inputs - 1]->name = describe_filter_link(fg, in,
> 1);
> > +    av_display_rotation_set(fg->inputs[fg->nb_inputs -
> 1]->display_matrix, 0);
> >
> >      fg->inputs[fg->nb_inputs - 1]->frame_queue = av_fifo_alloc(8 *
> sizeof(AVFrame*));
> >      if (!fg->inputs[fg->nb_inputs - 1]->frame_queue)
> > @@ -807,22 +808,43 @@ static int
> configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
> >      last_filter = ifilter->filter;
> >
> >      if (ist->autorotate) {
> > -        double theta = get_rotation(ist->st);
> > +        int hflip = 0;
> > +        double theta = get_rotation_hflip(ifilter->display_matrix,
> &hflip);
> >
> > -        if (fabs(theta - 90) < 1.0) {
> > +        if (fabs(theta) < 1.0) {
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> > +        } else if (fabs(theta - 90) < 1.0) {
> >              ret = insert_filter(&last_filter, &pad_idx, "transpose",
> "clock");
> > -        } else if (fabs(theta - 180) < 1.0) {
> > -            ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> >              if (ret < 0)
> >                  return ret;
> > -            ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> > +        } else if (fabs(theta - 180) < 1.0) {
> > +            if (hflip) { // rotate 180 and hflip equals vflip
> > +                ret = insert_filter(&last_filter, &pad_idx, "vflip",
> NULL);
> > +            } else {
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> > +                if (ret < 0)
> > +                    return ret;
> > +                ret = insert_filter(&last_filter, &pad_idx, "vflip",
> NULL);
> > +            }
> >          } else if (fabs(theta - 270) < 1.0) {
> >              ret = insert_filter(&last_filter, &pad_idx, "transpose",
> "cclock");
> > +            if (ret < 0)
> > +                return ret;
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> >          } else if (fabs(theta) > 1.0) {
> >              char rotate_buf[64];
> >              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180",
> theta);
> >              ret = insert_filter(&last_filter, &pad_idx, "rotate",
> rotate_buf);
> > +            if (ret < 0)
> > +                return ret;
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> >          }
> > +
> >          if (ret < 0)
> >              return ret;
> >      }
> > @@ -1182,6 +1204,53 @@ fail:
> >      return ret;
> >  }
> >
> > +static void set_display_matrix_from_frame(const AVFrame *frame, int32_t
> m[9])
> > +{
> > +    AVDictionaryEntry *entry = NULL;
> > +    int orientation = 0;
> > +
> > +    // read exif orientation data
> > +    entry = av_dict_get(frame->metadata, "Orientation", NULL, 0);
> > +    if (entry) {
> > +        orientation = atoi(entry->value);
> > +        memset(m, 0, 4 * 9);
> > +        switch (orientation)
> > +        {
> > +            case 1: // horizontal (normal)
> > +                av_display_rotation_set(m, 0.0);
> > +                break;
> > +            case 2: // mirror horizontal
> > +                av_display_rotation_set(m, 0.0);
> > +                av_display_matrix_flip(m, 1, 0);
> > +                break;
> > +            case 3: // rotate 180
> > +                av_display_rotation_set(m, 180.0);
> > +                break;
> > +            case 4: // mirror vertical
> > +                av_display_rotation_set(m, 0.0);
> > +                av_display_matrix_flip(m, 0, 1);
> > +                break;
> > +            case 5: // mirror horizontal and rotate 270 CW
> > +                av_display_rotation_set(m, 270.0);
> > +                av_display_matrix_flip(m, 0, 1);
> > +                break;
> > +            case 6: // rotate 90 CW
> > +                av_display_rotation_set(m, 90.0);
> > +                break;
> > +            case 7: // mirror horizontal and rotate 90 CW
> > +                av_display_rotation_set(m, 90.0);
> > +                av_display_matrix_flip(m, 0, 1);
> > +                break;
> > +            case 8: // rotate 270 CW
> > +                av_display_rotation_set(m, 270.0);
> > +                break;
> > +            default:
> > +                av_display_rotation_set(m, 0.0);
> > +                break;
> > +        }
> > +    }
> > +}
> > +
> >  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame
> *frame)
> >  {
> >      av_buffer_unref(&ifilter->hw_frames_ctx);
> > @@ -1202,6 +1271,8 @@ int ifilter_parameters_from_frame(InputFilter
> *ifilter, const AVFrame *frame)
> >              return AVERROR(ENOMEM);
> >      }
> >
> > +    set_display_matrix_from_frame(frame, ifilter->display_matrix);
> > +
> >      return 0;
> >  }
> >
> > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > index 8f050e16e6..717a9a830c 100644
> > --- a/fftools/ffplay.c
> > +++ b/fftools/ffplay.c
> > @@ -1914,19 +1914,37 @@ static int configure_video_filters(AVFilterGraph
> *graph, VideoState *is, const c
> >  } while (0)
> >
> >      if (autorotate) {
> > -        double theta  = get_rotation(is->video_st);
> > -
> > -        if (fabs(theta - 90) < 1.0) {
> > +        int hflip = 0;
> > +        double theta = 0.0;
> > +        int32_t* display_matrix =
> (int32_t*)av_stream_get_side_data(is->video_st,
> > +
> AV_PKT_DATA_DISPLAYMATRIX, NULL);
> > +        if (display_matrix)
> > +             theta  = get_rotation_hflip(display_matrix, &hflip);
> > +
> > +        if (fabs(theta) < 1.0) {
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> > +        } else if (fabs(theta - 90) < 1.0) {
> >              INSERT_FILT("transpose", "clock");
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> >          } else if (fabs(theta - 180) < 1.0) {
> > -            INSERT_FILT("hflip", NULL);
> > -            INSERT_FILT("vflip", NULL);
> > +            if (hflip) { // rotate 180 and hflip equals vflip
> > +                INSERT_FILT("vflip", NULL);
> > +            } else {
> > +                INSERT_FILT("hflip", NULL);
> > +                INSERT_FILT("vflip", NULL);
> > +            }
> >          } else if (fabs(theta - 270) < 1.0) {
> >              INSERT_FILT("transpose", "cclock");
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> >          } else if (fabs(theta) > 1.0) {
> >              char rotate_buf[64];
> >              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180",
> theta);
> >              INSERT_FILT("rotate", rotate_buf);
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> >          }
> >      }
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > 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".
> _______________________________________________
> 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".
Jun Li May 15, 2019, 6:57 p.m. UTC | #4 
On Wed, May 15, 2019 at 11:45 AM Andriy Gelman <andriy.gelman@gmail.com>
wrote:

> On Tue, 14. May 22:36, Jun Li wrote:
> > Fix #6945
> > Current implementaion for autorotate works fine for stream
> > level rotataion but no support for frame level operation
> > and frame flip. This patch is for adding flip support and
> > per frame operations.
> > ---
> >  fftools/cmdutils.c      |  9 ++---
> >  fftools/cmdutils.h      |  2 +-
> >  fftools/ffmpeg.c        | 21 ++++++++++-
> >  fftools/ffmpeg.h        |  2 +
> >  fftools/ffmpeg_filter.c | 81 ++++++++++++++++++++++++++++++++++++++---
> >  fftools/ffplay.c        | 28 +++++++++++---
> >  6 files changed, 126 insertions(+), 17 deletions(-)
> >
> > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> > index 9cfbc45c2b..b8bb904419 100644
> > --- a/fftools/cmdutils.c
> > +++ b/fftools/cmdutils.c
> > @@ -2172,13 +2172,12 @@ void *grow_array(void *array, int elem_size, int
> *size, int new_size)
> >      return array;
> >  }
> >
> > -double get_rotation(AVStream *st)
> > +double get_rotation_hflip(int32_t display_matrix[9], int* hflip)
> >  {
> > -    uint8_t* displaymatrix = av_stream_get_side_data(st,
> > -
>  AV_PKT_DATA_DISPLAYMATRIX, NULL);
> >      double theta = 0;
> > -    if (displaymatrix)
> > -        theta = -av_display_rotation_get((int32_t*) displaymatrix);
> > +
> > +    if (display_matrix)
> > +        theta = -av_display_rotation_hflip_get(display_matrix, hflip);
> >
> >      theta -= 360*floor(theta/360 + 0.9/360);
> >
> > diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> > index 6e2e0a2acb..dce44ed6e1 100644
> > --- a/fftools/cmdutils.h
> > +++ b/fftools/cmdutils.h
> > @@ -643,6 +643,6 @@ void *grow_array(void *array, int elem_size, int
> *size, int new_size);
> >      char name[128];\
> >      av_get_channel_layout_string(name, sizeof(name), 0, ch_layout);
> >
> > -double get_rotation(AVStream *st);
> > +double get_rotation_hflip(int32_t display_matrix[9], int* hflip);
> >
> >  #endif /* FFTOOLS_CMDUTILS_H */
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 01f04103cf..9ea1aaa930 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -2126,6 +2126,24 @@ static int
> ifilter_has_all_input_formats(FilterGraph *fg)
> >      return 1;
> >  }
> >
> > +static int ifilter_display_matrix_need_from_frame(InputFilter *ifilter,
> AVFrame* frame)
> > +{
> > +    int32_t* stream_matrix =
> (int32_t*)av_stream_get_side_data(ifilter->ist->st,
> > +
> AV_PKT_DATA_DISPLAYMATRIX, NULL);
> > +    // if we already have display matrix from stream, use it instead of
> extracting
> > +    // from frame.
> > +    if (stream_matrix) {
> > +        memcpy(ifilter->display_matrix, stream_matrix, 4 * 9);
> > +        return 0;
> > +    }
> > +
> > +    // cleanup the display matrix, may be from last frame
> > +    memset(ifilter->display_matrix, 0, 4 * 9);
> > +    av_display_rotation_set(ifilter->display_matrix, 0);
> > +
> > +    return !!av_dict_get(frame->metadata, "Orientation", NULL, 0);
> > +}
> > +
> >  static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame)
> >  {
> >      FilterGraph *fg = ifilter->graph;
> > @@ -2141,7 +2159,8 @@ 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 ||
> > +        need_reinit |= ifilter_display_matrix_need_from_frame(ifilter,
> frame) ||
> > +                       ifilter->width  != frame->width ||
> >                         ifilter->height != frame->height;
> >          break;
> >      }
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index eb1eaf6363..8331720663 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -251,6 +251,8 @@ typedef struct InputFilter {
> >      int channels;
> >      uint64_t channel_layout;
> >
> > +    int32_t display_matrix[9];
> > +
> >      AVBufferRef *hw_frames_ctx;
> >
> >      int eof;
> > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > index 72838de1e2..1894f30561 100644
> > --- a/fftools/ffmpeg_filter.c
> > +++ b/fftools/ffmpeg_filter.c
> > @@ -328,6 +328,7 @@ static void init_input_filter(FilterGraph *fg,
> AVFilterInOut *in)
> >      fg->inputs[fg->nb_inputs - 1]->format = -1;
> >      fg->inputs[fg->nb_inputs - 1]->type = ist->st->codecpar->codec_type;
> >      fg->inputs[fg->nb_inputs - 1]->name = describe_filter_link(fg, in,
> 1);
> > +    av_display_rotation_set(fg->inputs[fg->nb_inputs -
> 1]->display_matrix, 0);
> >
> >      fg->inputs[fg->nb_inputs - 1]->frame_queue = av_fifo_alloc(8 *
> sizeof(AVFrame*));
> >      if (!fg->inputs[fg->nb_inputs - 1]->frame_queue)
> > @@ -807,22 +808,43 @@ static int
> configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
> >      last_filter = ifilter->filter;
> >
> >      if (ist->autorotate) {
> > -        double theta = get_rotation(ist->st);
> > +        int hflip = 0;
> > +        double theta = get_rotation_hflip(ifilter->display_matrix,
> &hflip);
> >
> > -        if (fabs(theta - 90) < 1.0) {
> > +        if (fabs(theta) < 1.0) {
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> > +        } else if (fabs(theta - 90) < 1.0) {
> >              ret = insert_filter(&last_filter, &pad_idx, "transpose",
> "clock");
> > -        } else if (fabs(theta - 180) < 1.0) {
> > -            ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> >              if (ret < 0)
> >                  return ret;
> > -            ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> > +        } else if (fabs(theta - 180) < 1.0) {
> > +            if (hflip) { // rotate 180 and hflip equals vflip
> > +                ret = insert_filter(&last_filter, &pad_idx, "vflip",
> NULL);
> > +            } else {
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> > +                if (ret < 0)
> > +                    return ret;
> > +                ret = insert_filter(&last_filter, &pad_idx, "vflip",
> NULL);
> > +            }
> >          } else if (fabs(theta - 270) < 1.0) {
> >              ret = insert_filter(&last_filter, &pad_idx, "transpose",
> "cclock");
> > +            if (ret < 0)
> > +                return ret;
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> >          } else if (fabs(theta) > 1.0) {
> >              char rotate_buf[64];
> >              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180",
> theta);
> >              ret = insert_filter(&last_filter, &pad_idx, "rotate",
> rotate_buf);
> > +            if (ret < 0)
> > +                return ret;
> > +            if (hflip)
> > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> NULL);
> >          }
>
> Why do you need the complicated logic that uses horizontal
> flip and theta?
> Looking at the code before, it looks that all the cases were handled using
> only theta.
>

Thanks Andriy for review.
This is because we cannot achieve flip effect by image rotation.
If the frame's meta data requires mirror (hflip), the current input
filter-graph cannot handle it.
This is what this patch is for, ticket 6945 is an example requires mirror
for a jpg but not correctly processed.
Thanks.

Best Regards,
Jun





> +
> >          if (ret < 0)
> >              return ret;
> >      }
> > @@ -1182,6 +1204,53 @@ fail:
> >      return ret;
> >  }
> >
> > +static void set_display_matrix_from_frame(const AVFrame *frame, int32_t
> m[9])
> > +{
> > +    AVDictionaryEntry *entry = NULL;
> > +    int orientation = 0;
> > +
> > +    // read exif orientation data
> > +    entry = av_dict_get(frame->metadata, "Orientation", NULL, 0);
> > +    if (entry) {
> > +        orientation = atoi(entry->value);
> > +        memset(m, 0, 4 * 9);
> > +        switch (orientation)
> > +        {
> > +            case 1: // horizontal (normal)
> > +                av_display_rotation_set(m, 0.0);
> > +                break;
> > +            case 2: // mirror horizontal
> > +                av_display_rotation_set(m, 0.0);
> > +                av_display_matrix_flip(m, 1, 0);
> > +                break;
> > +            case 3: // rotate 180
> > +                av_display_rotation_set(m, 180.0);
> > +                break;
> > +            case 4: // mirror vertical
> > +                av_display_rotation_set(m, 0.0);
> > +                av_display_matrix_flip(m, 0, 1);
> > +                break;
> > +            case 5: // mirror horizontal and rotate 270 CW
> > +                av_display_rotation_set(m, 270.0);
> > +                av_display_matrix_flip(m, 0, 1);
> > +                break;
> > +            case 6: // rotate 90 CW
> > +                av_display_rotation_set(m, 90.0);
> > +                break;
> > +            case 7: // mirror horizontal and rotate 90 CW
> > +                av_display_rotation_set(m, 90.0);
> > +                av_display_matrix_flip(m, 0, 1);
> > +                break;
> > +            case 8: // rotate 270 CW
> > +                av_display_rotation_set(m, 270.0);
> > +                break;
> > +            default:
> > +                av_display_rotation_set(m, 0.0);
> > +                break;
> > +        }
> > +    }
> > +}
> > +
> >  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame
> *frame)
> >  {
> >      av_buffer_unref(&ifilter->hw_frames_ctx);
> > @@ -1202,6 +1271,8 @@ int ifilter_parameters_from_frame(InputFilter
> *ifilter, const AVFrame *frame)
> >              return AVERROR(ENOMEM);
> >      }
> >
> > +    set_display_matrix_from_frame(frame, ifilter->display_matrix);
> > +
> >      return 0;
> >  }
> >
> > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > index 8f050e16e6..717a9a830c 100644
> > --- a/fftools/ffplay.c
> > +++ b/fftools/ffplay.c
> > @@ -1914,19 +1914,37 @@ static int configure_video_filters(AVFilterGraph
> *graph, VideoState *is, const c
> >  } while (0)
> >
> >      if (autorotate) {
> > -        double theta  = get_rotation(is->video_st);
> > -
> > -        if (fabs(theta - 90) < 1.0) {
> > +        int hflip = 0;
> > +        double theta = 0.0;
> > +        int32_t* display_matrix =
> (int32_t*)av_stream_get_side_data(is->video_st,
> > +
> AV_PKT_DATA_DISPLAYMATRIX, NULL);
> > +        if (display_matrix)
> > +             theta  = get_rotation_hflip(display_matrix, &hflip);
> > +
> > +        if (fabs(theta) < 1.0) {
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> > +        } else if (fabs(theta - 90) < 1.0) {
> >              INSERT_FILT("transpose", "clock");
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> >          } else if (fabs(theta - 180) < 1.0) {
> > -            INSERT_FILT("hflip", NULL);
> > -            INSERT_FILT("vflip", NULL);
> > +            if (hflip) { // rotate 180 and hflip equals vflip
> > +                INSERT_FILT("vflip", NULL);
> > +            } else {
> > +                INSERT_FILT("hflip", NULL);
> > +                INSERT_FILT("vflip", NULL);
> > +            }
> >          } else if (fabs(theta - 270) < 1.0) {
> >              INSERT_FILT("transpose", "cclock");
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> >          } else if (fabs(theta) > 1.0) {
> >              char rotate_buf[64];
> >              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180",
> theta);
> >              INSERT_FILT("rotate", rotate_buf);
> > +            if (hflip)
> > +                INSERT_FILT("hflip", NULL);
> >          }
> >      }
> Same question about theta and hflip.

>
> > --
> > 2.17.1
> >
> > _______________________________________________
> > 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,
> Andriy
>
> _______________________________________________
> 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".
Andriy Gelman May 15, 2019, 7:48 p.m. UTC | #5 
On Wed, 15. May 11:57, Jun Li wrote:
> On Wed, May 15, 2019 at 11:45 AM Andriy Gelman <andriy.gelman@gmail.com>
> wrote:
> 
> > On Tue, 14. May 22:36, Jun Li wrote:
> > > Fix #6945
> > > Current implementaion for autorotate works fine for stream
> > > level rotataion but no support for frame level operation
> > > and frame flip. This patch is for adding flip support and
> > > per frame operations.
> > > ---
> > >  fftools/cmdutils.c      |  9 ++---
> > >  fftools/cmdutils.h      |  2 +-
> > >  fftools/ffmpeg.c        | 21 ++++++++++-
> > >  fftools/ffmpeg.h        |  2 +
> > >  fftools/ffmpeg_filter.c | 81 ++++++++++++++++++++++++++++++++++++++---
> > >  fftools/ffplay.c        | 28 +++++++++++---
> > >  6 files changed, 126 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> > > index 9cfbc45c2b..b8bb904419 100644
> > > --- a/fftools/cmdutils.c
> > > +++ b/fftools/cmdutils.c
> > > @@ -2172,13 +2172,12 @@ void *grow_array(void *array, int elem_size, int
> > *size, int new_size)
> > >      return array;
> > >  }
> > >
> > > -double get_rotation(AVStream *st)
> > > +double get_rotation_hflip(int32_t display_matrix[9], int* hflip)
> > >  {
> > > -    uint8_t* displaymatrix = av_stream_get_side_data(st,
> > > -
> >  AV_PKT_DATA_DISPLAYMATRIX, NULL);
> > >      double theta = 0;
> > > -    if (displaymatrix)
> > > -        theta = -av_display_rotation_get((int32_t*) displaymatrix);
> > > +
> > > +    if (display_matrix)
> > > +        theta = -av_display_rotation_hflip_get(display_matrix, hflip);
> > >
> > >      theta -= 360*floor(theta/360 + 0.9/360);
> > >
> > > diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> > > index 6e2e0a2acb..dce44ed6e1 100644
> > > --- a/fftools/cmdutils.h
> > > +++ b/fftools/cmdutils.h
> > > @@ -643,6 +643,6 @@ void *grow_array(void *array, int elem_size, int
> > *size, int new_size);
> > >      char name[128];\
> > >      av_get_channel_layout_string(name, sizeof(name), 0, ch_layout);
> > >
> > > -double get_rotation(AVStream *st);
> > > +double get_rotation_hflip(int32_t display_matrix[9], int* hflip);
> > >
> > >  #endif /* FFTOOLS_CMDUTILS_H */
> > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > > index 01f04103cf..9ea1aaa930 100644
> > > --- a/fftools/ffmpeg.c
> > > +++ b/fftools/ffmpeg.c
> > > @@ -2126,6 +2126,24 @@ static int
> > ifilter_has_all_input_formats(FilterGraph *fg)
> > >      return 1;
> > >  }
> > >
> > > +static int ifilter_display_matrix_need_from_frame(InputFilter *ifilter,
> > AVFrame* frame)
> > > +{
> > > +    int32_t* stream_matrix =
> > (int32_t*)av_stream_get_side_data(ifilter->ist->st,
> > > +
> > AV_PKT_DATA_DISPLAYMATRIX, NULL);
> > > +    // if we already have display matrix from stream, use it instead of
> > extracting
> > > +    // from frame.
> > > +    if (stream_matrix) {
> > > +        memcpy(ifilter->display_matrix, stream_matrix, 4 * 9);
> > > +        return 0;
> > > +    }
> > > +
> > > +    // cleanup the display matrix, may be from last frame
> > > +    memset(ifilter->display_matrix, 0, 4 * 9);
> > > +    av_display_rotation_set(ifilter->display_matrix, 0);
> > > +
> > > +    return !!av_dict_get(frame->metadata, "Orientation", NULL, 0);
> > > +}
> > > +
> > >  static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame)
> > >  {
> > >      FilterGraph *fg = ifilter->graph;
> > > @@ -2141,7 +2159,8 @@ 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 ||
> > > +        need_reinit |= ifilter_display_matrix_need_from_frame(ifilter,
> > frame) ||
> > > +                       ifilter->width  != frame->width ||
> > >                         ifilter->height != frame->height;
> > >          break;
> > >      }
> > > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > > index eb1eaf6363..8331720663 100644
> > > --- a/fftools/ffmpeg.h
> > > +++ b/fftools/ffmpeg.h
> > > @@ -251,6 +251,8 @@ typedef struct InputFilter {
> > >      int channels;
> > >      uint64_t channel_layout;
> > >
> > > +    int32_t display_matrix[9];
> > > +
> > >      AVBufferRef *hw_frames_ctx;
> > >
> > >      int eof;
> > > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> > > index 72838de1e2..1894f30561 100644
> > > --- a/fftools/ffmpeg_filter.c
> > > +++ b/fftools/ffmpeg_filter.c
> > > @@ -328,6 +328,7 @@ static void init_input_filter(FilterGraph *fg,
> > AVFilterInOut *in)
> > >      fg->inputs[fg->nb_inputs - 1]->format = -1;
> > >      fg->inputs[fg->nb_inputs - 1]->type = ist->st->codecpar->codec_type;
> > >      fg->inputs[fg->nb_inputs - 1]->name = describe_filter_link(fg, in,
> > 1);
> > > +    av_display_rotation_set(fg->inputs[fg->nb_inputs -
> > 1]->display_matrix, 0);
> > >
> > >      fg->inputs[fg->nb_inputs - 1]->frame_queue = av_fifo_alloc(8 *
> > sizeof(AVFrame*));
> > >      if (!fg->inputs[fg->nb_inputs - 1]->frame_queue)
> > > @@ -807,22 +808,43 @@ static int
> > configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
> > >      last_filter = ifilter->filter;
> > >
> > >      if (ist->autorotate) {
> > > -        double theta = get_rotation(ist->st);
> > > +        int hflip = 0;
> > > +        double theta = get_rotation_hflip(ifilter->display_matrix,
> > &hflip);
> > >
> > > -        if (fabs(theta - 90) < 1.0) {
> > > +        if (fabs(theta) < 1.0) {
> > > +            if (hflip)
> > > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> > NULL);
> > > +        } else if (fabs(theta - 90) < 1.0) {
> > >              ret = insert_filter(&last_filter, &pad_idx, "transpose",
> > "clock");
> > > -        } else if (fabs(theta - 180) < 1.0) {
> > > -            ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
> > >              if (ret < 0)
> > >                  return ret;
> > > -            ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
> > > +            if (hflip)
> > > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> > NULL);
> > > +        } else if (fabs(theta - 180) < 1.0) {
> > > +            if (hflip) { // rotate 180 and hflip equals vflip
> > > +                ret = insert_filter(&last_filter, &pad_idx, "vflip",
> > NULL);
> > > +            } else {
> > > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> > NULL);
> > > +                if (ret < 0)
> > > +                    return ret;
> > > +                ret = insert_filter(&last_filter, &pad_idx, "vflip",
> > NULL);
> > > +            }
> > >          } else if (fabs(theta - 270) < 1.0) {
> > >              ret = insert_filter(&last_filter, &pad_idx, "transpose",
> > "cclock");
> > > +            if (ret < 0)
> > > +                return ret;
> > > +            if (hflip)
> > > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> > NULL);
> > >          } else if (fabs(theta) > 1.0) {
> > >              char rotate_buf[64];
> > >              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180",
> > theta);
> > >              ret = insert_filter(&last_filter, &pad_idx, "rotate",
> > rotate_buf);
> > > +            if (ret < 0)
> > > +                return ret;
> > > +            if (hflip)
> > > +                ret = insert_filter(&last_filter, &pad_idx, "hflip",
> > NULL);
> > >          }
> >
> > Why do you need the complicated logic that uses horizontal
> > flip and theta?
> > Looking at the code before, it looks that all the cases were handled using
> > only theta.
> >
> 
> Thanks Andriy for review.
> This is because we cannot achieve flip effect by image rotation.
> If the frame's meta data requires mirror (hflip), the current input
> filter-graph cannot handle it.
> This is what this patch is for, ticket 6945 is an example requires mirror
> for a jpg but not correctly processed.
> Thanks.
> 
> Best Regards,
> Jun
> 

Yes, my bad. Thanks for the clarification!
I tested on different images from ticket 6945 and it worked. 

Thanks, 
Andriy
diff mbox

Patch

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 9cfbc45c2b..b8bb904419 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -2172,13 +2172,12 @@  void *grow_array(void *array, int elem_size, int *size, int new_size)
     return array;
 }
 
-double get_rotation(AVStream *st)
+double get_rotation_hflip(int32_t display_matrix[9], int* hflip)
 {
-    uint8_t* displaymatrix = av_stream_get_side_data(st,
-                                                     AV_PKT_DATA_DISPLAYMATRIX, NULL);
     double theta = 0;
-    if (displaymatrix)
-        theta = -av_display_rotation_get((int32_t*) displaymatrix);
+
+    if (display_matrix)
+        theta = -av_display_rotation_hflip_get(display_matrix, hflip);
 
     theta -= 360*floor(theta/360 + 0.9/360);
 
diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
index 6e2e0a2acb..dce44ed6e1 100644
--- a/fftools/cmdutils.h
+++ b/fftools/cmdutils.h
@@ -643,6 +643,6 @@  void *grow_array(void *array, int elem_size, int *size, int new_size);
     char name[128];\
     av_get_channel_layout_string(name, sizeof(name), 0, ch_layout);
 
-double get_rotation(AVStream *st);
+double get_rotation_hflip(int32_t display_matrix[9], int* hflip);
 
 #endif /* FFTOOLS_CMDUTILS_H */
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 01f04103cf..9ea1aaa930 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2126,6 +2126,24 @@  static int ifilter_has_all_input_formats(FilterGraph *fg)
     return 1;
 }
 
+static int ifilter_display_matrix_need_from_frame(InputFilter *ifilter, AVFrame* frame)
+{
+    int32_t* stream_matrix = (int32_t*)av_stream_get_side_data(ifilter->ist->st, 
+                                                AV_PKT_DATA_DISPLAYMATRIX, NULL);
+    // if we already have display matrix from stream, use it instead of extracting
+    // from frame.
+    if (stream_matrix) {
+        memcpy(ifilter->display_matrix, stream_matrix, 4 * 9);
+        return 0;
+    }
+
+    // cleanup the display matrix, may be from last frame
+    memset(ifilter->display_matrix, 0, 4 * 9);
+    av_display_rotation_set(ifilter->display_matrix, 0);
+
+    return !!av_dict_get(frame->metadata, "Orientation", NULL, 0);
+}
+
 static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame)
 {
     FilterGraph *fg = ifilter->graph;
@@ -2141,7 +2159,8 @@  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 ||
+        need_reinit |= ifilter_display_matrix_need_from_frame(ifilter, frame) ||
+                       ifilter->width  != frame->width ||
                        ifilter->height != frame->height;
         break;
     }
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index eb1eaf6363..8331720663 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -251,6 +251,8 @@  typedef struct InputFilter {
     int channels;
     uint64_t channel_layout;
 
+    int32_t display_matrix[9];
+    
     AVBufferRef *hw_frames_ctx;
 
     int eof;
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 72838de1e2..1894f30561 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -328,6 +328,7 @@  static void init_input_filter(FilterGraph *fg, AVFilterInOut *in)
     fg->inputs[fg->nb_inputs - 1]->format = -1;
     fg->inputs[fg->nb_inputs - 1]->type = ist->st->codecpar->codec_type;
     fg->inputs[fg->nb_inputs - 1]->name = describe_filter_link(fg, in, 1);
+    av_display_rotation_set(fg->inputs[fg->nb_inputs - 1]->display_matrix, 0);
 
     fg->inputs[fg->nb_inputs - 1]->frame_queue = av_fifo_alloc(8 * sizeof(AVFrame*));
     if (!fg->inputs[fg->nb_inputs - 1]->frame_queue)
@@ -807,22 +808,43 @@  static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
     last_filter = ifilter->filter;
 
     if (ist->autorotate) {
-        double theta = get_rotation(ist->st);
+        int hflip = 0;
+        double theta = get_rotation_hflip(ifilter->display_matrix, &hflip);
 
-        if (fabs(theta - 90) < 1.0) {
+        if (fabs(theta) < 1.0) {
+            if (hflip)
+                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
+        } else if (fabs(theta - 90) < 1.0) {
             ret = insert_filter(&last_filter, &pad_idx, "transpose", "clock");
-        } else if (fabs(theta - 180) < 1.0) {
-            ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
             if (ret < 0)
                 return ret;
-            ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
+            if (hflip)
+                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
+        } else if (fabs(theta - 180) < 1.0) {
+            if (hflip) { // rotate 180 and hflip equals vflip
+                ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
+            } else {
+                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
+                if (ret < 0)
+                    return ret;
+                ret = insert_filter(&last_filter, &pad_idx, "vflip", NULL);
+            }
         } else if (fabs(theta - 270) < 1.0) {
             ret = insert_filter(&last_filter, &pad_idx, "transpose", "cclock");
+            if (ret < 0)
+                return ret;
+            if (hflip)
+                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
         } else if (fabs(theta) > 1.0) {
             char rotate_buf[64];
             snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
             ret = insert_filter(&last_filter, &pad_idx, "rotate", rotate_buf);
+            if (ret < 0)
+                return ret;
+            if (hflip)
+                ret = insert_filter(&last_filter, &pad_idx, "hflip", NULL);
         }
+
         if (ret < 0)
             return ret;
     }
@@ -1182,6 +1204,53 @@  fail:
     return ret;
 }
 
+static void set_display_matrix_from_frame(const AVFrame *frame, int32_t m[9])
+{
+    AVDictionaryEntry *entry = NULL;
+    int orientation = 0;
+    
+    // read exif orientation data
+    entry = av_dict_get(frame->metadata, "Orientation", NULL, 0);
+    if (entry) {
+        orientation = atoi(entry->value);
+        memset(m, 0, 4 * 9);
+        switch (orientation)
+        {
+            case 1: // horizontal (normal)
+                av_display_rotation_set(m, 0.0);
+                break;
+            case 2: // mirror horizontal
+                av_display_rotation_set(m, 0.0);
+                av_display_matrix_flip(m, 1, 0);
+                break;
+            case 3: // rotate 180
+                av_display_rotation_set(m, 180.0);
+                break;
+            case 4: // mirror vertical
+                av_display_rotation_set(m, 0.0);
+                av_display_matrix_flip(m, 0, 1);
+                break;
+            case 5: // mirror horizontal and rotate 270 CW
+                av_display_rotation_set(m, 270.0);
+                av_display_matrix_flip(m, 0, 1);
+                break;
+            case 6: // rotate 90 CW
+                av_display_rotation_set(m, 90.0);
+                break;
+            case 7: // mirror horizontal and rotate 90 CW
+                av_display_rotation_set(m, 90.0);
+                av_display_matrix_flip(m, 0, 1);
+                break;
+            case 8: // rotate 270 CW
+                av_display_rotation_set(m, 270.0);
+                break;
+            default:
+                av_display_rotation_set(m, 0.0);
+                break;
+        }
+    }
+}
+
 int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
 {
     av_buffer_unref(&ifilter->hw_frames_ctx);
@@ -1202,6 +1271,8 @@  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
             return AVERROR(ENOMEM);
     }
 
+    set_display_matrix_from_frame(frame, ifilter->display_matrix);
+
     return 0;
 }
 
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 8f050e16e6..717a9a830c 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1914,19 +1914,37 @@  static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
 } while (0)
 
     if (autorotate) {
-        double theta  = get_rotation(is->video_st);
-
-        if (fabs(theta - 90) < 1.0) {
+        int hflip = 0;
+        double theta = 0.0;
+        int32_t* display_matrix = (int32_t*)av_stream_get_side_data(is->video_st, 
+                                                AV_PKT_DATA_DISPLAYMATRIX, NULL);
+        if (display_matrix)
+             theta  = get_rotation_hflip(display_matrix, &hflip);
+
+        if (fabs(theta) < 1.0) {
+            if (hflip)
+                INSERT_FILT("hflip", NULL);
+        } else if (fabs(theta - 90) < 1.0) {
             INSERT_FILT("transpose", "clock");
+            if (hflip)
+                INSERT_FILT("hflip", NULL);
         } else if (fabs(theta - 180) < 1.0) {
-            INSERT_FILT("hflip", NULL);
-            INSERT_FILT("vflip", NULL);
+            if (hflip) { // rotate 180 and hflip equals vflip
+                INSERT_FILT("vflip", NULL);
+            } else {
+                INSERT_FILT("hflip", NULL);
+                INSERT_FILT("vflip", NULL);
+            }
         } else if (fabs(theta - 270) < 1.0) {
             INSERT_FILT("transpose", "cclock");
+            if (hflip)
+                INSERT_FILT("hflip", NULL);
         } else if (fabs(theta) > 1.0) {
             char rotate_buf[64];
             snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
             INSERT_FILT("rotate", rotate_buf);
+            if (hflip)
+                INSERT_FILT("hflip", NULL);
         }
     }