[FFmpeg-devel] [PATCH] increase AVStream pts_buffer size

[Baptiste Coudurier]

Hi Michael,

Michael Niedermayer wrote:
> On Mon, Aug 11, 2008 at 03:47:37PM -0700, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> On Mon, Aug 11, 2008 at 12:47:28PM -0700, Baptiste Coudurier wrote:
>>>> Hi Michael,
>>>>
>>>> Michael Niedermayer wrote:
>>>>> On Sat, Aug 09, 2008 at 06:28:47PM -0700, Baptiste Coudurier wrote:
>>>>>> Hi Michael,
>>>>>>
>>>>>> Michael Niedermayer wrote:
>>>>>>> On Sat, Aug 09, 2008 at 05:34:51PM -0700, Baptiste Coudurier wrote:
>>>>>>>> Baptiste Coudurier wrote:
>>>>>>>>> M?ns Rullg?rd wrote:
>>>>>>>>>> Baptiste Coudurier <baptiste.coudurier at smartjog.com> writes:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> H264 decoder can (now?) set has_b_frames to MAX_DELAYED_PIC_COUNT which
>>>>>>>>>>> is 16, need to port this to pts_buffer in AVStream struct.
>>>>>>>>>>>
>>>>>>>>>>> compute_pkt_files in utils.c:
>>>>>>>>>>> int delay = FFMAX(st->codec->has_b_frames, !!st->codec->max_b_frames);
>>>>>>>>>>>
>>>>>>>>>>> [...]
>>>>>>>>>>>
>>>>>>>>>>> //calculate dts from pts
>>>>>>>>>>> if(pkt->pts != AV_NOPTS_VALUE && pkt->dts == AV_NOPTS_VALUE){
>>>>>>>>>>>     st->pts_buffer[0]= pkt->pts;
>>>>>>>>>>>     for(i=1; i<delay+1 && st->pts_buffer[i] == AV_NOPTS_VALUE; i++)
>>>>>>>>>>>         st->pts_buffer[i]= (i-delay-1) * pkt->duration;
>>>>>>>>>>>     for(i=0; i<delay && st->pts_buffer[i] > st->pts_buffer[i+1]; i++)
>>>>>>>>>>>         FFSWAP(int64_t, st->pts_buffer[i], st->pts_buffer[i+1]);
>>>>>>>>>>>      pkt->dts= st->pts_buffer[0];
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> Patch attached.
>>>>>>>>>>>
>>>>>>>>>>> Index: libavformat/avformat.h
>>>>>>>>>>> ===================================================================
>>>>>>>>>>> --- libavformat/avformat.h	(revision 14513)
>>>>>>>>>>> +++ libavformat/avformat.h	(working copy)
>>>>>>>>>>> @@ -385,7 +385,7 @@
>>>>>>>>>>>  
>>>>>>>>>>>      int64_t nb_frames;                 ///< number of frames in this stream if known or 0
>>>>>>>>>>>  
>>>>>>>>>>> -#define MAX_REORDER_DELAY 4
>>>>>>>>>>> +#define MAX_REORDER_DELAY 16
>>>>>>>>>>>      int64_t pts_buffer[MAX_REORDER_DELAY+1];
>>>>>>>>>>>  
>>>>>>>>>>>      char *filename; /**< source filename of the stream */
>>>>>>>>>> This breaks ABI, so it needs a version bump.  Others can have an
>>>>>>>>>> opinion on how bad that is.
>>>>>>>>>>
>>>>>>>>> Right, we can avoid going too far in the mean time to avoid breaking
>>>>>>>>> ABI, it is not the right solution I believe but it should fix the bug,
>>>>>>>>> patch attached.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ------------------------------------------------------------------------
>>>>>>>>>
>>>>>>>>> Index: libavformat/utils.c
>>>>>>>>> ===================================================================
>>>>>>>>> --- libavformat/utils.c	(revision 14513)
>>>>>>>>> +++ libavformat/utils.c	(working copy)
>>>>>>>>> @@ -2472,7 +2472,7 @@
>>>>>>>>>      //calculate dts from pts
>>>>>>>>>      if(pkt->pts != AV_NOPTS_VALUE && pkt->dts == AV_NOPTS_VALUE){
>>>>>>>>>          st->pts_buffer[0]= pkt->pts;
>>>>>>>>> -        for(i=1; i<delay+1 && st->pts_buffer[i] == AV_NOPTS_VALUE; i++)
>>>>>>>>> +        for(i=1; i<delay+1 && i < MAX_REORDER_DELAY+1 && st->pts_buffer[i] == AV_NOPTS_VALUE; i++)
>>>>>>>>>              st->pts_buffer[i]= (i-delay-1) * pkt->duration;
>>>>>>>>>          for(i=0; i<delay && st->pts_buffer[i] > st->pts_buffer[i+1]; i++)
>>>>>>>>>              FFSWAP(int64_t, st->pts_buffer[i], st->pts_buffer[i+1]);
>>>>>>>>>
>>>>>>>> Any comments ? This fixes a bug :>
>>>>>>> too fe hours each day ... :)
>>>>>>>
>>>>>>> i think there ae 2 issues here.
>>>>>>> first is that MAX_REORDER_DELAY is too small, that can be fixed adding a
>>>>>>> new pts_buffer at the end of AVStream that is bigger and putting the old
>>>>>>> one under #if VERSION<...
>>>>>>>
>>>>>> Thanks for review, patch attached.
>>>>> ok, but i would name the old pts_buffer2 and the new pts_buffer, this should
>>>>> make the diff smaller
>>>>>
>>>> Oh yeah, good idea, and both hunk can be applied separately.
>>> patch to avformat.h is ok
>>>
>>> for the other i think that 
>>> -if(pkt->pts != AV_NOPTS_VALUE){
>>> +if(pkt->pts != AV_NOPTS_VALUE && delay <= MAX_REORDER_DELAY){
>>>
>>> would be better
>>> because the code will result in random timestamps otherwise.
>>> The reorder buffer would be smaller than what the codec needs.
>>>
>> Ok, I was just thinking that some delay could be set would actually be
> 
> patch ok
> 

Both patch applied.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA