wangyaqiang <[email protected]> 于2022年9月1日周四 12:05写道:
>
>
>
> > 2022年9月1日 上午9:24,Steven Liu <[email protected]> 写道:
> >
> > Steven Liu <[email protected]> 于2022年9月1日周四 09:17写道:
> >>
> >> Zhao Zhili <[email protected]> 于2022年9月1日周四 00:34写道:
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: [email protected] <[email protected]> 
> >>>> On Behalf Of [email protected]
> >>>> Sent: 2022年8月31日 18:45
> >>>> To: [email protected]
> >>>> Cc: Wang Yaqiang <[email protected]>; [email protected]
> >>>> Subject: [FFmpeg-devel] [PATCH] fftools/ffplay: fix rotation incorrect 
> >>>> when frame contains the side_data(type is
> >>>> AV_FRAME_DATA_DISPLAYMATRIX)
> >>>>
> >>>> From: Wang Yaqiang <[email protected]>
> >>>>
> >>>> 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
> >>>> [email protected]
> >>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>>
> >>>> To unsubscribe, visit link above, or email
> >>>> [email protected] with subject "unsubscribe".
> >>>
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> [email protected]
> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >>> To unsubscribe, visit link above, or email
> >>> [email protected] with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > [email protected]
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > [email protected] with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> [email protected]
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> [email protected] with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".

Reply via email to