[FFmpeg-devel] Export display matrix mirroring info as part of the rotate API

[Jun Li]

On Thu, May 16, 2019 at 7:38 PM Vittorio Giovara <vittorio.giovara at gmail.com>
wrote:

> On Thu, May 16, 2019 at 9:32 PM Jun Li <junli1026 at gmail.com> wrote:
>
> > On Thu, May 16, 2019 at 4:34 PM Ted Meyer <
> > tmathmeyer-at-google.com at ffmpeg.org> wrote:
> >
> > > Right now ffmpeg doesn't export a mirroring status when checking the
> > > display matrix for rotation.
> > > Here is an example video: https://files.tedm.io/flip.mp4
> > > -Ted
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
> >
> > There is a patch but not merged into master, quality not guaranteed :)
> > Hope this helps.
> > https://patchwork.ffmpeg.org/patch/13130/
> >
>
> hey i just noticed this patch
>
> +    if (CONV_FP(m[0]) * CONV_FP(m[4]) < CONV_FP(m[1]) *
> CONV_FP(m[3])) {+        *hflip = 1;+        av_display_matrix_flip(m,
> 1, 0);+    }
>
> the long if is basically computing the determinant of the matrix, but you
> only need the fact whether it's positive or negative, you can discard the
> result so you can avoid converting to CONV_FP, and just cast to int64_t
>
+    return av_display_rotation_get(m);
>
> don't you need to set vertical flip only if det < 0 and rot = 180?
>

I think it should be fine and not need to check. Because vlip could be
represented as hflip+rotation, the cross product (Michael's brilliant idea)
m[0] * m[4] - m[1]*[3]  cannot reflect it is horizontal or vertical flip.
It all depends how we want to represents the transform, it could be
"rotation+hflip"  or "rotation+vlip".    Any combination of rotation+flip
can be represented by either one.



> beside that small point, this patch introduces an api that basically
> supersedes the normal av_display_rotation_get(), and does many more things,
> I'd be tempted to deprecate any other use, and what do you think? in that
> case you could just call it av_display_rotation_get2() like is tradition

I can't find the other patches from the set to review, would you be able to
> send an updated version?
>

Thanks Vittorio, I updated the version as suggested:
https://patchwork.ffmpeg.org/patch/13181/

The original patch includes two part, but I am going to abandon it after
discussing with Nicolas.
The patches were for per frame flip/mirror and rotation. Basically it
checks every frame's meta-data and address flip or rotation or both.
I think it also covers stream level flip, addressed in ffmpeg_filter.c
(correct me if I am wrong).
https://patchwork.ffmpeg.org/patch/13131/
https://patchwork.ffmpeg.org/patch/13130/

Best Regards,
Jun


> thanks
> --
> Vittorio
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".