On Fri, Oct 07, 2016 at 03:31:46PM -0400, 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 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 <vittorio.giov...@gmail.com>
> ---
> Updated according review.
> Vittorio
>
> libavformat/isom.h | 2 ++
> libavformat/mov.c | 63
> +++++++++++++++++++++++++++------
> tests/fate/mov.mak | 6 +++-
> tests/ref/fate/mov-movie-display-matrix | 10 ++++++
> 4 files changed, 70 insertions(+), 11 deletions(-)
> create mode 100644 tests/ref/fate/mov-movie-display-matrix
>
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 2246fed..2aeb8fa 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -238,6 +238,8 @@ 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 a15c8d1..307ce08 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -1211,6 +1211,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 */
> @@ -1238,7 +1239,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 */
> @@ -3798,9 +3804,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 int64_t
> +#define CONV_FP2INT(x, sh) ((int64_t) (x)) / (1 << sh)
> +
> +// int64_t to fixed point
> +#define CONV_INT2FP(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];
> @@ -3855,13 +3876,7 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
>
> // 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;
> + if (IS_MATRIX_FULL(display_matrix)) {
> double rotate;
>
> av_freep(&sc->display_matrix);
> @@ -3884,13 +3899,41 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext
> *pb, MOVAtom atom)
> }
> }
>
> + // 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
> + int64_t 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_FP2INT(display_matrix[i][e], sh) *
> + CONV_FP2INT(c->movie_display_matrix[e][j],
> sh);
This does not work (you are dividing the 32bit numbers down to 2 bit)
also its not tested by the fate testcase
i can just replace it by 0 and fate-mov-movie-display-matrix still
passes
the macros also lack some () protection giving probably unintended
results
i dont mind fixing it myself to work with int64 but i need a testcase
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
