[FFmpeg-devel] obseve PTS overflow when seeking

[Wolfram Gloger]

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?  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?

> 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.

> 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?

Regards,
Wolfram.
---

 libavformat/utils.c |   48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 44 insertions(+), 4 deletions(-)


diff --git a/libavformat/utils.c b/libavformat/utils.c
index 0ba1f3b..480c383 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1649,21 +1649,56 @@ int64_t ff_gen_search(AVFormatContext *s, int stream_index, int64_t target_ts,
     }
 
     if(ts_max == AV_NOPTS_VALUE){
-        int step= 1024;
+        int64_t step= 1024;
         filesize = avio_size(s->pb);
         pos_max = filesize - 1;
         do{
             pos_max -= step;
             ts_max = read_timestamp(s, stream_index, &pos_max, pos_max + step);
-            step += step;
-        }while(ts_max == AV_NOPTS_VALUE && pos_max >= step);
+#ifdef DEBUG_SEEK
+            av_log(s, AV_LOG_DEBUG, "ts_min=0x%llx ts_max=0x%llx\n",
+                   ts_min, ts_max);
+#endif
+
+            if (ts_max == AV_NOPTS_VALUE || ts_max < target_ts)
+                step += step;
+            else {
+		if (ts_max < ts_min) { /* wraparound */
+#ifdef DEBUG_SEEK
+		    av_log(s, AV_LOG_DEBUG, "wraparound: target=%lld ts_min=%lld ts_max=%lld\n",
+			   target_ts, ts_min, ts_max);
+#endif
+		    /* try to adjust pos_min until ts_min<=target_ts */
+		    step = pos_max - pos_min;
+		    while (step > 0) {
+			step >>= 1;
+			pos= pos_min + step;
+			ts= read_timestamp(s, stream_index, &pos, pos_max);
+#ifdef DEBUG_SEEK
+			av_log(s, AV_LOG_DEBUG, "wraparound2: pos=%lld step=%lld ts=%lld\n", pos, step, ts);
+#endif
+			if (ts != AV_NOPTS_VALUE) {
+			    if (ts <= target_ts ) {
+				pos_min = pos;
+				ts_min = ts;
+				break;
+			    } else if (ts_max < ts) {
+				pos_min = pos;
+				step = pos_max - pos_min;
+			    }
+			}
+		    }
+		}
+                break;
+            }
+        } while (pos_max >= step);
         if (ts_max == AV_NOPTS_VALUE)
             return -1;
 
         for(;;){
             int64_t tmp_pos= pos_max + 1;
             int64_t tmp_ts= read_timestamp(s, stream_index, &tmp_pos, INT64_MAX);
-            if(tmp_ts == AV_NOPTS_VALUE)
+            if(tmp_ts == AV_NOPTS_VALUE || tmp_ts < ts_max)
                 break;
             ts_max= tmp_ts;
             pos_max= tmp_pos;
@@ -1684,6 +1719,11 @@ int64_t ff_gen_search(AVFormatContext *s, int stream_index, int64_t target_ts,
         pos_limit= pos_min;
     }
 
+#ifdef DEBUG_SEEK
+    av_log(s, AV_LOG_DEBUG, "ts_min=0x%llx ts_max=0x%llx pos_limit=%lld\n",
+           ts_min, ts_max, pos_limit);
+#endif
+
     no_change=0;
     while (pos_min < pos_limit) {
         av_dlog(s, "pos_min=0x%"PRIx64" pos_max=0x%"PRIx64" dts_min=%"PRId64" dts_max=%"PRId64"\n",