[FFmpeg-devel] [PATCH] Fix failure in av_read_frame on timestamp rollover

[Stephen Dredge i]

On 06/12/2010 12:03 AM, Michael Niedermayer wrote:
> On Fri, Jun 11, 2010 at 06:59:01PM +1000, Stephen Dredge i wrote:
>    
>> On 06/10/2010 07:11 PM, Michael Niedermayer wrote:
>>      
>>> On Thu, Jun 10, 2010 at 12:19:19PM +1000, Stephen Dredge i wrote:
>>>
>>>        
>>>> On 06/10/2010 03:25 AM, Michael Niedermayer wrote:
>>>>
>>>>          
>>>>> On Wed, Jun 09, 2010 at 03:02:52PM +1000, Stephen Dredge i wrote:
>>>>>            
> [...]
>    
>>> but maybe the code messing with dts could be changed too, i dont know
>>>
>>>        
>> It's obviously a hack which could be eliminated from avformats point of
>> view but it's probable there are apps which have come to rely on this
>> behaviour.
>>      
>>>
>>>        
>>>> or where pts_wrap_bits = 64.
>>>>
>>>>          
>>> i dont see that problem
>>>
>>>        
>> Shifts greater than the width of the type are undefined and compiler
>> dependant AFAIK. Certainly the version of gcc I'm using doesn't produce the
>> expected output.
>>
>> So you couldn't do something like av_compare_mod(a, b, 1<<  wrap_bits)
>> without testing wrap_bits first -painful.
>>      
> 2ULL<<  (wrap_bits-1)
>
>
> [....]
>    
>> @@ -1152,18 +1152,19 @@ int av_read_frame(AVFormatContext *s, AVPacket *pkt)
>>       AVPacketList *pktl;
>>       int eof=0;
>>       const int genpts= s->flags&  AVFMT_FLAG_GENPTS;
>> +    int wrap_bits;
>>
>>       for(;;){
>>           pktl = s->packet_buffer;
>>           if (pktl) {
>>               AVPacket *next_pkt=&pktl->pkt;
>> +            wrap_bits = s->streams[next_pkt->stream_index]->pts_wrap_bits;
>>      
> declaration and init can be merged
>    
OK
>
>    
>>
>>               if(genpts&&  next_pkt->dts != AV_NOPTS_VALUE){
>>                   while(pktl&&  next_pkt->pts == AV_NOPTS_VALUE){
>>                       if(   pktl->pkt.stream_index == next_pkt->stream_index
>> -&&  next_pkt->dts<  pktl->pkt.dts
>> -&&  pktl->pkt.pts != pktl->pkt.dts //not b frame
>> -                       /*&&  pktl->pkt.dts != AV_NOPTS_VALUE*/){
>>      
>    
>> +&&  (0>  av_compare_mod(next_pkt->dts, pktl->pkt.dts, wrap_bits))		
>>      
> our svn doesnt like trailing whitespace
>    
Ok Fixed
>
>    
>> +&&  av_compare_mod(pktl->pkt.pts, pktl->pkt.dts, wrap_bits)){ //not b frame
>>                           next_pkt->pts= pktl->pkt.dts;
>>                       }
>>                       pktl= pktl->next;
>> diff --git a/libavutil/mathematics.c b/libavutil/mathematics.c
>> index 00b81f1..fff6914 100644
>> --- a/libavutil/mathematics.c
>> +++ b/libavutil/mathematics.c
>> @@ -144,8 +144,13 @@ int av_compare_ts(int64_t ts_a, AVRational tb_a, int64_t ts_b, AVRational tb_b){
>>       return 0;
>>   }
>>
>> -int64_t av_compare_mod(uint64_t a, uint64_t b, uint64_t mod){
>> -    int64_t c= (a-b)&  (mod-1);
>> +int64_t av_compare_mod(int64_t a, int64_t b, int mod_bits){
>> +    int64_t c;
>> +    uint64_t mod;	
>> +    if(mod_bits<= 0 || mod_bits>= 63)
>> +        return a-b;
>>      
> how can it be<=0 ?
>    
True this shouldn't happen.

> [...]
>    
>> -int64_t av_compare_mod(uint64_t a, uint64_t b, uint64_t mod);
>> +int64_t av_compare_mod(int64_t a, int64_t b, int mod_bits);
>>      
> uint64_t is fine
>    
Ok

Fixes Attached
> [...]
>    
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel


-- 
   Stephen Dredge                        sdredge at tpg.com.au
_______________________________________________________________
  System Administrator
  +61 2 9850 0979

   TPG Internet
  www.tpg.com.au



-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: rollover_fix3.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100615/1691c77a/attachment.asc>