[FFmpeg-devel] obseve PTS overflow when seeking

[Michael Niedermayer]

On Mon, Aug 20, 2012 at 02:04:57PM +0200, Wolfram Gloger wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
> > [1:multipart/signed Hide]
> > [1/1:text/plain Hide]
> > On Sat, Aug 18, 2012 at 11:12:19AM +0200, Rainer Hochecker wrote:
> >> Hi,
> >> 
> >> I have tracked down a problem where seeking did not work because PTS
> >> overflow in mpegts file. The following would fix it but I am not
> >> sure whether it's entirely correct. Could you have a look at this?
> >
> > yes, it wont work like this :(
> >
> > the first problem is the index (see av_index* and ff_seek_frame_binary)
> > it is required to be ordered and it wont be for such files.
> 
> The mpegts demuxer doesn't set AVFMT_GENERIC_INDEX, so how is this
> relevant in the case in question?

mpegts calls av_add_index_entry() directly


> If I'd make the appended patch
> dependant on !(flags & AVFMT_GENERIC_INDEX), or the non-presence of an
> av_index, could it be accepted?

the index would become present during the first seek for mpegts
so this would stop working for subsequent seeks


> 
> > also the timestamps passed may lay before start_time for the reason
> > of seeking to the first keyframe, some code is doing that IIRC
> 
> You mean in av_seek_frame() in ffmpeg_opt.c?  That is true, but after
> that seek I would argue that avformat_find_stream_info() has to be
> called again anyway (e.g. in mpegts, _everything_ may change,
> resolution, aspect ratio, number of audio channels...) and that also
> corrects start_time.

Its very possible that it might work out with ffmpeg_opt.c but thats
not the only application that uses libavformat
seeking ~5 seconds before starttime (thats what you will likely get
when pressing the left arrow in some players at the begin) should not
seek to the end of the file


> 
> > For the discontinuity removial case, it would have to be optional
> > because it has some disadvanatges.
> > The simplest way to implement it is probably to add a array of
> > pos,ts_offset pairs to AVFormatContext
> > Now each time a packet is demuxed in streams that allow discontinuities
> > (AVFMT_TS_DISCONT), its timestamp is corrected based on this array and
> > the array is updated when new discontinuities are discovered.
> 
> Didn't we already have such code long ago (w/o the array though),
> with such a function:
> 
> +static int64_t lsb2full(int64_t lsb, int64_t last_ts, int lsb_bits){
> +    int64_t mask = lsb_bits < 64 ? (1LL<<lsb_bits)-1 : -1LL;
> +    int64_t delta= last_ts - mask/2;
> +    return  ((lsb - delta)&mask) + delta;
> +}
> 
> > This way the index can be kept with monotonic timestamps and the
> > user application would not have to deal with discontinuities. The
> > price for this is that in such a discontinuity removial mode timestamps
> > of packets would not be imutable but could change when more
> > discontinuities are discovered. Thats why it has to be optional
> 
> If I'd resurrect the use of lsb2full() in compute_pkt_fields() and make
> that optional, could such a patch be accepted?

yes but its a half-solution, it may work with a single timestamp
wrap, it wont help with general discontinuities.

also i suspect the correction would be needed at more places,
especially the seeking code and index would need changes too

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

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120820/bf1a2621/attachment.asc>