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

[Michael Niedermayer]

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:
>>    
>>> av_read_frame can fail on some (TS) files at timestamp rollover, reading 
>>> to
>>> eof before returning a frame.
>>>
>>> The attached patch adds some utility functions for rollover aware 
>>> timestamp
>>> comparison  and uses them in av_read_frame to avoid problems at timestamp
>>> rollover.
>>>
>>> The utility functions provide normal comparisons if pts_wrap_bits is 0 or
>>>      
>>>> = 63 so behaviour on formats without timestamp wrapping should be
>>>>        
>>> unaffected.
>>>
>>> -- 
>>>    Stephen Dredge                        sdredge at tpg.com.au
>>> _______________________________________________________________
>>>   System Administrator
>>>   +61 2 9850 0979
>>>
>>>    TPG Internet
>>>   www.tpg.com.au
>>>
>>>
>>>
>>>      
>>    
>>>   libavformat/utils.c     |    9 ++++---
>>>   libavutil/mathematics.c |   59 
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>   libavutil/mathematics.h |   25 ++++++++++++++++++++
>>>   3 files changed, 89 insertions(+), 4 deletions(-)
>>> a5af48c8203ac3261e2c383073a131c64ca0d294  rollover_fix.patch
>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>> index 6365f3e..02ffce1 100644
>>> --- a/libavformat/utils.c
>>> +++ b/libavformat/utils.c
>>> @@ -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;		
>>>               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
>>>      
>> a != check should
>>    
> dts can be negative and pts positive for the same time (compute_pkt_fields 
> can emit negative dts), so a mod comparison is required.
>> [...]
>>    
>>> +int64_t av_mod_wrap_bits(int64_t x, int wrap_bits){
>>> +	int64_t result;
>>> +	int64_t modulo;
>>> +	if(!wrap_bits || wrap_bits>= 63) {
>>> +		return x;
>>> +	}
>>> +	modulo = 1LL<<  wrap_bits;
>>> +	result = x % modulo;
>>> +	if(result<  0) {
>>> +		result += modulo;
>>> +	}
>>> +	return result;
>>> +}
>>> +
>>> +int av_gt_mod_wrap_bits(int64_t x, int64_t y, int wrap_bits){
>>> +	int64_t modulo;
>>> +	int64_t result;
>>> +	if(!wrap_bits || wrap_bits>= 63) {
>>> +		return x>y?1:0;
>>> +	}	
>>> +	modulo = 1LL<<  wrap_bits;
>>> +	result = av_mod_wrap_bits(x - y, wrap_bits);
>>> +	if(result<  modulo/2) {
>>> +		return 1;
>>> +	}
>>> +	return 0;
>>> +}
>>>      
>> this is a mess, ive added a much simpler av_compare_mod() that could be 
>> used
>>    
> Agreed it's clunky. I was going for clarity and simplicity of use over 
> elegance.
>
> av_compare_mod is not much help here. It can't be used for negative numbers 

a&(m-1) makes an argument positiv
but maybe the code messing with dts could be changed too, i dont know


> or where pts_wrap_bits = 64.

i dont see that problem

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20100610/f5ec56e4/attachment.pgp>