[FFmpeg-devel] [PATCH 2/3] avformat/mov: use av_display_rotation_get() for rotate metadata.

[Michael Niedermayer]

On Tue, May 20, 2014 at 08:24:27PM +0200, Clément Bœsch wrote:
> On Tue, May 20, 2014 at 07:59:06PM +0200, Clément Bœsch wrote:
> > ---
> >  libavformat/mov.c | 29 +++++++++++++----------------
> >  1 file changed, 13 insertions(+), 16 deletions(-)
> > 
> 
> Simpler version attached.
> 
> -- 
> Clément B.

>  mov.c |   30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 4d6d5c60fb35687fb1df912f9b6ae9548be4edef  0002-avformat-mov-use-av_display_rotation_get-for-rotate-.patch
> From ab965828ba39b3b26a1095b1578c322882fed0c6 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <u at pkh.me>
> Date: Tue, 20 May 2014 19:51:27 +0200
> Subject: [PATCH 2/3] avformat/mov: use av_display_rotation_get() for rotate
>  metadata.
> 
> ---
>  libavformat/mov.c | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 26fb0ed..31f19b3 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -31,6 +31,7 @@
>  
>  #include "libavutil/attributes.h"
>  #include "libavutil/channel_layout.h"
> +#include "libavutil/display.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/intfloat.h"
>  #include "libavutil/mathematics.h"
> @@ -2610,22 +2611,8 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      sc->width = width >> 16;
>      sc->height = height >> 16;
>  
> -    //Assign clockwise rotate values based on transform matrix so that
> -    //we can compensate for iPhone orientation during capture.
> -
> -    if (display_matrix[1][0] == -65536 && display_matrix[0][1] == 65536) {
> -         av_dict_set(&st->metadata, "rotate", "90", 0);
> -    }
> -
> -    if (display_matrix[0][0] == -65536 && display_matrix[1][1] == -65536) {
> -         av_dict_set(&st->metadata, "rotate", "180", 0);
> -    }
> -
> -    if (display_matrix[1][0] == 65536 && display_matrix[0][1] == -65536) {
> -         av_dict_set(&st->metadata, "rotate", "270", 0);
> -    }
> -
> -    // save the matrix when it is not the default identity
> +    // save the matrix and add rotate metadata when it is not the default
> +    // identity
>      if (display_matrix[0][0] != (1 << 16) ||
>          display_matrix[1][1] != (1 << 16) ||
>          display_matrix[2][2] != (1 << 30) ||
> @@ -2633,6 +2620,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          display_matrix[1][0] || display_matrix[1][2] ||
>          display_matrix[2][0] || display_matrix[2][1]) {
>          int i, j;
> +        double rotate;
>  
>          av_freep(&sc->display_matrix);
>          sc->display_matrix = av_malloc(sizeof(int32_t) * 9);
> @@ -2642,6 +2630,16 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          for (i = 0; i < 3; i++)
>              for (j = 0; j < 3; j++)
>                  sc->display_matrix[i * 3 + j] = display_matrix[j][i];
> +
> +        rotate = av_display_rotation_get(sc->display_matrix);

> +        if (rotate != NAN) {

i dont think this is a valid way to check for nan
(for example it makes the assumtation that there is just one NAN)
but i didnt check IEEE/C99 specs, just feels wrong ...

patch LGTM otherwise

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140520/d68eb02b/attachment.asc>