[FFmpeg-devel] [PATCH] mov tkhd' width and height usage

[Baptiste Coudurier]

Hi Maksym,

On 02/14/2011 01:05 AM, Maksym Veremeyenko wrote:
> Baptiste Coudurier ???????(??):
>> On 2/13/11 6:29 AM, Maksym Veremeyenko wrote:
>>> Benjamin Larsson ???????(??):
>>> [...]
>>>> clef and enof should be written. Flip writes those atoms. There is
>>>> another patch on the mailinglist to use these atoms for DAR and PAR
>>>> calculation. It would make sense to merge both of these patches.
>>> updated patches attached.
>>
>> tapt stuff is only valid in mov currently, and it only needs to be
>> written when sample
>> aspect ratio is not 1.
>>
>
> patch updated.
>
>
> 0001-use-tapt-atom-for-sample-aspect-ratio.patch
>
>
>  From 1b939e434c77a10e54658f282791890776e04d3b Mon Sep 17 00:00:00 2001
> From: Maksym Veremeyenko<verem at m1stereo.tv>
> Date: Mon, 14 Feb 2011 10:52:06 +0200
> Subject: [PATCH 1/2] use tapt atom for sample aspect ratio
>
> ---
>   libavformat/movenc.c |   48 +++++++++++++++++++++++++++++++++++++++++++-----
>   1 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 5e6e3a6..b05fcf0 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -1215,11 +1215,8 @@ static int mov_write_tkhd_tag(ByteIOContext *pb, MOVTrack *track, AVStream *st)
>       /* Track width and height, for visual only */
>       if(st&&  (track->enc->codec_type == AVMEDIA_TYPE_VIDEO ||
>                 track->enc->codec_type == AVMEDIA_TYPE_SUBTITLE)) {
> -        double sample_aspect_ratio = av_q2d(st->sample_aspect_ratio);
> -        if(!sample_aspect_ratio || track->height != track->enc->height)
> -            sample_aspect_ratio = 1;
> -        put_be32(pb, sample_aspect_ratio * track->enc->width*0x10000);
> -        put_be32(pb, track->height*0x10000);
> +        put_be32(pb, track->enc->width<<  16);
> +        put_be32(pb, track->height<<  16);

So, did you confirm that this works for mp4 ? Otherwise we need to keep 
the old logic in that case.

>       }
>       else {
>           put_be32(pb, 0);
> @@ -1228,6 +1225,42 @@ static int mov_write_tkhd_tag(ByteIOContext *pb, MOVTrack *track, AVStream *st)
>       return 0x5c;
>   }
>
> +static int mov_write_clef_tag(ByteIOContext *pb, MOVTrack *track)
> +{
> +    int32_t width;
> +
> +    width = av_q2d(track->enc->sample_aspect_ratio) * track->enc->width;

Please avoid using doubles, use av_rescale.

> +
> +    put_be32(pb, 0x14);

Use sizes in decimal.

> +    put_tag(pb, "clef");
> +    put_be32(pb, 0);
> +    put_be32(pb, width<<  16);
> +    put_be32(pb, track->height<<  16);
> +
> +    return 0x14;
> +};
> +
> +static int mov_write_enof_tag(ByteIOContext *pb, MOVTrack *track)
> +{
> +    put_be32(pb, 0x14);
> +    put_tag(pb, "enof");
> +    put_be32(pb, 0);
> +    put_be32(pb, track->enc->width<<  16);
> +    put_be32(pb, track->enc->height<<  16);
> +
> +    return 0x14;
> +};
> +
> +static int mov_write_tapt_tag(ByteIOContext *pb, MOVTrack *track)
> +{
> +    int64_t pos = url_ftell(pb);
> +    put_be32(pb, 0); /* size */
> +    put_tag(pb, "tapt");
> +    mov_write_clef_tag(pb, track);
> +    mov_write_enof_tag(pb, track);
> +    return updateSize(pb, pos);
> +};

Maybe inlining the enof and clef atoms in tapt is simpler, but I don't 
care much about that, do as you prefer.

[...]

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org