Re: [FFmpeg-devel] [PATCH v2 1/2] lavu/display: add av_display_rotation_hflip_get and bump version

2019-05-16 Thread Jun Li
On Wed, May 15, 2019 at 3:22 PM Michael Niedermayer 
wrote:

> On Tue, May 14, 2019 at 10:36:44PM -0700, Jun Li wrote:
> > Add av_display_rotation_hflip_get to get information whether the
> > matrix indicates horizontal flip.
> > ---
> >  doc/APIchanges|  3 +++
> >  libavutil/display.c   | 14 ++
> >  libavutil/display.h   | 14 ++
> >  libavutil/tests/display.c |  8 
> >  libavutil/version.h   |  2 +-
> >  tests/ref/fate/display|  4 
> >  6 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index e75c4044ce..73c6809563 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> >
> >  API changes, most recent first:
> >
> > +2019-05-13 - XX - lavu 56.28.100 - display.h
> > +  Add av_display_rotation_hflip_get
> > +
> >  2019-04-20 - 3153a6502a - lavc 58.52.100 - avcodec.h
> >Add AV_CODEC_FLAG_DROPCHANGED to allow avcodec_receive_frame to drop
> >frames whose parameters differ from first decoded frame in stream.
> > diff --git a/libavutil/display.c b/libavutil/display.c
> > index a0076e067b..f5a6a4a08d 100644
> > --- a/libavutil/display.c
> > +++ b/libavutil/display.c
> > @@ -71,3 +71,17 @@ void av_display_matrix_flip(int32_t matrix[9], int
> hflip, int vflip)
> >  for (i = 0; i < 9; i++)
> >  matrix[i] *= flip[i % 3];
> >  }
> > +
> > +double av_display_rotation_hflip_get(const int32_t matrix[9], int
> *hflip)
> > +{
> > +int32_t m[9];
> > +*hflip = 0;
> > +memcpy(m, matrix, sizeof(m));
> > +
>
> > +if (m[0] > 0 && m[4] < 0 || m[0] < 0 && m[4] > 0 ||
> > +m[1] > 0 && m[3] > 0 || m[1] < 0 && m[3] < 0) {
> > +*hflip = 1;
> > +av_display_matrix_flip(m, 1, 0);
> > +}
>
> This is not correct
> consider the identity transform matrix
> 1   0
> 0   1
>
> now if the 0 elements are only so slightly off 0 this could set hflip, this
> does not make the matrix flip anything, its still basically a identity
> matrix.
>
> What you want to detect here, i _think_, is the direction in which the
> matrix
> rotates the vector. Not sure what the technical term is but the "z" sign of
> the cross product of the 2 basis vectors should be that if iam not
> mistaken.
>
> I think that results in something like m[0]*m[4] < m[1]*m[3] but ive not
> checked this so please check before using it
>

Thanks Michael for review, it is very helpful.
Yes, using cross product should be the right way. Negative value of
m[0]*m[4] - m[1]*m[3]  indicates flipped (the vector points into the
screen).
I tested on the content you shared, "47j9R7PXBep.mov" and "bug.jpg", also
on some exif jpg images from the link in the tickets. All works well.
Let me know if you have any more content need me to test. I have the
updated version:
https://patchwork.ffmpeg.org/patch/13130/
https://patchwork.ffmpeg.org/patch/13131/
Thanks!

Best Regards,
Jun

> thx
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who would give up essential Liberty, to purchase a little
> temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] lavu/display: add av_display_rotation_hflip_get and bump version

2019-05-15 Thread Michael Niedermayer
On Tue, May 14, 2019 at 10:36:44PM -0700, Jun Li wrote:
> Add av_display_rotation_hflip_get to get information whether the
> matrix indicates horizontal flip.
> ---
>  doc/APIchanges|  3 +++
>  libavutil/display.c   | 14 ++
>  libavutil/display.h   | 14 ++
>  libavutil/tests/display.c |  8 
>  libavutil/version.h   |  2 +-
>  tests/ref/fate/display|  4 
>  6 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index e75c4044ce..73c6809563 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>  
>  API changes, most recent first:
>  
> +2019-05-13 - XX - lavu 56.28.100 - display.h
> +  Add av_display_rotation_hflip_get
> +
>  2019-04-20 - 3153a6502a - lavc 58.52.100 - avcodec.h
>Add AV_CODEC_FLAG_DROPCHANGED to allow avcodec_receive_frame to drop
>frames whose parameters differ from first decoded frame in stream.
> diff --git a/libavutil/display.c b/libavutil/display.c
> index a0076e067b..f5a6a4a08d 100644
> --- a/libavutil/display.c
> +++ b/libavutil/display.c
> @@ -71,3 +71,17 @@ void av_display_matrix_flip(int32_t matrix[9], int hflip, 
> int vflip)
>  for (i = 0; i < 9; i++)
>  matrix[i] *= flip[i % 3];
>  }
> +
> +double av_display_rotation_hflip_get(const int32_t matrix[9], int *hflip)
> +{
> +int32_t m[9];
> +*hflip = 0;
> +memcpy(m, matrix, sizeof(m));
> +

> +if (m[0] > 0 && m[4] < 0 || m[0] < 0 && m[4] > 0 ||
> +m[1] > 0 && m[3] > 0 || m[1] < 0 && m[3] < 0) {
> +*hflip = 1;
> +av_display_matrix_flip(m, 1, 0);
> +}

This is not correct
consider the identity transform matrix
1   0
0   1

now if the 0 elements are only so slightly off 0 this could set hflip, this
does not make the matrix flip anything, its still basically a identity matrix.

What you want to detect here, i _think_, is the direction in which the matrix
rotates the vector. Not sure what the technical term is but the "z" sign of
the cross product of the 2 basis vectors should be that if iam not mistaken.

I think that results in something like m[0]*m[4] < m[1]*m[3] but ive not
checked this so please check before using it 

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".