[FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[1035567130]

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
---
 fftools/ffplay.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 9242047f5c..868123dc65 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1917,7 +1917,8 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
     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);
-
+        int frame_rotation_checked = 0;
+    rotation:
         if (fabs(theta - 90) < 1.0) {
             INSERT_FILT("transpose", "clock");
         } else if (fabs(theta - 180) < 1.0) {
@@ -1930,6 +1931,15 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
             snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
             INSERT_FILT("rotate", rotate_buf);
         }
+        if (!frame_rotation_checked) {
+            frame_rotation_checked = 1;
+            AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);
+            if (sd) {
+                displaymatrix = (int32_t *)sd->data;
+                theta = get_rotation(displaymatrix);
+                goto rotation;
+            }
+        }
     }
 
     if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
-- 
2.33.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[Zhao Zhili]

> -----Original Message-----
> From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of 1035567130@qq.com
> Sent: 2022年8月31日 18:45
> To: ffmpeg-devel@ffmpeg.org
> Cc: Wang Yaqiang <wangyaqiang03@kuaishou.com>; 1445440736@qq.com
> Subject: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is
> AV_FRAME_DATA_DISPLAYMATRIX)
> 
> 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

When both per-stream and per-frame DISPLAYMATRIX exist, we need to decode how they work
together. Does one applied after the other, or one override the other? I have no idea.

Secondly, even If one is applied after the other, we shouldn't use two rotation filter
but merge the operation into a single one.

> ---
>  fftools/ffplay.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 9242047f5c..868123dc65 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1917,7 +1917,8 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>      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);
> -
> +        int frame_rotation_checked = 0;
> +    rotation:
>          if (fabs(theta - 90) < 1.0) {
>              INSERT_FILT("transpose", "clock");
>          } else if (fabs(theta - 180) < 1.0) {
> @@ -1930,6 +1931,15 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
>              INSERT_FILT("rotate", rotate_buf);
>          }
> +        if (!frame_rotation_checked) {
> +            frame_rotation_checked = 1;
> +            AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);
> +            if (sd) {
> +                displaymatrix = (int32_t *)sd->data;
> +                theta = get_rotation(displaymatrix);
> +                goto rotation;
> +            }
> +        }
>      }
> 
>      if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
> --
> 2.33.0
> 
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[Steven Liu]

Zhao Zhili <quinkblack@foxmail.com> 于2022年9月1日周四 00:34写道：
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of 1035567130@qq.com
> > Sent: 2022年8月31日 18:45
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Wang Yaqiang <wangyaqiang03@kuaishou.com>; 1445440736@qq.com
> > Subject: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is
> > AV_FRAME_DATA_DISPLAYMATRIX)
> >
> > 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
>
> When both per-stream and per-frame DISPLAYMATRIX exist, we need to decode how they work
> together. Does one applied after the other, or one override the other? I have no idea.
I think we should "Does one applied after the other", do frame rotate
adter stream rotate, because user maybe not need every frames rotate.

>
> Secondly, even If one is applied after the other, we shouldn't use two rotation filter
> but merge the operation into a single one.
Agreed, should merge two rotate value into. one value.
>
> > ---
> >  fftools/ffplay.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > index 9242047f5c..868123dc65 100644
> > --- a/fftools/ffplay.c
> > +++ b/fftools/ffplay.c
> > @@ -1917,7 +1917,8 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
> >      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);
> > -
> > +        int frame_rotation_checked = 0;
> > +    rotation:
> >          if (fabs(theta - 90) < 1.0) {
> >              INSERT_FILT("transpose", "clock");
> >          } else if (fabs(theta - 180) < 1.0) {
> > @@ -1930,6 +1931,15 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
> >              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
> >              INSERT_FILT("rotate", rotate_buf);
> >          }
> > +        if (!frame_rotation_checked) {
> > +            frame_rotation_checked = 1;
> > +            AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);
> > +            if (sd) {
> > +                displaymatrix = (int32_t *)sd->data;
> > +                theta = get_rotation(displaymatrix);
> > +                goto rotation;
> > +            }
> > +        }
> >      }
> >
> >      if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
> > --
> > 2.33.0
> >
> > _______________________________________________
> > 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[Steven Liu]

Steven Liu <lingjiujianke@gmail.com> 于2022年9月1日周四 09:17写道：
>
> Zhao Zhili <quinkblack@foxmail.com> 于2022年9月1日周四 00:34写道：
> >
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of 1035567130@qq.com
> > > Sent: 2022年8月31日 18:45
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: Wang Yaqiang <wangyaqiang03@kuaishou.com>; 1445440736@qq.com
> > > Subject: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is
> > > AV_FRAME_DATA_DISPLAYMATRIX)
> > >
> > > 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
> >
> > When both per-stream and per-frame DISPLAYMATRIX exist, we need to decode how they work
> > together. Does one applied after the other, or one override the other? I have no idea.
> I think we should "Does one applied after the other", do frame rotate
> adter stream rotate, because user maybe not need every frames rotate.
BTW,  we get jpeg picture need rotate by exif, but there have no
rotate data in stream,
the other sample we get there have rotate in stream only.
have not get rotate data in one time both stream and frames by one sample yet,
So we think it should do twice rotate operation in ffplay. because it
have no problem in ffmpeg, so we only modify ffplay.
>
> >
> > Secondly, even If one is applied after the other, we shouldn't use two rotation filter
> > but merge the operation into a single one.
> Agreed, should merge two rotate value into. one value.
> >
> > > ---
> > >  fftools/ffplay.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > > index 9242047f5c..868123dc65 100644
> > > --- a/fftools/ffplay.c
> > > +++ b/fftools/ffplay.c
> > > @@ -1917,7 +1917,8 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
> > >      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);
> > > -
> > > +        int frame_rotation_checked = 0;
> > > +    rotation:
> > >          if (fabs(theta - 90) < 1.0) {
> > >              INSERT_FILT("transpose", "clock");
> > >          } else if (fabs(theta - 180) < 1.0) {
> > > @@ -1930,6 +1931,15 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
> > >              snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
> > >              INSERT_FILT("rotate", rotate_buf);
> > >          }
> > > +        if (!frame_rotation_checked) {
> > > +            frame_rotation_checked = 1;
> > > +            AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);
> > > +            if (sd) {
> > > +                displaymatrix = (int32_t *)sd->data;
> > > +                theta = get_rotation(displaymatrix);
> > > +                goto rotation;
> > > +            }
> > > +        }
> > >      }
> > >
> > >      if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
> > > --
> > > 2.33.0
> > >
> > > _______________________________________________
> > > 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[wangyaqiang]

> 2022年9月1日 上午9:24，Steven Liu <lingjiujianke@gmail.com> 写道：
> 
> Steven Liu <lingjiujianke@gmail.com> 于2022年9月1日周四 09:17写道：
>> 
>> Zhao Zhili <quinkblack@foxmail.com> 于2022年9月1日周四 00:34写道：
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of 1035567130@qq.com
>>>> Sent: 2022年8月31日 18:45
>>>> To: ffmpeg-devel@ffmpeg.org
>>>> Cc: Wang Yaqiang <wangyaqiang03@kuaishou.com>; 1445440736@qq.com
>>>> Subject: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is
>>>> AV_FRAME_DATA_DISPLAYMATRIX)
>>>> 
>>>> 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
>>> 
>>> When both per-stream and per-frame DISPLAYMATRIX exist, we need to decode how they work
>>> together. Does one applied after the other, or one override the other? I have no idea.
>> I think we should "Does one applied after the other", do frame rotate
>> adter stream rotate, because user maybe not need every frames rotate.
> BTW,  we get jpeg picture need rotate by exif, but there have no
> rotate data in stream,
> the other sample we get there have rotate in stream only.
> have not get rotate data in one time both stream and frames by one sample yet,
> So we think it should do twice rotate operation in ffplay. because it
> have no problem in ffmpeg, so we only modify ffplay.

See the FFmpeg rotation information processing logic:
First read the DisplayMatrix in the Frame, 
and then read the stream if it doesn't exist, 
modify it to the same logic as FFmpeg?

>> 
>>> 
>>> Secondly, even If one is applied after the other, we shouldn't use two rotation filter
>>> but merge the operation into a single one.
>> Agreed, should merge two rotate value into. one value.
>>> 
>>>> ---
>>>> fftools/ffplay.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
>>>> index 9242047f5c..868123dc65 100644
>>>> --- a/fftools/ffplay.c
>>>> +++ b/fftools/ffplay.c
>>>> @@ -1917,7 +1917,8 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>>>>     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);
>>>> -
>>>> +        int frame_rotation_checked = 0;
>>>> +    rotation:
>>>>         if (fabs(theta - 90) < 1.0) {
>>>>             INSERT_FILT("transpose", "clock");
>>>>         } else if (fabs(theta - 180) < 1.0) {
>>>> @@ -1930,6 +1931,15 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>>>>             snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
>>>>             INSERT_FILT("rotate", rotate_buf);
>>>>         }
>>>> +        if (!frame_rotation_checked) {
>>>> +            frame_rotation_checked = 1;
>>>> +            AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);
>>>> +            if (sd) {
>>>> +                displaymatrix = (int32_t *)sd->data;
>>>> +                theta = get_rotation(displaymatrix);
>>>> +                goto rotation;
>>>> +            }
>>>> +        }
>>>>     }
>>>> 
>>>>     if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
>>>> --
>>>> 2.33.0
>>>> 
>>>> _______________________________________________
>>>> 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".
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[wangyaqiang]

> 2022年9月1日 上午9:24，Steven Liu <lingjiujianke@gmail.com> 写道：
> 
> Steven Liu <lingjiujianke@gmail.com> 于2022年9月1日周四 09:17写道：
>> 
>> Zhao Zhili <quinkblack@foxmail.com> 于2022年9月1日周四 00:34写道：
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of 1035567130@qq.com
>>>> Sent: 2022年8月31日 18:45
>>>> To: ffmpeg-devel@ffmpeg.org
>>>> Cc: Wang Yaqiang <wangyaqiang03@kuaishou.com>; 1445440736@qq.com
>>>> Subject: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is
>>>> AV_FRAME_DATA_DISPLAYMATRIX)
>>>> 
>>>> 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
>>> 
>>> When both per-stream and per-frame DISPLAYMATRIX exist, we need to decode how they work
>>> together. Does one applied after the other, or one override the other? I have no idea.
>> I think we should "Does one applied after the other", do frame rotate
>> adter stream rotate, because user maybe not need every frames rotate.
> BTW,  we get jpeg picture need rotate by exif, but there have no
> rotate data in stream,
> the other sample we get there have rotate in stream only.
> have not get rotate data in one time both stream and frames by one sample yet,
> So we think it should do twice rotate operation in ffplay. because it
> have no problem in ffmpeg, so we only modify ffplay.

See the FFmpeg rotation information processing logic:
First read the DisplayMatrix in the Frame, 
and then read the stream if it doesn't exist, 
modify it to the same logic as FFmpeg?

>> 
>>> 
>>> Secondly, even If one is applied after the other, we shouldn't use two rotation filter
>>> but merge the operation into a single one.
>> Agreed, should merge two rotate value into. one value.
>>> 
>>>> ---
>>>> fftools/ffplay.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
>>>> index 9242047f5c..868123dc65 100644
>>>> --- a/fftools/ffplay.c
>>>> +++ b/fftools/ffplay.c
>>>> @@ -1917,7 +1917,8 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>>>>     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);
>>>> -
>>>> +        int frame_rotation_checked = 0;
>>>> +    rotation:
>>>>         if (fabs(theta - 90) < 1.0) {
>>>>             INSERT_FILT("transpose", "clock");
>>>>         } else if (fabs(theta - 180) < 1.0) {
>>>> @@ -1930,6 +1931,15 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>>>>             snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
>>>>             INSERT_FILT("rotate", rotate_buf);
>>>>         }
>>>> +        if (!frame_rotation_checked) {
>>>> +            frame_rotation_checked = 1;
>>>> +            AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);
>>>> +            if (sd) {
>>>> +                displaymatrix = (int32_t *)sd->data;
>>>> +                theta = get_rotation(displaymatrix);
>>>> +                goto rotation;
>>>> +            }
>>>> +        }
>>>>     }
>>>> 
>>>>     if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
>>>> --
>>>> 2.33.0
>>>> 
>>>> _______________________________________________
>>>> 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".
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is AV_FRAME_DATA_DISPLAYMATRIX)

[Steven Liu]

wangyaqiang <1035567130@qq.com> 于2022年9月1日周四 12:05写道：
>
>
>
> > 2022年9月1日 上午9:24，Steven Liu <lingjiujianke@gmail.com> 写道：
> >
> > Steven Liu <lingjiujianke@gmail.com> 于2022年9月1日周四 09:17写道：
> >>
> >> Zhao Zhili <quinkblack@foxmail.com> 于2022年9月1日周四 00:34写道：
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel-bounces@ffmpeg.org <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of 1035567130@qq.com
> >>>> Sent: 2022年8月31日 18:45
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Cc: Wang Yaqiang <wangyaqiang03@kuaishou.com>; 1445440736@qq.com
> >>>> Subject: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect when frame contains the side_data(type is
> >>>> AV_FRAME_DATA_DISPLAYMATRIX)
> >>>>
> >>>> 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
> >>>
> >>> When both per-stream and per-frame DISPLAYMATRIX exist, we need to decode how they work
> >>> together. Does one applied after the other, or one override the other? I have no idea.
> >> I think we should "Does one applied after the other", do frame rotate
> >> adter stream rotate, because user maybe not need every frames rotate.
> > BTW,  we get jpeg picture need rotate by exif, but there have no
> > rotate data in stream,
> > the other sample we get there have rotate in stream only.
> > have not get rotate data in one time both stream and frames by one sample yet,
> > So we think it should do twice rotate operation in ffplay. because it
> > have no problem in ffmpeg, so we only modify ffplay.
>
> See the FFmpeg rotation information processing logic:
> First read the DisplayMatrix in the Frame,
> and then read the stream if it doesn't exist,
> modify it to the same logic as FFmpeg?

Yes, i think same as ffmpeg should be ok.
>
> >>
> >>>
> >>> Secondly, even If one is applied after the other, we shouldn't use two rotation filter
> >>> but merge the operation into a single one.
> >> Agreed, should merge two rotate value into. one value.
> >>>
> >>>> ---
> >>>> fftools/ffplay.c | 12 +++++++++++-
> >>>> 1 file changed, 11 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> >>>> index 9242047f5c..868123dc65 100644
> >>>> --- a/fftools/ffplay.c
> >>>> +++ b/fftools/ffplay.c
> >>>> @@ -1917,7 +1917,8 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
> >>>>     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);
> >>>> -
> >>>> +        int frame_rotation_checked = 0;
> >>>> +    rotation:
> >>>>         if (fabs(theta - 90) < 1.0) {
> >>>>             INSERT_FILT("transpose", "clock");
> >>>>         } else if (fabs(theta - 180) < 1.0) {
> >>>> @@ -1930,6 +1931,15 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
> >>>>             snprintf(rotate_buf, sizeof(rotate_buf), "%f*PI/180", theta);
> >>>>             INSERT_FILT("rotate", rotate_buf);
> >>>>         }
> >>>> +        if (!frame_rotation_checked) {
> >>>> +            frame_rotation_checked = 1;
> >>>> +            AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);
> >>>> +            if (sd) {
> >>>> +                displaymatrix = (int32_t *)sd->data;
> >>>> +                theta = get_rotation(displaymatrix);
> >>>> +                goto rotation;
> >>>> +            }
> >>>> +        }
> >>>>     }
> >>>>
> >>>>     if ((ret = configure_filtergraph(graph, vfilters, filt_src, last_filter)) < 0)
> >>>> --
> >>>> 2.33.0
> >>>>
> >>>> _______________________________________________
> >>>> 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".
> > _______________________________________________
> > 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".
_______________________________________________
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] [PATCH v2] fftools/ffplay: fix rotation incorrect when frame contains the displaymatrix

[1035567130]

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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 9242047f5c..5bda29169d 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1915,7 +1915,12 @@ 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);
+        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);
         double theta = get_rotation(displaymatrix);
 
         if (fabs(theta - 90) < 1.0) {
-- 
2.33.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH v2] fftools/ffplay: fix rotation incorrect when frame contains the displaymatrix

["zhilizhao(赵志立)"]

> On Sep 1, 2022, at 12:53 PM, 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
> 

I’m OK with this strategy.

> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
> fftools/ffplay.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 9242047f5c..5bda29169d 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1915,7 +1915,12 @@ 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);
> +        int32_t *displaymatrix = NULL;
> +        AVFrameSideData *sd = av_frame_get_side_data(frame,AV_FRAME_DATA_DISPLAYMATRIX);

Coding style: missing a space after comma.

> +        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);
>         double theta = get_rotation(displaymatrix);
> 
>         if (fabs(theta - 90) < 1.0) {

Mixed declaration and code.

> -- 
> 2.33.0
> 
> _______________________________________________
> 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".

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

[1035567130]

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");
-- 
2.33.0

_______________________________________________
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".

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

[Zhao Zhili]

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.

_______________________________________________
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".

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

[Steven Liu]

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
_______________________________________________
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".