[FFmpeg-devel] [PATCH] fix start time with delay

[Michael Niedermayer]

On Sun, Nov 28, 2010 at 11:03:46PM -0800, Baptiste Coudurier wrote:
> On 11/25/10 3:32 PM, Michael Niedermayer wrote:
> > On Thu, Nov 25, 2010 at 03:20:35PM -0800, Baptiste Coudurier wrote:
> >> On 11/25/10 3:16 PM, Michael Niedermayer wrote:
> >>> On Wed, Nov 24, 2010 at 06:29:29PM -0800, Baptiste Coudurier wrote:
> >>>> Hi guys,
> >>>>
> >>>> $subject.
> >>>>
> >>>> This happens if the first I frame is delayed and the following frames
> >>>> are decodable and reordered.
> >>>>
> >>>> -- 
> >>>> Baptiste COUDURIER
> >>>> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> >>>> FFmpeg maintainer                                  http://www.ffmpeg.org
> >>>
> >>>>  utils.c |    2 ++
> >>>>  1 file changed, 2 insertions(+)
> >>>> 8ea30827f607b6af0ffafaa87a573895ef8b18d8  fix_start_time_with_delay.patch
> >>>> Index: libavformat/utils.c
> >>>> ===================================================================
> >>>> --- libavformat/utils.c	(revision 25824)
> >>>> +++ libavformat/utils.c	(working copy)
> >>>> @@ -1045,6 +1045,8 @@
> >>>>          if(st->codec->codec_id == CODEC_ID_H264){ //we skiped it above so we try here
> >>>>              update_initial_timestamps(s, pkt->stream_index, pkt->dts, pkt->pts); // this should happen on the first packet
> >>>>          }
> >>>> +        if (pkt->pts < st->start_time)
> >>>> +            st->start_time = pkt->pts;
> >>>
> >>> This breaks with timestamp discontinuities as well as with timestamp wrap
> >>> around
> >>
> >> What do you suggest ?
> > 
> > * limit this to the first 10 or so frames
> > * consider pts and pts+(1<<bits) and use the one that is closer to the start_time
> >   for the comparission and for setting
> > 
> > also changes to update_initial_timestamps() might be needed too
> 
> Humm, ok, another try. I moved the code to av_find_stream_info since I
> don't feel like checking of codec_info_nb_frames in compute_pkt_fields.
> 
> -- 
> Baptiste COUDURIER
> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> FFmpeg maintainer                                  http://www.ffmpeg.org

>  utils.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> c5cbc9e8816f267e314f23ea81299879e453914c  fix_start_time_with_delay.patch
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 25824)
> +++ libavformat/utils.c	(working copy)
> @@ -2308,6 +2308,12 @@
>          read_size += pkt->size;
>  
>          st = ic->streams[pkt->stream_index];
> +        if (pkt->pts != AV_NOPTS_VALUE && st->start_time != AV_NOPTS_VALUE &&
> +            pkt->pts < st->start_time &&

> +            st->start_time - pkt->pts <
> +            pkt->pts+(1LL<<st->pts_wrap_bits) - st->start_time &&


this can overflow at several places, st->start_time - pkt->pts if signed can
overflow, 1LL<<st->pts_wrap_bits can if there are 64bit

i think the following is ok but i didnt test it

st->start_time - pkt->pts < 1ULL<<(st->pts_wrap_bits-1)


if that works then the patch is ok

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

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110107/517dc80c/attachment.pgp>