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.giov...@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel