[FFmpeg-devel] [PATCH] timestamp discontinuity in subtitles is OK

[Michael Niedermayer]

On Sun, Oct 05, 2008 at 10:23:03PM -0000, Wolfram Gloger wrote:
> Hi,
> 
> > carefull, you are missing a few issues here
> 
> Possibly..
> 
> > the first being b frame reordering (if the code is used for video)
> 
> AFAICS the current code does:
> 

> next_pts = (current_dts if valid, else last_pts) + (computed pkt duration)

this isnt exactly true ...
its pretty much adding 1/timebase for video, samples/samplerate for audio
and leaves it untouched for subtitles
now 1/timebase isnt correct for video, it should consider repeat_pict
it just works out fine because lavf fills in timestamps considering
repeat_pict (so its a bug in a case that isnt really used, still a patch
fixing this is very welcome, ive just noticed this now ...)


> 
> so may I ask 2 questions:
> 
> - how is this affected by B frames (pts!=dts)?

next_pts represents the timestamp of the decoder _output_
pkt->duration represents the duration of the decode _input_
now when there is frame reordering these do not match up that nicely
besides there are some obscure codecs that put more than one codec
packet in a AVPacket in that case the next_pts increas of course is just
part of AVPacket.duration


> 
> - why is a manually computed offset added rather than pkt.duration?

because pkt.duration is just wrong besides it is often not known, you
do know the sample rate and how many samples the audio decoder produced.


> 
> > second being that this isnt strictly correct nor what i suggested,
> > its not making the discontinuity code check the last duration it
> > changes next_pts which really is used for more than discontinuity
> > detection.
> > third, while AVPacket =3D=3D decoder packet whenever possible this is not
> > always possible, when its not next_pts is the variable that keeps track
> > of the next pts and this is needed even when duration=3D=3D0
> 
> OK -- if that's the case, how about remembering a "last duration" per
> stream and looking at that in the discontinuity check?

this would almost work, if i had not missed that the "last duration" isnt
strictly correct (due to frame reordering). Now in reality it should be fine
as known and unknown durations should not be mixed but ...

i think there is a easier solution
just check for AVFMT_TS_DISCONT which would prevent misdetection in all
containers that do not allow discontinuities.

[...]

-- 
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/20081006/67b398d3/attachment.pgp>