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

[Michael Niedermayer]

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


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


> +                       && 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 ?

[...]
> -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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100611/947ecddf/attachment.pgp>