[Ffmpeg-cvslog] CVS: ffmpeg/libavformat matroska.c,1.21,1.22

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> Hi
>
> On Wed, Mar 22, 2006 at 10:41:58PM +0000, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > Hi
>> >
>> > On Wed, Mar 22, 2006 at 07:11:09PM +0000, M?ns Rullg?rd wrote:
>> >> michael at mplayerhq.hu (Michael Niedermayer CVS) writes:
>> >> 
>> >> > +                av_reduce(&st->codec->sample_aspect_ratio.num,
>> >> > +                          &st->codec->sample_aspect_ratio.den,
>> >> > +                          st->codec->height * videotrack->display_width,
>> >> > +                          st->codec-> width * videotrack->display_height,
>> >> > +                          255);
>> >> 
>> >> Why do you set the limit at 255?  I don't have any videos that require
>> >> a higher limit, but I see no reason to restrict it either.
>> >
>> > h263 and mpeg4 use 8/8bits to store custom ratios so more would cause
>> > problems for them
>> 
>> Wouldn't it be better to have limited codecs do the truncation
>> themselves?  Demuxers shouldn't be imposing arbitrary limits.
>
> its simpler that way, and you could also argue that muxers&encoders
> should not round values behind the users back but instead fail and give
> the user a chance to make the best choice

Now the demuxer is rounding values behind the user's back even when
it's not needed.  That's not ideal either.

> furthermore 8/8 bit is pretty accurate for an aspect ratio
>
> so iam against changeing this until we find at least one file which needs
> it to be changed

8/8 is probably close enough for SAR.  I agree we can wait until we
see a problem.

-- 
M?ns Rullg?rd
mru at inprovide.com