[FFmpeg-devel] [PATCHv5] mov: Evaluate the movie display matrix

James Almer jamrial at gmail.com
Thu Nov 17 20:02:23 EET 2016


On 11/15/2016 1:14 PM, Vittorio Giovara wrote:
> This matrix needs to be applied after all others have (currently only
> display matrix from trak), but cannot be handled in movie box, since
> streams are not allocated yet. So store it in main context, and apply
> it when appropriate, that is after parsing the tkhd one.
> 
> Signed-off-by: Vittorio Giovara <vittorio.giovara at gmail.com>
> ---
> Fixed a boolean operation. No other changes, ready to be committed.
> Please CC.
> Vittorio
> 
>  libavformat/isom.h               |  1 +
>  libavformat/mov.c                | 48 +++++++++++++++++++++++++++++-----------
>  tests/fate/mov.mak               |  5 ++++-
>  tests/ref/fate/mov-displaymatrix | 10 +++++++++
>  4 files changed, 50 insertions(+), 14 deletions(-)
>  create mode 100644 tests/ref/fate/mov-displaymatrix
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index d684502..02bfedd 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -240,6 +240,7 @@ typedef struct MOVContext {
>      uint8_t *decryption_key;
>      int decryption_key_len;
>      int enable_drefs;
> +    int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd
>  } MOVContext;
>  
>  int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8d6cc12..b7d0b12 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1239,6 +1239,7 @@ static int mov_read_mdhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  
>  static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  {
> +    int i;
>      int64_t creation_time;
>      int version = avio_r8(pb); /* version */
>      avio_rb24(pb); /* flags */
> @@ -1269,7 +1270,12 @@ static int mov_read_mvhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  
>      avio_skip(pb, 10); /* reserved */
>  
> -    avio_skip(pb, 36); /* display matrix */
> +    /* movie display matrix, store it in main context and use it later on */
> +    for (i = 0; i < 3; i++) {
> +        c->movie_display_matrix[i][0] = avio_rb32(pb); // 16.16 fixed point
> +        c->movie_display_matrix[i][1] = avio_rb32(pb); // 16.16 fixed point
> +        c->movie_display_matrix[i][2] = avio_rb32(pb); //  2.30 fixed point
> +    }
>  
>      avio_rb32(pb); /* preview time */
>      avio_rb32(pb); /* preview duration */
> @@ -3847,12 +3853,22 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      return 0;
>  }
>  
> +// return 1 when matrix is identity, 0 otherwise
> +#define IS_MATRIX_IDENT(matrix)            \
> +    ( (matrix)[0][0] == (1 << 16) &&       \
> +      (matrix)[1][1] == (1 << 16) &&       \
> +      (matrix)[2][2] == (1 << 30) &&       \
> +     !(matrix)[0][1] && !(matrix)[0][2] && \
> +     !(matrix)[1][0] && !(matrix)[1][2] && \
> +     !(matrix)[2][0] && !(matrix)[2][1])
> +
>  static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  {
> -    int i;
> +    int i, j, e;
>      int width;
>      int height;
>      int display_matrix[3][3];
> +    int res_display_matrix[3][3] = { { 0 } };
>      AVStream *st;
>      MOVStreamContext *sc;
>      int version;
> @@ -3902,15 +3918,20 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      sc->width = width >> 16;
>      sc->height = height >> 16;
>  
> -    // 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) ||
> -        display_matrix[0][1] || display_matrix[0][2] ||
> -        display_matrix[1][0] || display_matrix[1][2] ||
> -        display_matrix[2][0] || display_matrix[2][1]) {
> -        int i, j;
> +    // apply the moov display matrix (after the tkhd one)
> +    for (i = 0; i < 3; i++) {
> +        const int sh[3] = { 16, 16, 30 };
> +        for (j = 0; j < 3; j++) {
> +            for (e = 0; e < 3; e++) {
> +                res_display_matrix[i][j] +=
> +                    ((int64_t) display_matrix[i][e] *
> +                     c->movie_display_matrix[e][j]) >> sh[e];
> +            }
> +        }
> +    }
> +
> +    // save the matrix when it is not the default identity
> +    if (!IS_MATRIX_IDENT(res_display_matrix)) {
>          double rotate;
>  
>          av_freep(&sc->display_matrix);
> @@ -3920,7 +3941,7 @@ 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[i][j];
> +                sc->display_matrix[i * 3 + j] = res_display_matrix[i][j];
>  
>          rotate = av_display_rotation_get(sc->display_matrix);
>          if (!isnan(rotate)) {
> @@ -3939,7 +3960,8 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          double disp_transform[2];
>  
>          for (i = 0; i < 2; i++)
> -            disp_transform[i] = hypot(display_matrix[i][0], display_matrix[i][1]);
> +            disp_transform[i] = hypot(sc->display_matrix[0 + i],
> +                                      sc->display_matrix[3 + i]);
>  
>          if (disp_transform[0] > 0       && disp_transform[1] > 0 &&
>              disp_transform[0] < (1<<24) && disp_transform[1] < (1<<24) &&
> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> index bb02d6b..d7ed580 100644
> --- a/tests/fate/mov.mak
> +++ b/tests/fate/mov.mak
> @@ -7,7 +7,8 @@ FATE_MOV = fate-mov-3elist \
>             fate-mov-2elist-elist1-ends-bframe \
>             fate-mov-zombie \
>             fate-mov-aac-2048-priming \
> -           fate-mp4-init-nonkeyframe
> +           fate-mp4-init-nonkeyframe \
> +           fate-mov-displaymatrix \
>  
>  FATE_SAMPLES_AVCONV += $(FATE_MOV)
>  
> @@ -38,3 +39,5 @@ fate-mov-zombie: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packe
>  
>  fate-mp4-init-nonkeyframe: ffprobe$(PROGSSUF)$(EXESUF)
>  fate-mp4-init-nonkeyframe: CMD = run ffprobe$(PROGSSUF)$(EXESUF) -show_packets -print_format compact -select_streams v $(TARGET_SAMPLES)/mov/mp4-init-nonkeyframe.mp4
> +
> +fate-mov-displaymatrix: CMD = framemd5 -i $(TARGET_SAMPLES)/mov/displaymatrix.mov -frames 1
> diff --git a/tests/ref/fate/mov-displaymatrix b/tests/ref/fate/mov-displaymatrix
> new file mode 100644
> index 0000000..52528c1
> --- /dev/null
> +++ b/tests/ref/fate/mov-displaymatrix
> @@ -0,0 +1,10 @@
> +#format: frame checksums
> +#version: 2
> +#hash: MD5
> +#tb 0: 1001/30000
> +#media_type 0: video
> +#codec_id 0: rawvideo
> +#dimensions 0: 240x160
> +#sar 0: 2/1
> +#stream#, dts,        pts, duration,     size, hash
> +0,          0,          0,        1,    57600, be949aa661551010f461069804f68e76
> 

The display matrix doesn't seem to affect decoding with ffmpeg so this
isn't really testing anything other than h264 decoding.
You could instead use ffprobe -show_streams, which will print side data
information.

For this file it would print:

[SIDE_DATA]
side_data_type=Display Matrix
side_data_size=36
displaymatrix=
00000000:            0      131072           0
00000001:       -65536           0           0
00000002:     47185920           0  1073741824

No comments about the mov.c changes. Will let someone else review that.



More information about the ffmpeg-devel mailing list