[FFmpeg-devel] [PATCH 16/24] fftools/ffmpeg: disable -fix_sub_duration_heartbeat
Anton Khirnov
anton at khirnov.net
Sat Nov 4 09:56:25 EET 2023
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
More information about the ffmpeg-devel
mailing list