[FFmpeg-devel] [PATCH] MPEG-TS/-PS : better probing for duration and start-time for all streams in a container

[Gaullier Nicolas]

>> Please see below my new proposal-patch:
>> The first_dts is the reason why the start_time was stuck to the first 
>> packet, since update_initial_timestamps()  returned immediately without any further processing.
>> I thus skiped this first_dts test.
>> I then nearly replaced "x=y" with "x=FFMIN(x,y)" with appropriate checkings.
>> I think it does not change the structure of the code, so it should be safe.
>> Thank you,
>> Nicolas
>
>patch breaks "make fate"

(I finally managed to have fate working on my environment, so I will not bother you with patch breaks anymore.)

It appears that, in the fate-suite, mpeg2/mpeg2_field_encoding.ts is somewhat broken.
This sample was certainly extracted by means of a simple binary cut. It is common practice with TS, 
so there is nothing really "wrong" with it, but the fact is that the first broken packets were
considered valid in the current implementation.
Besides, since there is no "B" frames (only I/P in this stream), the following fallback rule apply:
[in utils.c/update_initial_durations]
if (!st->codec->has_b_frames)
                pktl->pkt.pts = cur_dts;
At the end, a corrupted (truncated) packet is assigned a valid PTS value, and the new algorithm
of the patch takes it into account, leading to a wrong estimation of the start_time (and fate breaks).
To resolve this, I think the cleanest solution is to consider the first truncated packets in the stream
as non-valid. I thus propose here a patch in the avcodec/mpeg layer to impose detection of a picture_start_code.
That way, it is not possible to assign a PTS where there is no picture.

Second thing, there was another break in FATE, because of the CAVS sample (cavs.mpg).
It appears that in this sample, most of the PES come with no PTS/DTS. I am not aware of CAVS
and suppose it is normal, but at this end, this must be taken into account in avutils.c/compute_pkt_fields.
I discovered there was already a onein_oneout defined, so I just appended CAVS.
There is another onein_oneout in 'select_from_pts_buffer' where CAVS could possibly be appended too,
but I really do not know anything about CAVS, so I just made the minimum change to have FATE pass.

Nicolas
diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index 7aa3660..fa6e624 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -32,7 +32,7 @@ struct MpvParseContext {
 };
 
 
-static void mpegvideo_extract_headers(AVCodecParserContext *s,
+static int mpegvideo_extract_headers(AVCodecParserContext *s,
                                       AVCodecContext *avctx,
                                       const uint8_t *buf, int buf_size)
 {
@@ -46,6 +46,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
     int did_set_size=0;
     int bit_rate = 0;
     int vbv_delay = 0;
+    int found_picture_start_code = 0;
 //FIXME replace the crap with get_bits()
     s->repeat_pict = 0;
 
@@ -55,6 +56,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
         bytes_left = buf_end - buf;
         switch(start_code) {
         case PICTURE_START_CODE:
+            found_picture_start_code = 1;
             if (bytes_left >= 2) {
                 s->pict_type = (buf[1] >> 3) & 7;
                 if (bytes_left >= 4)
@@ -149,6 +151,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
                (bit_rate != 0x3FFFF || vbv_delay != 0xFFFF)) {
         avctx->bit_rate = 400*bit_rate;
     }
+    return !found_picture_start_code;
 }
 
 static int mpegvideo_parse(AVCodecParserContext *s,
@@ -159,6 +162,7 @@ static int mpegvideo_parse(AVCodecParserContext *s,
     struct MpvParseContext *pc1 = s->priv_data;
     ParseContext *pc= &pc1->pc;
     int next;
+    int picture_data_present = 0;
 
     if(s->flags & PARSER_FLAG_COMPLETE_FRAMES){
         next= buf_size;
@@ -175,12 +179,13 @@ static int mpegvideo_parse(AVCodecParserContext *s,
     /* we have a full frame : we just parse the first few MPEG headers
        to have the full timing information. The time take by this
        function should be negligible for uncorrupted streams */
-    mpegvideo_extract_headers(s, avctx, buf, buf_size);
-    av_dlog(NULL, "pict_type=%d frame_rate=%0.3f repeat_pict=%d\n",
+    picture_data_present = !mpegvideo_extract_headers(s, avctx, buf, buf_size);
+    if (picture_data_present)
+        av_dlog(NULL, "pict_type=%d frame_rate=%0.3f repeat_pict=%d\n",
             s->pict_type, (double)avctx->time_base.den / avctx->time_base.num, s->repeat_pict);
 
     *poutbuf = buf;
-    *poutbuf_size = buf_size;
+    *poutbuf_size = picture_data_present ? buf_size : 0;
     return next;
 }
 
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 5c646c6..2046481 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1037,14 +1037,15 @@ static void update_initial_timestamps(AVFormatContext *s, int stream_index,
     int64_t shift;
     int i, delay;
 
-    if (st->first_dts != AV_NOPTS_VALUE ||
-        dts           == AV_NOPTS_VALUE ||
+    if (dts           == AV_NOPTS_VALUE ||
         st->cur_dts   == AV_NOPTS_VALUE ||
         is_relative(dts))
         return;
 
     delay         = st->codec->has_b_frames;
-    st->first_dts = dts - (st->cur_dts - RELATIVE_TS_BASE);
+    if (st->first_dts == AV_NOPTS_VALUE) {
+           st->first_dts = dts - (st->cur_dts - RELATIVE_TS_BASE);
+    } else st->first_dts = FFMIN(st->first_dts,dts - (st->cur_dts - RELATIVE_TS_BASE));
     st->cur_dts   = dts;
     shift         = st->first_dts - RELATIVE_TS_BASE;
 
@@ -1062,9 +1063,11 @@ static void update_initial_timestamps(AVFormatContext *s, int stream_index,
 
         if (is_relative(pktl->pkt.dts))
             pktl->pkt.dts += shift;
-
-        if (st->start_time == AV_NOPTS_VALUE && pktl->pkt.pts != AV_NOPTS_VALUE)
-            st->start_time = pktl->pkt.pts;
+        if (pktl->pkt.pts != AV_NOPTS_VALUE) {
+            if (st->start_time == AV_NOPTS_VALUE) {
+                   st->start_time = pktl->pkt.pts;
+            } else st->start_time = FFMIN(st->start_time, pktl->pkt.pts);
+        }
 
         if (pktl->pkt.pts != AV_NOPTS_VALUE && delay <= MAX_REORDER_DELAY && has_decode_delay_been_guessed(st)) {
             pts_buffer[0] = pktl->pkt.pts;
@@ -1075,8 +1078,11 @@ static void update_initial_timestamps(AVFormatContext *s, int stream_index,
         }
     }
 
-    if (st->start_time == AV_NOPTS_VALUE)
-        st->start_time = pts;
+    if (pts != AV_NOPTS_VALUE) {
+        if (st->start_time == AV_NOPTS_VALUE) {
+               st->start_time = pts;
+        } else st->start_time = FFMIN(st->start_time, pts);
+    }
 }
 
 static void update_initial_durations(AVFormatContext *s, AVStream *st,
@@ -1139,6 +1145,7 @@ static void compute_pkt_fields(AVFormatContext *s, AVStream *st,
     int64_t offset;
     AVRational duration;
     int onein_oneout = st->codec->codec_id != AV_CODEC_ID_H264 &&
+                       st->codec->codec_id != AV_CODEC_ID_CAVS &&
                        st->codec->codec_id != AV_CODEC_ID_HEVC;
 
     if (s->flags & AVFMT_FLAG_NOFILLIN)
@@ -2511,12 +2518,13 @@ static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
 
         avio_seek(ic->pb, offset, SEEK_SET);
         read_size = 0;
+        st->last_dts_for_order_check=0;
         for (;;) {
             if (read_size >= DURATION_MAX_READ_SIZE << (FFMAX(retry - 1, 0)))
                 break;
 
             do {
-                ret = ff_read_packet(ic, pkt);
+                ret = read_frame_internal(ic,pkt);
             } while (ret == AVERROR(EAGAIN));
             if (ret != 0)
                 break;