[FFmpeg-devel] [PATCH] fftools/ffmpeg: stop accessing av_stream_get_parser() from the main thread

[Anton Khirnov]

It races with the demuxing thread. Instead, send the information along
with the demuxed packets.

Ideally, the code should stop using the stream-internal parsing
completely, but that requires considerably more effort.

Fixes races, e.g. in:
- fate-h264-brokensps-2580
- fate-h264-extradata-reload
- fate-iv8-demux
- fate-m4v-cfr
- fate-m4v
---
 fftools/ffmpeg.c       |  8 ++++++--
 fftools/ffmpeg.h       |  6 ++++++
 fftools/ffmpeg_demux.c | 16 ++++++++++++++--
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index ef7177fc33..ff74534ad0 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2452,7 +2452,9 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
                 if (pkt && pkt->duration) {
                     duration_dts = av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
                 } else if(ist->dec_ctx->framerate.num != 0 && ist->dec_ctx->framerate.den != 0) {
-                    int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict+1 : ist->dec_ctx->ticks_per_frame;
+                    int ticks = ist->last_pkt_repeat_pict >= 0 ?
+                                ist->last_pkt_repeat_pict + 1  :
+                                ist->dec_ctx->ticks_per_frame;
                     duration_dts = ((int64_t)AV_TIME_BASE *
                                     ist->dec_ctx->framerate.den * ticks) /
                                     ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
@@ -2555,7 +2557,9 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
             } else if (pkt->duration) {
                 ist->next_dts += av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
             } else if(ist->dec_ctx->framerate.num != 0) {
-                int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict + 1 : ist->dec_ctx->ticks_per_frame;
+                int ticks = ist->last_pkt_repeat_pict >= 0 ?
+                            ist->last_pkt_repeat_pict + 1  :
+                            ist->dec_ctx->ticks_per_frame;
                 ist->next_dts += ((int64_t)AV_TIME_BASE *
                                   ist->dec_ctx->framerate.den * ticks) /
                                   ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 921e579c32..27ef92654c 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -348,6 +348,12 @@ typedef struct InputStream {
     int64_t       pts;       ///< current pts of the decoded frame  (in AV_TIME_BASE units)
     int           wrap_correction_done;
 
+    // the value of AVCodecParserContext.repeat_pict from the AVStream parser
+    // for the last packet returned from ifile_get_packet()
+    // -1 if unknown
+    // FIXME: this is a hack, the avstream parser should not be used
+    int last_pkt_repeat_pict;
+
     int64_t filter_in_rescale_delta_last;
 
     int64_t min_pts; /* pts with the smallest value in a current stream */
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 6dfb5bb35b..6e89f5999a 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -32,6 +32,9 @@
 typedef struct DemuxMsg {
     AVPacket *pkt;
     int looping;
+
+    // repeat_pict from the demuxer-internal parser
+    int repeat_pict;
 } DemuxMsg;
 
 static void report_new_stream(InputFile *file, const AVPacket *pkt)
@@ -114,7 +117,7 @@ static int seek_to_start(InputFile *ifile)
     return ret;
 }
 
-static void ts_fixup(InputFile *ifile, AVPacket *pkt)
+static void ts_fixup(InputFile *ifile, AVPacket *pkt, int *repeat_pict)
 {
     InputStream *ist = input_streams[ifile->ist_index + pkt->stream_index];
     const int64_t start_time = ifile->ctx->start_time;
@@ -167,6 +170,11 @@ static void ts_fixup(InputFile *ifile, AVPacket *pkt)
 
     if (pkt->dts != AV_NOPTS_VALUE)
         pkt->dts += duration;
+
+    *repeat_pict = -1;
+    if (ist->st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
+        av_stream_get_parser(ist->st))
+        *repeat_pict = av_stream_get_parser(ist->st)->repeat_pict;
 }
 
 static void *input_thread(void *arg)
@@ -231,7 +239,7 @@ static void *input_thread(void *arg)
             }
         }
 
-        ts_fixup(f, pkt);
+        ts_fixup(f, pkt, &msg.repeat_pict);
 
         msg.pkt = av_packet_alloc();
         if (!msg.pkt) {
@@ -352,6 +360,7 @@ int init_input_threads(void)
 
 int ifile_get_packet(InputFile *f, AVPacket **pkt)
 {
+    InputStream *ist;
     DemuxMsg msg;
     int ret;
 
@@ -382,6 +391,9 @@ int ifile_get_packet(InputFile *f, AVPacket **pkt)
     if (msg.looping)
         return 1;
 
+    ist = input_streams[f->ist_index + msg.pkt->stream_index];
+    ist->last_pkt_repeat_pict = msg.repeat_pict;
+
     *pkt = msg.pkt;
     return 0;
 }
-- 
2.35.1