[FFmpeg-cvslog] r19773 - in trunk/libavformat: seek.c seek.h

[Uoti Urpala]

On Fri, 2009-09-18 at 02:04 +0200, Michael Niedermayer wrote:
> On Thu, Sep 17, 2009 at 07:33:37PM +0200, Ivan Schreter wrote:
> >>> +static int convert_ts(uint64_t ts, AVRational tb_from, AVRational tb_to)
> >>> +{
> >>> +    // this algorithm is copied from NUT spec and works only for 
> >>> non-negative numbers
> >>> +    uint64_t ln = (uint64_t) tb_from.num * (uint64_t) tb_to.den;
> >>> +    uint64_t d1 = tb_from.den;
> >>> +    uint64_t d2 = tb_to.num;
> >>> +    return (ln / d1 * ts + ln % d1 * ts / d1) / d2;
> >>> +}

> > BTW, I simply assumed the formula given in NUT spec is correct, I didn't do 
> > mathematic proof of that. So I hope it is correct :-)
> 
> the code works, but it seems a little typo sliped through so that 2 variables
> where exchanged

It's more broken than can be explained with a "little typo". You'd need
to add more arithmetic to make it work correctly, even with an
assumption that the scaled times will be below 2^32. I guess the NUT
spec implicitly assumed also that the timestamp will a 32-bit number
(_unscaled_ value)? I looked at the NUT spec that computation was copied
from, and it seems quite vague about the allowed values. There's
"time_base_denom MUST be < 2^31" but I see nothing similar at all for
the timestamp and numerator values; OTOH the convert_ts description says
about calculating timestamp*num*denom/num/denom naively that it "would
require a 96 bit integer" which seems to assume about 32-bit timestamps
with 32-bit numerators and denominators.