Message ID | 20161115161406.85214-1-vittorio.giovara@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Tue, Nov 15, 2016 at 11:14 AM, Vittorio Giovara <vittorio.giovara@gmail.com> 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@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 > -- > 2.10.0 > ping
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@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.
On Thu, Nov 17, 2016 at 1:02 PM, James Almer <jamrial@gmail.com> wrote: > On 11/15/2016 1:14 PM, Vittorio Giovara wrote: >> 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. The `sar` field is affected, but yeah it's probably better to use ffprobe -show_streams. Thanks,
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
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@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