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

[Ivan Schreter]

Hi,

Michael Niedermayer wrote:
> On Tue, Sep 15, 2009 at 02:55:30AM +0300, Uoti Urpala wrote:
>   
>> On Tue, 2009-09-15 at 00:22 +0200, Michael Niedermayer wrote:
>>     
>>> On Sun, Sep 13, 2009 at 09:30:31PM +0200, Ivan Schreter wrote:
>>>       
>>>> Attached is a patch that fixes it for timestamp comparison by using 
>>>> comparison routine from NUT spec. A bit more expensive, but so what... I 
>>>> hope it is more to your liking. OK so or any further comments?
>>>>         
>> What range of values is the comparison supposed to work for? The code
>> uses 64-bit types for timestamps, but OTOH the comparison uses this
>> function:
>>
>> + * Convert timestamp to different timebase.
>> + *
>> + * This function converts the timestamp in such a way that no numerical overflow
>> + * can happen. Effectively, it computes ts * tb_to / tb_from.
>> + *
>> + * @param ts      timestamp to convert (non-negative)
>> + * @param tb_from source time base
>> + * @param tb_to   destination time base
>> + * @return timestamp in destination time base
>> + */
>> +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;
>> +}
>> [...]
>> and it
>> wouldn't work with a larger return type (the return statement is of the
>> form "something/d2", so it cannot possibly correctly return anything
>> bigger than UINT64_MAX / d2 - or about 32 bits if d2 is assumed to have
>> full 32 bit range).
>>     
>
> yes, it works only with files of about 100 years duration
> and actually av_rescale_rnd() should be used that should work with the whole
> 32/32*64bit range and lead to simpler code, 
> avoiding code duplication and so on ...
>   

Uhm, I don't quite understand what you mean. Do you mean to use (public) 
av_rescale_rnd() instead of (private) timestamp conversion code? The 
code in av_rescale_rnd() is potentially much slower, but works with 
signed integers, so it would simplify the comparison code...

OTOH, the timestamp conversion code (the one stolen from NUT spec) might 
be interesting for everyone so it possibly could be introduced to 
mathematics.h or somewhere related...

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 :-)

[...]

Regards,

Ivan