diff mbox series

[FFmpeg-devel] fftools/ffplay.c: fix rotate error when displaymatrix contains flip

Message ID tencent_8EF931E4C9CD28C5F581EF6B082F194D6505@qq.com
State New
Headers show
Series [FFmpeg-devel] fftools/ffplay.c: fix rotate error when displaymatrix contains flip | 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 March 9, 2023, 9:49 a.m. UTC
From: Wang Yaqiang <wangyaqiang03@kuaishou.com>

The angle is calculated by the get_rotation function,
but it needs to be combined with the displaymatrix to determine whether flip or rotate,
which is consistent with the processing logic in ffmpeg_filter.c

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

Comments

Leo Izen March 9, 2023, 2 p.m. UTC | #1
On 3/9/23 04:49, 1035567130@qq.com wrote:
> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> 
> The angle is calculated by the get_rotation function,
> but it needs to be combined with the displaymatrix to determine whether flip or rotate,
> which is consistent with the processing logic in ffmpeg_filter.c
> 
> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
>   fftools/ffplay.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index d6479aef5f..860bac367a 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1925,16 +1925,24 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>           theta = get_rotation(displaymatrix);
>   
>           if (fabs(theta - 90) < 1.0) {
> -            INSERT_FILT("transpose", "clock");
> +            INSERT_FILT("transpose", displaymatrix[3] > 0 ? "cclock_flip" : "clock");
>           } else if (fabs(theta - 180) < 1.0) {
> -            INSERT_FILT("hflip", NULL);
> -            INSERT_FILT("vflip", NULL);
> +            if (displaymatrix[0] < 0) {
> +                INSERT_FILT("hflip", NULL);
> +            }
> +            if (displaymatrix[4] < 0) {
> +                INSERT_FILT("vflip", NULL);
> +            }
Minor code style point, we don't use {} if there's only one line inside 
the braces.


>           } else if (fabs(theta - 270) < 1.0) {
> -            INSERT_FILT("transpose", "cclock");
> +            INSERT_FILT("transpose", displaymatrix[3] < 0 ? "clock_flip" : "cclock");
>           } 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);
> +        } else if (fabs(theta) < 1.0) {
> +            if (displaymatrix && displaymatrix[4] < 0) { > +                INSERT_FILT("vflip", NULL);
> +            }
>           }
>       }
>   

Otherwise looks mathematically sound.

- Leo Izen (thebombzen)
Marton Balint March 9, 2023, 8:27 p.m. UTC | #2
On Thu, 9 Mar 2023, 1035567130@qq.com wrote:

> From: Wang Yaqiang <wangyaqiang03@kuaishou.com>
>
> The angle is calculated by the get_rotation function,
> but it needs to be combined with the displaymatrix to determine whether flip or rotate,
> which is consistent with the processing logic in ffmpeg_filter.c

If there is common logic, then it should be factorized to a shared 
function and both ffmpeg and ffplay should use that.

Regards,
Marton

>
> Signed-off-by: Wang Yaqiang <wangyaqiang03@kuaishou.com>
> ---
> fftools/ffplay.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index d6479aef5f..860bac367a 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1925,16 +1925,24 @@ static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>         theta = get_rotation(displaymatrix);
>
>         if (fabs(theta - 90) < 1.0) {
> -            INSERT_FILT("transpose", "clock");
> +            INSERT_FILT("transpose", displaymatrix[3] > 0 ? "cclock_flip" : "clock");
>         } else if (fabs(theta - 180) < 1.0) {
> -            INSERT_FILT("hflip", NULL);
> -            INSERT_FILT("vflip", NULL);
> +            if (displaymatrix[0] < 0) {
> +                INSERT_FILT("hflip", NULL);
> +            }
> +            if (displaymatrix[4] < 0) {
> +                INSERT_FILT("vflip", NULL);
> +            }
>         } else if (fabs(theta - 270) < 1.0) {
> -            INSERT_FILT("transpose", "cclock");
> +            INSERT_FILT("transpose", displaymatrix[3] < 0 ? "clock_flip" : "cclock");
>         } 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);
> +        } else if (fabs(theta) < 1.0) {
> +            if (displaymatrix && displaymatrix[4] < 0) {
> +                INSERT_FILT("vflip", NULL);
> +            }
>         }
>     }
>
> -- 
> 2.39.2
>
> _______________________________________________
> 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".
>
diff mbox series

Patch

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index d6479aef5f..860bac367a 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1925,16 +1925,24 @@  static int configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
         theta = get_rotation(displaymatrix);
 
         if (fabs(theta - 90) < 1.0) {
-            INSERT_FILT("transpose", "clock");
+            INSERT_FILT("transpose", displaymatrix[3] > 0 ? "cclock_flip" : "clock");
         } else if (fabs(theta - 180) < 1.0) {
-            INSERT_FILT("hflip", NULL);
-            INSERT_FILT("vflip", NULL);
+            if (displaymatrix[0] < 0) {
+                INSERT_FILT("hflip", NULL);
+            }
+            if (displaymatrix[4] < 0) {
+                INSERT_FILT("vflip", NULL);
+            }
         } else if (fabs(theta - 270) < 1.0) {
-            INSERT_FILT("transpose", "cclock");
+            INSERT_FILT("transpose", displaymatrix[3] < 0 ? "clock_flip" : "cclock");
         } 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);
+        } else if (fabs(theta) < 1.0) {
+            if (displaymatrix && displaymatrix[4] < 0) {
+                INSERT_FILT("vflip", NULL);
+            }
         }
     }