[FFmpeg-soc] [soc]: r656 - matroska/matroskaenc.c

[Aurelien Jacobs]

On Fri, 10 Aug 2007 11:08:04 +0100
"Robert Swain" <robert.swain at gmail.com> wrote:

> On 10/08/07, Aurelien Jacobs <aurel at gnuage.org> wrote:
> > On Fri, 10 Aug 2007 01:37:12 +0200 (CEST)
> > conrad <subversion at mplayerhq.hu> 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 !

Aurel