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

[wangyaqiang <1035567130@qq.com>]

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