Re: [libav-devel] [PATCH] mov: Evaluate the movie display matrix

[Vittorio Giovara]

On Mon, Oct 24, 2016 at 5:46 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.
>
> Fate tests are updated accordingly.
>
> Signed-off-by: Vittorio Giovara 
> ---

any more feedback about this?
--
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] mov: Evaluate the movie display matrix

[Vittorio Giovara]

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.

Fate tests are updated accordingly.

Signed-off-by: Vittorio Giovara 
---
Now with the correct variable declaration.
Forgot to mention the macro is switched to check for identity rather than 
fullness.
Vittorio
 libavformat/isom.h|  2 ++
 libavformat/mov.c | 49 ---
 tests/ref/fate/mov-dar|  2 +-
 tests/ref/fate/mov-display-matrix |  2 +-
 tests/ref/fate/mov-sar|  2 +-
 5 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 58f0a20..1aa2091 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -167,6 +167,8 @@ typedef struct MOVContext {
 int export_all;
 int export_xmp;
 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 fee9f36..9e1640a 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -951,6 +951,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;
 time_t creation_time;
 int version = avio_r8(pb); /* version */
 avio_rb24(pb); /* flags */
@@ -974,7 +975,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 */
@@ -2751,13 +2757,23 @@ 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;
 int64_t disp_transform[2];
 int display_matrix[3][3];
+int res_display_matrix[3][3] = { { 0 } };
 AVStream *st;
 MOVStreamContext *sc;
 int version;
@@ -2807,15 +2823,20 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 sc->width = width >> 16;
 sc->height = height >> 16;
 
-// save the matrix 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)) {
 av_freep(>display_matrix);
 sc->display_matrix = av_malloc(sizeof(int32_t) * 9);
 if (!sc->display_matrix)
@@ -2823,7 +2844,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];
 }
 
 // transform the display width/height according to the matrix
@@ -2832,9 +2853,9 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 if (width && height && sc->display_matrix) {
 for (i = 0; i < 2; i++)
 disp_transform[i] =
-(int64_t)  width  * display_matrix[0][i] +
-(int64_t)  height * display_matrix[1][i] +
-((int64_t) display_matrix[2][i] << 16);
+

[libav-devel] [PATCH] mov: Evaluate the movie display matrix

[Vittorio Giovara]

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 if not identity, apply it when appropriate,
handling the case when trak display matrix is identity and when it is not.

Signed-off-by: Vittorio Giovara 
---
 libavformat/isom.h |  2 ++
 libavformat/mov.c  | 64 --
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 58f0a20..1aa2091 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -167,6 +167,8 @@ typedef struct MOVContext {
 int export_all;
 int export_xmp;
 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 00ecdc7..a5609d3 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -950,6 +950,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;
 time_t creation_time;
 int version = avio_r8(pb); /* version */
 avio_rb24(pb); /* flags */
@@ -973,7 +974,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 */
@@ -2747,9 +2753,24 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 return 0;
 }
 
+// return 0 when matrix is identity, 1 otherwise
+#define IS_MATRIX_FULL(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])
+
+// fixed point to double
+#define CONV_FP(x, sh) ((double) (x)) / (1 << sh)
+
+// double to fixed point
+#define CONV_DB(x, sh) (int32_t) ((x) * (1 << sh))
+
 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];
@@ -2803,14 +2824,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 sc->height = height >> 16;
 
 // save the matrix 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;
-
+if (IS_MATRIX_FULL(display_matrix)) {
 av_freep(>display_matrix);
 sc->display_matrix = av_malloc(sizeof(int32_t) * 9);
 if (!sc->display_matrix)
@@ -2821,13 +2835,41 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext 
*pb, MOVAtom atom)
 sc->display_matrix[i * 3 + j] = display_matrix[i][j];
 }
 
+// if movie display matrix is not identity, and if this is a video track
+if (IS_MATRIX_FULL(c->movie_display_matrix) && width && height) {
+// if trak display matrix was identity, just copy the movie one
+if (!sc->display_matrix) {
+sc->display_matrix = av_malloc(sizeof(int32_t) * 9);
+if (!sc->display_matrix)
+return AVERROR(ENOMEM);
+
+for (i = 0; i < 3; i++)
+for (j = 0; j < 3; j++)
+sc->display_matrix[i * 3 + j] = 
c->movie_display_matrix[i][j];
+} else { // otherwise multiply the two and store the result
+double val = 0;
+for (i = 0; i < 3; i++) {
+for (j = 0; j < 3; j++) {
+int sh = j == 2 ? 30 : 16;
+for (e = 0; e < 3; e++) {
+val += CONV_FP(display_matrix[i][e], sh) *
+   CONV_FP(c->movie_display_matrix[e][j], sh);
+}
+sc->display_matrix[i * 3 + j] = CONV_DB(val, sh);
+val = 0;
+}
+}
+}
+}
+
 // transform the display width/height according to the matrix
 // skip this when the display matrix is the identity one
 if (width && height && sc->display_matrix) {
 double disp_transform[2];
 
 for (i =