diff mbox series

[FFmpeg-devel,v3] fftools/ffplay: fix rotation incorrect when frame contains the displaymatrix

Message ID tencent_2AB3646595CA47E4273F2379D715BEA9A107@qq.com
State Accepted
Commit 3f0fac9303b430c5114da62f9a63ed0b945b435f
Headers show
Series [FFmpeg-devel,v3] fftools/ffplay: fix rotation incorrect when frame contains the displaymatrix | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

wangyaqiang Sept. 5, 2022, 10:40 a.m. UTC 
From: Wang Yaqiang <wangyaqiang03@kuaishou.com>

For example, if the jpeg contains exif information
and the rotation direction is included in the exif,
the displaymatrix will be set on the side_data of the frame when decoding.
However, when ffplay is used to play the image,
only the side data in the stream will be determined.
It does not check whether the frame also contains rotation information,
causing it to play in the wrong direction

Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
---
 fftools/ffplay.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Zhao Zhili Sept. 6, 2022, 6:38 a.m. UTC | #1 
On Mon, 2022-09-05 at 18:40 +0800, 1035567130@qq.com wrote:
> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> 
> For example, if the jpeg contains exif information
> and the rotation direction is included in the exif,
> the displaymatrix will be set on the side_data of the frame when
> decoding.
> However, when ffplay is used to play the image,
> only the side data in the stream will be determined.
> It does not check whether the frame also contains rotation
> information,
> causing it to play in the wrong direction
> 
> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
>  fftools/ffplay.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 9242047f5c..bcc00afe31 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1915,8 +1915,14 @@ static int
> configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>  } while (0)
>  
>      if (autorotate) {
> -        int32_t *displaymatrix = (int32_t
> *)av_stream_get_side_data(is->video_st, AV_PKT_DATA_DISPLAYMATRIX,
> NULL);
> -        double theta = get_rotation(displaymatrix);
> +        double theta = 0.0;
> +        int32_t *displaymatrix = NULL;
> +        AVFrameSideData *sd = av_frame_get_side_data(frame,
> AV_FRAME_DATA_DISPLAYMATRIX);
> +        if (sd)
> +            displaymatrix = (int32_t *)sd->data;
> +        if (!displaymatrix)
> +            displaymatrix = (int32_t *)av_stream_get_side_data(is-
> >video_st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
> +        theta = get_rotation(displaymatrix);
>  
>          if (fabs(theta - 90) < 1.0) {
>              INSERT_FILT("transpose", "clock");

LGTM.
Steven Liu Sept. 15, 2022, 8:59 a.m. UTC | #2 
Zhao Zhili <quinkblack@foxmail.com> 于2022年9月6日周二 14:38写道：
>
> On Mon, 2022-09-05 at 18:40 +0800, 1035567130@qq.com wrote:
> > From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> >
> > For example, if the jpeg contains exif information
> > and the rotation direction is included in the exif,
> > the displaymatrix will be set on the side_data of the frame when
> > decoding.
> > However, when ffplay is used to play the image,
> > only the side data in the stream will be determined.
> > It does not check whether the frame also contains rotation
> > information,
> > causing it to play in the wrong direction
> >
> > Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> > ---
> >  fftools/ffplay.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > index 9242047f5c..bcc00afe31 100644
> > --- a/fftools/ffplay.c
> > +++ b/fftools/ffplay.c
> > @@ -1915,8 +1915,14 @@ static int
> > configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
> >  } while (0)
> >
> >      if (autorotate) {
> > -        int32_t *displaymatrix = (int32_t
> > *)av_stream_get_side_data(is->video_st, AV_PKT_DATA_DISPLAYMATRIX,
> > NULL);
> > -        double theta = get_rotation(displaymatrix);
> > +        double theta = 0.0;
> > +        int32_t *displaymatrix = NULL;
> > +        AVFrameSideData *sd = av_frame_get_side_data(frame,
> > AV_FRAME_DATA_DISPLAYMATRIX);
> > +        if (sd)
> > +            displaymatrix = (int32_t *)sd->data;
> > +        if (!displaymatrix)
> > +            displaymatrix = (int32_t *)av_stream_get_side_data(is-
> > >video_st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
> > +        theta = get_rotation(displaymatrix);
> >
> >          if (fabs(theta - 90) < 1.0) {
> >              INSERT_FILT("transpose", "clock");
>
> LGTM.

Applied!


Thanks
Steven
diff mbox series

Patch

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 9242047f5c..bcc00afe31 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1915,8 +1915,14 @@  static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
 } while (0)
 
     if (autorotate) {
-        int32_t *displaymatrix = (int32_t *)av_stream_get_side_data(is->video_st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
-        double theta = get_rotation(displaymatrix);
+        double theta = 0.0;
+        int32_t *displaymatrix = NULL;
+        AVFrameSideData *sd = av_frame_get_side_data(frame, AV_FRAME_DATA_DISPLAYMATRIX);
+        if (sd)
+            displaymatrix = (int32_t *)sd->data;
+        if (!displaymatrix)
+            displaymatrix = (int32_t *)av_stream_get_side_data(is->video_st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
+        theta = get_rotation(displaymatrix);
 
         if (fabs(theta - 90) < 1.0) {
             INSERT_FILT("transpose", "clock");