[FFmpeg-devel] [PATCH] increase AVStream pts_buffer size
Michael Niedermayer
michaelni
Mon Aug 11 23:55:09 CEST 2008
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.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20080811/8ec1f828/attachment.pgp>
More information about the ffmpeg-devel
mailing list