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

[Ivan Schreter]

Hi Michael,

Michael Niedermayer wrote:
> On Sun, Sep 06, 2009 at 04:49:51PM +0200, Ivan Schreter wrote:
>   
>> Michael Niedermayer wrote:
>>     
>>> On Sat, Sep 05, 2009 at 09:31:01PM +0200, schreter wrote:
>>>   
>>>       
>>> [...]
>>>>  static int compare_ts(int64_t ts_a, AVRational tb_a, int64_t ts_b, 
>>>> AVRational tb_b)
>>>>  {
>>>> @@ -95,9 +63,9 @@ static int compare_ts(int64_t ts_a, AVRa
>>>>      if (ts_a == INT64_MIN)
>>>>          return ts_a < ts_b ? -1 : 0;
>>>>      if (ts_a == INT64_MAX)
>>>> -        return ts_a > ts_b ? 1 : 0;
>>>> +        return ts_a > ts_b ?  1 : 0;
>>>>      if (ts_b == INT64_MIN)
>>>> -        return ts_a > ts_b ? 1 : 0;
>>>> +        return ts_a > ts_b ?  1 : 0;
>>>>      if (ts_b == INT64_MAX)
>>>>          return ts_a < ts_b ? -1 : 0;
>>>>  @@ -105,7 +73,7 @@ static int compare_ts(int64_t ts_a, AVRa
>>>>      b = ts_b * tb_b.num * tb_a.den;
>>>>       res = a - b;
>>>> -    if (res == 0)
>>>> +    if (!res)
>>>>          return 0;
>>>>      else
>>>>          return (res >> 63) | 1;
>>>>     
>>>>         
>>> [...]
>
> you multiply 2 32bit values and a 64 bit value, this needs 128bit but it
> doesnt have that amount
> i belive i quoted code that does work when when suggested this, if not i
> can now if needed ...
>   

Yes, that's the only problem when it overflows. I assumed noone will use 
such wild timestamps and yet wilder timebases, obviously wrongly.

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?

There is one issue remaining: how to determine which timestamp from two 
timestamps in timebase tb_a is actually closer to target timestamp in 
another timebase tb_b (routine find_closer_ts). I used a distance 
routine, which multiplies the distances by numerators and denumerators 
of respective timebases to have a comparable value. This suffers from 
the possible overflow problem as well. Any idea how to solve this? It is 
also in the attached patch (as TODO, I already changed the rest of the 
code below to use find_closer_ts instead of possibly overflowing distance).

Alternatively, we can let the distance routine as-is, but check if an 
overflow happened and return INT64_MAX in that case. That'd work 
reliably as well, I think, since only relatively close timestamps are 
compared. What do you think?

FYI: I will split the patch in two afterwards (one for timestamp 
comparisons, one for distance).

Thanks & regards,

Ivan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: seek_uncooked_timestamp.patch
Type: text/x-patch
Size: 5564 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20090913/e2102941/attachment.bin>