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

Michael Niedermayer michaelni
Tue Aug 12 05:51:38 CEST 2008


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


> less according to pts (which can be the case now when decoder sets delay
> to 16, right ?), though 16 is rather large, dunno about SVC though.

SVC hmmmm, i wish that didnt exist ....
luckily it doesnt, at least not outside the halls of the JVT just like
scaleable mpeg2. Lets just hope it does not escape ;)

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

I count him braver who overcomes his desires than him who conquers his
enemies for the hardest victory is over self. -- Aristotle
-------------- 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/20080812/cf22472d/attachment.pgp>



More information about the ffmpeg-devel mailing list