[FFmpeg-devel] [PATCH 16/24] fftools/ffmpeg: disable -fix_sub_duration_heartbeat

[Anton Khirnov <anton@khirnov.net>]

As it causes subtitle packets processed by encoders/muxers to signal
back to decoding, it depends on packets being processed in a specific
order and is thus in its current form fundamentally incompatible with
threading architecture.
---
 fftools/ffmpeg.c          | 31 -------------------------------
 fftools/ffmpeg.h          | 10 ----------
 fftools/ffmpeg_dec.c      | 23 -----------------------
 fftools/ffmpeg_enc.c      |  7 -------
 fftools/ffmpeg_mux.c      | 10 ----------
 fftools/ffmpeg_mux_init.c |  4 ----
 fftools/ffmpeg_opt.c      |  9 +++++++--
 tests/fate/ffmpeg.mak     | 24 ++++++++++++------------
 8 files changed, 19 insertions(+), 99 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 038649d9b5..f2293e0250 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -769,37 +769,6 @@ int subtitle_wrap_frame(AVFrame *frame, AVSubtitle *subtitle, int copy)
     return 0;
 }
 
-int trigger_fix_sub_duration_heartbeat(OutputStream *ost, const AVPacket *pkt)
-{
-    OutputFile *of = output_files[ost->file_index];
-    int64_t signal_pts = av_rescale_q(pkt->pts, pkt->time_base,
-                                      AV_TIME_BASE_Q);
-
-    if (!ost->fix_sub_duration_heartbeat || !(pkt->flags & AV_PKT_FLAG_KEY))
-        // we are only interested in heartbeats on streams configured, and
-        // only on random access points.
-        return 0;
-
-    for (int i = 0; i < of->nb_streams; i++) {
-        OutputStream *iter_ost = of->streams[i];
-        InputStream  *ist      = iter_ost->ist;
-        int ret = AVERROR_BUG;
-
-        if (iter_ost == ost || !ist || !ist->decoding_needed ||
-            ist->dec_ctx->codec_type != AVMEDIA_TYPE_SUBTITLE)
-            // We wish to skip the stream that causes the heartbeat,
-            // output streams without an input stream, streams not decoded
-            // (as fix_sub_duration is only done for decoded subtitles) as
-            // well as non-subtitle streams.
-            continue;
-
-        if ((ret = fix_sub_duration_heartbeat(ist, signal_pts)) < 0)
-            return ret;
-    }
-
-    return 0;
-}
-
 /* pkt = NULL means EOF (needed to flush decoder buffers) */
 static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eof)
 {
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 8de91ab85a..c954ed5ebf 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -248,8 +248,6 @@ typedef struct OptionsContext {
     int        nb_reinit_filters;
     SpecifierOpt *fix_sub_duration;
     int        nb_fix_sub_duration;
-    SpecifierOpt *fix_sub_duration_heartbeat;
-    int        nb_fix_sub_duration_heartbeat;
     SpecifierOpt *canvas_sizes;
     int        nb_canvas_sizes;
     SpecifierOpt *pass;
@@ -604,12 +602,6 @@ typedef struct OutputStream {
 
     EncStats enc_stats_pre;
     EncStats enc_stats_post;
-
-    /*
-     * bool on whether this stream should be utilized for splitting
-     * subtitles utilizing fix_sub_duration at random access points.
-     */
-    unsigned int fix_sub_duration_heartbeat;
 } OutputStream;
 
 typedef struct OutputFile {
@@ -875,8 +867,6 @@ InputStream *ist_iter(InputStream *prev);
 OutputStream *ost_iter(OutputStream *prev);
 
 void close_output_stream(OutputStream *ost);
-int trigger_fix_sub_duration_heartbeat(OutputStream *ost, const AVPacket *pkt);
-int fix_sub_duration_heartbeat(InputStream *ist, int64_t signal_pts);
 void update_benchmark(const char *fmt, ...);
 
 #define SPECIFIER_OPT_FMT_str  "%s"
diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index b60bad1220..798ddc25b3 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -439,29 +439,6 @@ static int process_subtitle(InputStream *ist, AVFrame *frame)
     return 0;
 }
 
-int fix_sub_duration_heartbeat(InputStream *ist, int64_t signal_pts)
-{
-    Decoder *d = ist->decoder;
-    int ret = AVERROR_BUG;
-    AVSubtitle *prev_subtitle = d->sub_prev[0]->buf[0] ?
-        (AVSubtitle*)d->sub_prev[0]->buf[0]->data : NULL;
-    AVSubtitle *subtitle;
-
-    if (!ist->fix_sub_duration || !prev_subtitle ||
-        !prev_subtitle->num_rects || signal_pts <= prev_subtitle->pts)
-        return 0;
-
-    av_frame_unref(d->sub_heartbeat);
-    ret = subtitle_wrap_frame(d->sub_heartbeat, prev_subtitle, 1);
-    if (ret < 0)
-        return ret;
-
-    subtitle = (AVSubtitle*)d->sub_heartbeat->buf[0]->data;
-    subtitle->pts = signal_pts;
-
-    return process_subtitle(ist, d->sub_heartbeat);
-}
-
 static int transcode_subtitles(InputStream *ist, const AVPacket *pkt,
                                AVFrame *frame)
 {
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index fa4539664f..aae0ba7a73 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -692,13 +692,6 @@ static int encode_frame(OutputFile *of, OutputStream *ost, AVFrame *frame)
                    av_ts2str(pkt->duration), av_ts2timestr(pkt->duration, &enc->time_base));
         }
 
-        if ((ret = trigger_fix_sub_duration_heartbeat(ost, pkt)) < 0) {
-            av_log(NULL, AV_LOG_ERROR,
-                   "Subtitle heartbeat logic failed in %s! (%s)\n",
-                   __func__, av_err2str(ret));
-            return ret;
-        }
-
         e->data_size += pkt->size;
 
         e->packets_encoded++;
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 57fb8a8413..bc6ce33483 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -502,16 +502,6 @@ int of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
     }
     opkt->dts -= ts_offset;
 
-    {
-        int ret = trigger_fix_sub_duration_heartbeat(ost, pkt);
-        if (ret < 0) {
-            av_log(NULL, AV_LOG_ERROR,
-                   "Subtitle heartbeat logic failed in %s! (%s)\n",
-                   __func__, av_err2str(ret));
-            return ret;
-        }
-    }
-
     ret = of_output_packet(of, ost, opkt);
     if (ret < 0)
         return ret;
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 63a25a350f..d5a10e92bd 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -64,7 +64,6 @@ static const char *const opt_name_enc_stats_post_fmt[]        = {"enc_stats_post
 static const char *const opt_name_mux_stats_fmt[]             = {"mux_stats_fmt", NULL};
 static const char *const opt_name_filters[]                   = {"filter", "af", "vf", NULL};
 static const char *const opt_name_filter_scripts[]            = {"filter_script", NULL};
-static const char *const opt_name_fix_sub_duration_heartbeat[] = {"fix_sub_duration_heartbeat", NULL};
 static const char *const opt_name_fps_mode[]                  = {"fps_mode", NULL};
 static const char *const opt_name_force_fps[]                 = {"force_fps", NULL};
 static const char *const opt_name_forced_key_frames[]         = {"forced_key_frames", NULL};
@@ -1389,9 +1388,6 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
     MATCH_PER_STREAM_OPT(bits_per_raw_sample, i, ost->bits_per_raw_sample,
                          oc, st);
 
-    MATCH_PER_STREAM_OPT(fix_sub_duration_heartbeat, i, ost->fix_sub_duration_heartbeat,
-                         oc, st);
-
     if (oc->oformat->flags & AVFMT_GLOBALHEADER && ost->enc_ctx)
         ost->enc_ctx->flags |= AV_CODEC_FLAG_GLOBAL_HEADER;
 
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 304471dd03..cd1aaabccc 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -970,6 +970,12 @@ static int opt_vstats(void *optctx, const char *opt, const char *arg)
     return opt_vstats_file(NULL, opt, filename);
 }
 
+static int opt_fix_sub_duration_heartbeat(void *optctx, const char *opt, const char *arg)
+{
+    av_log(NULL, AV_LOG_FATAL, "Option '%s' is disabled\n", opt);
+    return AVERROR(ENOSYS);
+}
+
 static int opt_video_frames(void *optctx, const char *opt, const char *arg)
 {
     OptionsContext *o = optctx;
@@ -1740,8 +1746,7 @@ const OptionDef options[] = {
     { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
                           OPT_EXPERT | OPT_OUTPUT,                               { .off = OFFSET(autoscale) },
         "automatically insert a scale filter at the end of the filter graph" },
-    { "fix_sub_duration_heartbeat", OPT_VIDEO | OPT_BOOL | OPT_EXPERT |
-                                    OPT_SPEC | OPT_OUTPUT,                       { .off = OFFSET(fix_sub_duration_heartbeat) },
+    { "fix_sub_duration_heartbeat", OPT_VIDEO | OPT_EXPERT,                      { .func_arg = opt_fix_sub_duration_heartbeat },
         "set this video output stream to be a heartbeat stream for "
         "fix_sub_duration, according to which subtitles should be split at "
         "random access points" },
diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index 835770a924..ebc1e1f189 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -139,18 +139,18 @@ fate-ffmpeg-fix_sub_duration: CMD = fmtstdout srt -fix_sub_duration \
 
 # Basic test for fix_sub_duration_heartbeat, which causes a buffered subtitle
 # to be pushed out when a video keyframe is received from an encoder.
-FATE_SAMPLES_FFMPEG-$(call FILTERDEMDECENCMUX, MOVIE, MPEGVIDEO, \
-                           MPEG2VIDEO, SUBRIP, SRT, LAVFI_INDEV  \
-                           MPEGVIDEO_PARSER CCAPTION_DECODER \
-                           MPEG2VIDEO_ENCODER NULL_MUXER PIPE_PROTOCOL) \
-                           += fate-ffmpeg-fix_sub_duration_heartbeat
-fate-ffmpeg-fix_sub_duration_heartbeat: CMD = fmtstdout srt -fix_sub_duration \
-  -real_time 1 -f lavfi \
-  -i "movie=$(TARGET_SAMPLES)/sub/Closedcaption_rollup.m2v[out0+subcc]" \
-  -map 0:v  -map 0:s -fix_sub_duration_heartbeat:v:0 \
-  -c:v mpeg2video -b:v 2M -g 30 -sc_threshold 1000000000 \
-  -c:s srt \
-  -f null -
+#FATE_SAMPLES_FFMPEG-$(call FILTERDEMDECENCMUX, MOVIE, MPEGVIDEO, \
+#                           MPEG2VIDEO, SUBRIP, SRT, LAVFI_INDEV  \
+#                           MPEGVIDEO_PARSER CCAPTION_DECODER \
+#                           MPEG2VIDEO_ENCODER NULL_MUXER PIPE_PROTOCOL) \
+#                           += fate-ffmpeg-fix_sub_duration_heartbeat
+#fate-ffmpeg-fix_sub_duration_heartbeat: CMD = fmtstdout srt -fix_sub_duration \
+#  -real_time 1 -f lavfi \
+#  -i "movie=$(TARGET_SAMPLES)/sub/Closedcaption_rollup.m2v[out0+subcc]" \
+#  -map 0:v  -map 0:s -fix_sub_duration_heartbeat:v:0 \
+#  -c:v mpeg2video -b:v 2M -g 30 -sc_threshold 1000000000 \
+#  -c:s srt \
+#  -f null -
 
 # FIXME: the integer AAC decoder does not produce the same output on all platforms
 # so until that is fixed we use the volume filter to silence the data
-- 
2.42.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".