[FFmpeg-devel] [PATCH] Use tkhd matrix for proper display in mov

[John Schmiederer]

> > Attached is a patch to account for the transformation matrix contained in the tkhd atom for proper display width/height.
>
> > The video
> > http://samples.mplayerhq.hu/mov/tkhd_matrix/white_zombie_scrunch.mov
> > plays at 160x240 when it should really be scaled to 320x240.
>
> > +    int i;
> > +    int width;
> > +    int height;
> > +    float disp_transform[3];
> > +    float display_matrix[3][3];
>
> please use integers, there is no need for floats.
>
Ok, I changed display_matrix to int, but left disp_transform as float to avoid any possible overflow from multiplying large ints.

>
> >      AVStream *st = c->fc->streams[c->fc->nb_streams-1];
> >      int version = get_byte(pb);
> >
> > @@ -1402,12 +1407,38 @@
> >      get_be16(pb); /* volume */
> >      get_be16(pb); /* reserved */
> >
> > -    url_fskip(pb, 36); /* display matrix */
> > +    //read in the display matrix (outlined in ISO 14496-12, Section
> 6.2.2)
> > +    display_matrix[0][0] = (float)get_be32(pb)/(1<<16); //a
> > +    display_matrix[0][1] = (float)get_be32(pb)/(1<<16); //b
> > +    display_matrix[0][2] = (float)get_be32(pb)/(1<<30); //u
> > +    display_matrix[1][0] = (float)get_be32(pb)/(1<<16); //c
> > +    display_matrix[1][1] = (float)get_be32(pb)/(1<<16); //d
> > +    display_matrix[1][2] = (float)get_be32(pb)/(1<<30); //v
> > +    display_matrix[2][0] = (float)get_be32(pb)/(1<<16); //x
> > +    display_matrix[2][1] = (float)get_be32(pb)/(1<<16); //y
> > +    display_matrix[2][2] = (float)get_be32(pb)/(1<<30); //z
>
> for(i=0; i<3; i++)
>
> (and this loop can be merged with the next)
>
I put the reading in of the matrix into a separate loop.

>
> > +
>
> trailing whitespace is forbidden in svn
>
Fixed.

>
> > +    width = get_be32(pb);  /* track width */
> > +    height = get_be32(pb); /* track height */
> > +
> > +    //transform the display width/height according to the matrix
> > +    if (width && height) {
> > +      for (i=0; i<3; i++)
> > +        disp_transform[i] =
> > +          width * display_matrix[0][i] +
> > +          height * display_matrix[1][i] +
> > +          display_matrix[2][i];
> > +      disp_transform[0] /= disp_transform[2];
> > +      disp_transform[1] /= disp_transform[2];
> >
> > +      //the sample aspect ratio is new width/height divided by old
> width/height
> > +      av_reduce(&st->codec->sample_aspect_ratio.num,
> > +        &st->codec->sample_aspect_ratio.den,
> > +        disp_transform[0] * height,
> > +        disp_transform[1] * width,
> > +        INT_MAX);
>
> dividing both by disp_transform[2] does not change the value thus it is
> useless.
>
That's true -- anything related to the scaling factor has been removed.

Thanks for the review, an updated patch is attached.

-John

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ff_mov_tkhd_matrix2.diff
Type: application/octet-stream
Size: 1761 bytes
Desc: ff_mov_tkhd_matrix2.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080527/ccd6219f/attachment.obj>