On Aug 10, 2007, at 4:23 PM, Robert Swain wrote: > On 10/08/07, Aurelien Jacobs <[EMAIL PROTECTED]> wrote: > >> On Fri, 10 Aug 2007 11:08:04 +0100 >> "Robert Swain" <[EMAIL PROTECTED]> wrote: >> >> >>> On 10/08/07, Aurelien Jacobs <[EMAIL PROTECTED]> wrote: >>> >>>> On Fri, 10 Aug 2007 01:37:12 +0200 (CEST) >>>> conrad <[EMAIL PROTECTED]> wrote: >>>> >>>> >>>>> Author: conrad >>>>> Date: Fri Aug 10 01:37:12 2007 >>>>> New Revision: 656 >>>>> >>>>> Log: >>>>> Write the display size elements >>>>> >>>>> >>>>> Modified: >>>>> matroska/matroskaenc.c >>>>> >>>>> Modified: matroska/matroskaenc.c >>>>> ================================================================== >>>>> ============ >>>>> --- matroska/matroskaenc.c (original) >>>>> +++ matroska/matroskaenc.c Fri Aug 10 01:37:12 2007 >>>>> @@ -476,7 +476,10 @@ static int mkv_write_tracks(AVFormatCont >>>>> // XXX: interlace flag? >>>>> put_ebml_uint (pb, >>>>> MATROSKA_ID_VIDEOPIXELWIDTH , codec->width); >>>>> put_ebml_uint (pb, >>>>> MATROSKA_ID_VIDEOPIXELHEIGHT, codec->height); >>>>> - // XXX: display width/height >>>>> + if (codec->sample_aspect_ratio.num) { >>>>> + put_ebml_uint(pb, >>>>> MATROSKA_ID_VIDEODISPLAYWIDTH , codec->sample_aspect_ratio.num); >>>>> + put_ebml_uint(pb, >>>>> MATROSKA_ID_VIDEODISPLAYHEIGHT, codec->sample_aspect_ratio.den); >>>>> >>>> >>>> I'm not sure, but I suppose that sample_aspect_ratio is something >>>> different than videodisplaywidth/height. >>>> I guess that you could have sample_aspect_ratio = 4/3 when >>>> videodisplaywidth/height = 640/480. >>>> Anyone can confirm? >>>> >>> >>> Sample aspect ratio is also known as pixel aspect ratio. >>> >>> DAR = display width / display height >>> SAR = sample width / sample height >>> >>> DAR = SAR * encoded frame width / encoded frame height >>> >>> Computer screens generally have SAR = 1 (i.e. square pixels). As I >>> recall, TV screens do not. DVDs have SARs calculated from the >>> encoded >>> frame width and height to obtain the correct display aspect ratio >>> (either 4/3 or 16/9). >>> >>> Anyway, these values should take whatever values FFmpeg produces >>> in my >>> opinion, so that commit looks correct. >>> >> >> I agree that SAR values produced by ffmpeg should be right. But the >> values stored in the mkv file are in fact the DAR (well, the display >> width and height). >> So storing the SAR here still seems plain wrong to me ! >> > > Oops, I overlooked the MATROSKA_ID_VIDEODISPLAYWIDTH etc. :) You're > correct, the commit is wrong, sorry for the nouse. The DAR should be > calculated as mentioned and used instead.
I think I fixed this correctly now. _______________________________________________ FFmpeg-soc mailing list FFmpeg-soc@mplayerhq.hu http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc