* [FFmpeg-devel] [PATCH 02/33] fftools/ffmpeg_demux: return errors from ifile_open() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 03/33] fftools/ffmpeg_demux: drop a redundant avio_flush() Anton Khirnov
` (31 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_demux.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 5d3e043793..a6df2a1904 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1349,7 +1349,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
int64_t start = start_time == AV_NOPTS_VALUE ? 0 : start_time;
if (stop_time <= start) {
av_log(d, AV_LOG_ERROR, "-to value smaller than -ss; aborting.\n");
- exit_program(1);
+ return AVERROR(EINVAL);
} else {
recording_time = stop_time - start;
}
@@ -1358,7 +1358,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
if (o->format) {
if (!(file_iformat = av_find_input_format(o->format))) {
av_log(d, AV_LOG_FATAL, "Unknown input format: '%s'\n", o->format);
- exit_program(1);
+ return AVERROR(EINVAL);
}
}
@@ -1372,7 +1372,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
/* get default parameters from command line */
ic = avformat_alloc_context();
if (!ic)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
if (o->nb_audio_sample_rate) {
av_dict_set_int(&o->g->format_opts, "sample_rate", o->audio_sample_rate[o->nb_audio_sample_rate - 1].u.i, 0);
}
@@ -1446,7 +1446,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
"Error opening input: %s\n", av_err2str(err));
if (err == AVERROR_PROTOCOL_NOT_FOUND)
av_log(d, AV_LOG_ERROR, "Did you mean file:%s?\n", filename);
- exit_program(1);
+ return err;
}
av_strlcat(d->log_name, "/", sizeof(d->log_name));
@@ -1477,7 +1477,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
av_log(d, AV_LOG_FATAL, "could not find codec parameters\n");
if (ic->nb_streams == 0) {
avformat_close_input(&ic);
- exit_program(1);
+ return ret;
}
}
}
@@ -1490,7 +1490,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
if (start_time_eof != AV_NOPTS_VALUE) {
if (start_time_eof >= 0) {
av_log(d, AV_LOG_ERROR, "-sseof value must be negative; aborting\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
if (ic->duration > 0) {
start_time = start_time_eof + ic->duration;
@@ -1547,7 +1547,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
f->readrate = o->readrate ? o->readrate : 0.0;
if (f->readrate < 0.0f) {
av_log(d, AV_LOG_ERROR, "Option -readrate is %0.3f; it must be non-negative.\n", f->readrate);
- exit_program(1);
+ return AVERROR(EINVAL);
}
if (o->rate_emu) {
if (f->readrate) {
@@ -1562,7 +1562,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
av_log(d, AV_LOG_ERROR,
"Option -readrate_initial_burst is %0.3f; it must be non-negative.\n",
d->readrate_initial_burst);
- exit_program(1);
+ return AVERROR(EINVAL);
}
} else if (o->readrate_initial_burst) {
av_log(d, AV_LOG_WARNING, "Option -readrate_initial_burst ignored "
@@ -1601,7 +1601,7 @@ int ifile_open(const OptionsContext *o, const char *filename)
if (!(option->flags & AV_OPT_FLAG_DECODING_PARAM)) {
av_log(d, AV_LOG_ERROR, "Codec AVOption %s (%s) is not a decoding "
"option.\n", e->key, option->help ? option->help : "");
- exit_program(1);
+ return AVERROR(EINVAL);
}
av_log(d, AV_LOG_WARNING, "Codec AVOption %s (%s) has not been used "
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 03/33] fftools/ffmpeg_demux: drop a redundant avio_flush()
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 02/33] fftools/ffmpeg_demux: return errors from ifile_open() " Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 04/33] fftools/ffmpeg_demux: forward errors from dump_attachment() instead of aborting Anton Khirnov
` (30 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
It is immediately followed by avio_close(), which is documented to flush
the buffers.
---
fftools/ffmpeg_demux.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index a6df2a1904..40bbfa0c0e 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1286,7 +1286,6 @@ static void dump_attachment(InputStream *ist, const char *filename)
}
avio_write(out, st->codecpar->extradata, st->codecpar->extradata_size);
- avio_flush(out);
avio_close(out);
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 04/33] fftools/ffmpeg_demux: forward errors from dump_attachment() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 02/33] fftools/ffmpeg_demux: return errors from ifile_open() " Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 03/33] fftools/ffmpeg_demux: drop a redundant avio_flush() Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 05/33] fftools/ffmpeg_demux: add logging for -dump_attachment Anton Khirnov
` (29 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
Also, check the return code of avio_close().
---
fftools/ffmpeg_demux.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 40bbfa0c0e..498830098d 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1259,7 +1259,7 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
ist->codec_desc = avcodec_descriptor_get(ist->par->codec_id);
}
-static void dump_attachment(InputStream *ist, const char *filename)
+static int dump_attachment(InputStream *ist, const char *filename)
{
AVStream *st = ist->st;
int ret;
@@ -1268,13 +1268,13 @@ static void dump_attachment(InputStream *ist, const char *filename)
if (!st->codecpar->extradata_size) {
av_log(ist, AV_LOG_WARNING, "No extradata to dump.\n");
- return;
+ return 0;
}
if (!*filename && (e = av_dict_get(st->metadata, "filename", NULL, 0)))
filename = e->value;
if (!*filename) {
av_log(ist, AV_LOG_FATAL, "No filename specified and no 'filename' tag");
- exit_program(1);
+ return AVERROR(EINVAL);
}
assert_file_overwrite(filename);
@@ -1282,11 +1282,13 @@ static void dump_attachment(InputStream *ist, const char *filename)
if ((ret = avio_open2(&out, filename, AVIO_FLAG_WRITE, &int_cb, NULL)) < 0) {
av_log(ist, AV_LOG_FATAL, "Could not open file %s for writing.\n",
filename);
- exit_program(1);
+ return ret;
}
avio_write(out, st->codecpar->extradata, st->codecpar->extradata_size);
- avio_close(out);
+ ret = avio_close(out);
+
+ return ret;
}
static const char *input_file_item_name(void *obj)
@@ -1617,8 +1619,11 @@ int ifile_open(const OptionsContext *o, const char *filename)
for (j = 0; j < f->nb_streams; j++) {
InputStream *ist = f->streams[j];
- if (check_stream_specifier(ic, ist->st, o->dump_attachment[i].specifier) == 1)
- dump_attachment(ist, o->dump_attachment[i].u.str);
+ if (check_stream_specifier(ic, ist->st, o->dump_attachment[i].specifier) == 1) {
+ ret = dump_attachment(ist, o->dump_attachment[i].u.str);
+ if (ret < 0)
+ return ret;
+ }
}
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 05/33] fftools/ffmpeg_demux: add logging for -dump_attachment
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (2 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 04/33] fftools/ffmpeg_demux: forward errors from dump_attachment() instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 06/33] fftools/ffmpeg: return errors from assert_file_overwrite() instead of aborting Anton Khirnov
` (28 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
Makes it more clear what was written where.
---
fftools/ffmpeg_demux.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 498830098d..ab99daf9f8 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1288,6 +1288,10 @@ static int dump_attachment(InputStream *ist, const char *filename)
avio_write(out, st->codecpar->extradata, st->codecpar->extradata_size);
ret = avio_close(out);
+ if (ret >= 0)
+ av_log(ist, AV_LOG_INFO, "Wrote attachment (%d bytes) to '%s'\n",
+ st->codecpar->extradata_size, filename);
+
return ret;
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 06/33] fftools/ffmpeg: return errors from assert_file_overwrite() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (3 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 05/33] fftools/ffmpeg_demux: add logging for -dump_attachment Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 07/33] fftools/ffmpeg_demux: return errors from ist_add() " Anton Khirnov
` (27 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg.h | 2 +-
fftools/ffmpeg_demux.c | 4 +++-
fftools/ffmpeg_mux_init.c | 11 ++++++++---
fftools/ffmpeg_opt.c | 12 +++++++-----
4 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 3201163a4f..30020cd0f8 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -700,7 +700,7 @@ void show_usage(void);
void remove_avoptions(AVDictionary **a, AVDictionary *b);
void assert_avoptions(AVDictionary *m);
-void assert_file_overwrite(const char *filename);
+int assert_file_overwrite(const char *filename);
char *file_read(const char *filename);
AVDictionary *strip_specifiers(const AVDictionary *dict);
const AVCodec *find_codec_or_die(void *logctx, const char *name,
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index ab99daf9f8..bc915178ec 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1277,7 +1277,9 @@ static int dump_attachment(InputStream *ist, const char *filename)
return AVERROR(EINVAL);
}
- assert_file_overwrite(filename);
+ ret = assert_file_overwrite(filename);
+ if (ret < 0)
+ return ret;
if ((ret = avio_open2(&out, filename, AVIO_FLAG_WRITE, &int_cb, NULL)) < 0) {
av_log(ist, AV_LOG_FATAL, "Could not open file %s for writing.\n",
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 8a3e7b98cf..4d40ceda05 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -2442,7 +2442,9 @@ int of_open(const OptionsContext *o, const char *filename)
if (!(oc->oformat->flags & AVFMT_NOFILE)) {
/* test if it already exists to avoid losing precious files */
- assert_file_overwrite(filename);
+ err = assert_file_overwrite(filename);
+ if (err < 0)
+ return err;
/* open the file */
if ((err = avio_open2(&oc->pb, filename, AVIO_FLAG_WRITE,
@@ -2452,8 +2454,11 @@ int of_open(const OptionsContext *o, const char *filename)
filename, av_err2str(err));
return err;
}
- } else if (strcmp(oc->oformat->name, "image2")==0 && !av_filename_number_test(filename))
- assert_file_overwrite(filename);
+ } else if (strcmp(oc->oformat->name, "image2")==0 && !av_filename_number_test(filename)) {
+ err = assert_file_overwrite(filename);
+ if (err < 0)
+ return err;
+ }
if (o->mux_preload) {
av_dict_set_int(&mux->opts, "preload", o->mux_preload*AV_TIME_BASE, 0);
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 7f22b22604..300f042660 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -652,13 +652,13 @@ const AVCodec *find_codec_or_die(void *logctx, const char *name,
return codec;
}
-void assert_file_overwrite(const char *filename)
+int assert_file_overwrite(const char *filename)
{
const char *proto_name = avio_find_protocol_name(filename);
if (file_overwrite && no_file_overwrite) {
fprintf(stderr, "Error, both -y and -n supplied. Exiting.\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
if (!file_overwrite) {
@@ -670,13 +670,13 @@ void assert_file_overwrite(const char *filename)
signal(SIGINT, SIG_DFL);
if (!read_yesno()) {
av_log(NULL, AV_LOG_FATAL, "Not overwriting - exiting\n");
- exit_program(1);
+ return AVERROR_EXIT;
}
term_init();
}
else {
av_log(NULL, AV_LOG_FATAL, "File '%s' already exists. Exiting.\n", filename);
- exit_program(1);
+ return AVERROR_EXIT;
}
}
}
@@ -689,10 +689,12 @@ void assert_file_overwrite(const char *filename)
if (!strcmp(filename, file->ctx->url)) {
av_log(NULL, AV_LOG_FATAL, "Output %s same as Input #%d - exiting\n", filename, i);
av_log(NULL, AV_LOG_WARNING, "FFmpeg cannot edit existing files in-place.\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
}
}
+
+ return 0;
}
/* read file contents into a string */
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 07/33] fftools/ffmpeg_demux: return errors from ist_add() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (4 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 06/33] fftools/ffmpeg: return errors from assert_file_overwrite() instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 08/33] fftools/ffmpeg_mux_init: return errors from create_streams() " Anton Khirnov
` (26 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_demux.c | 70 ++++++++++++++++++++++++++----------------
1 file changed, 43 insertions(+), 27 deletions(-)
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index bc915178ec..9e7f13a2b4 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -964,8 +964,8 @@ static int guess_input_channel_layout(InputStream *ist, int guess_layout_max)
return 1;
}
-static void add_display_matrix_to_stream(const OptionsContext *o,
- AVFormatContext *ctx, InputStream *ist)
+static int add_display_matrix_to_stream(const OptionsContext *o,
+ AVFormatContext *ctx, InputStream *ist)
{
AVStream *st = ist->st;
double rotation = DBL_MAX;
@@ -982,12 +982,12 @@ static void add_display_matrix_to_stream(const OptionsContext *o,
vflip_set = vflip != -1;
if (!rotation_set && !hflip_set && !vflip_set)
- return;
+ return 0;
buf = (int32_t *)av_stream_new_side_data(st, AV_PKT_DATA_DISPLAYMATRIX, sizeof(int32_t) * 9);
if (!buf) {
av_log(ist, AV_LOG_FATAL, "Failed to generate a display matrix!\n");
- exit_program(1);
+ return AVERROR(ENOMEM);
}
av_display_rotation_set(buf,
@@ -996,6 +996,8 @@ static void add_display_matrix_to_stream(const OptionsContext *o,
av_display_matrix_flip(buf,
hflip_set ? hflip : 0,
vflip_set ? vflip : 0);
+
+ return 0;
}
static const char *input_stream_item_name(void *obj)
@@ -1031,7 +1033,7 @@ static DemuxStream *demux_stream_alloc(Demuxer *d, AVStream *st)
return ds;
}
-static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
+static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
{
AVFormatContext *ic = d->f.ctx;
AVCodecParameters *par = st->codecpar;
@@ -1079,7 +1081,9 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
}
if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
- add_display_matrix_to_stream(o, ic, ist);
+ ret = add_display_matrix_to_stream(o, ic, ist);
+ if (ret < 0)
+ return ret;
MATCH_PER_STREAM_OPT(hwaccels, str, hwaccel, ic, st);
MATCH_PER_STREAM_OPT(hwaccel_output_formats, str,
@@ -1137,7 +1141,7 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
av_log(ist, AV_LOG_FATAL, "%s ",
av_hwdevice_get_type_name(type));
av_log(ist, AV_LOG_FATAL, "\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
}
}
@@ -1146,7 +1150,7 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
if (hwaccel_device) {
ist->hwaccel_device = av_strdup(hwaccel_device);
if (!ist->hwaccel_device)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
}
}
@@ -1165,20 +1169,22 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
(o->data_disable && ist->st->codecpar->codec_type == AVMEDIA_TYPE_DATA))
ist->user_set_discard = AVDISCARD_ALL;
- if (discard_str && av_opt_eval_int(&cc, discard_opt, discard_str, &ist->user_set_discard) < 0) {
- av_log(ist, AV_LOG_ERROR, "Error parsing discard %s.\n",
- discard_str);
- exit_program(1);
+ if (discard_str) {
+ ret = av_opt_eval_int(&cc, discard_opt, discard_str, &ist->user_set_discard);
+ if (ret < 0) {
+ av_log(ist, AV_LOG_ERROR, "Error parsing discard %s.\n", discard_str);
+ return ret;
+ }
}
ist->dec_ctx = avcodec_alloc_context3(ist->dec);
if (!ist->dec_ctx)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
ret = avcodec_parameters_to_context(ist->dec_ctx, par);
if (ret < 0) {
av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
- exit_program(1);
+ return ret;
}
if (o->bitexact)
@@ -1187,11 +1193,13 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
switch (par->codec_type) {
case AVMEDIA_TYPE_VIDEO:
MATCH_PER_STREAM_OPT(frame_rates, str, framerate, ic, st);
- if (framerate && av_parse_video_rate(&ist->framerate,
- framerate) < 0) {
- av_log(ist, AV_LOG_ERROR, "Error parsing framerate %s.\n",
- framerate);
- exit_program(1);
+ if (framerate) {
+ ret = av_parse_video_rate(&ist->framerate, framerate);
+ if (ret < 0) {
+ av_log(ist, AV_LOG_ERROR, "Error parsing framerate %s.\n",
+ framerate);
+ return ret;
+ }
}
ist->top_field_first = -1;
@@ -1211,10 +1219,13 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
char *canvas_size = NULL;
MATCH_PER_STREAM_OPT(fix_sub_duration, i, ist->fix_sub_duration, ic, st);
MATCH_PER_STREAM_OPT(canvas_sizes, str, canvas_size, ic, st);
- if (canvas_size &&
- av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height, canvas_size) < 0) {
- av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size);
- exit_program(1);
+ if (canvas_size) {
+ ret = av_parse_video_size(&ist->dec_ctx->width, &ist->dec_ctx->height,
+ canvas_size);
+ if (ret < 0) {
+ av_log(ist, AV_LOG_FATAL, "Invalid canvas size: %s.\n", canvas_size);
+ return ret;
+ }
}
/* Compute the size of the canvas for the subtitles stream.
@@ -1248,15 +1259,17 @@ static void ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
ist->par = avcodec_parameters_alloc();
if (!ist->par)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx);
if (ret < 0) {
av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
- exit_program(1);
+ return ret;
}
ist->codec_desc = avcodec_descriptor_get(ist->par->codec_id);
+
+ return 0;
}
static int dump_attachment(InputStream *ist, const char *filename)
@@ -1579,8 +1592,11 @@ int ifile_open(const OptionsContext *o, const char *filename)
d->thread_queue_size = o->thread_queue_size;
/* Add all the streams from the given input file to the demuxer */
- for (int i = 0; i < ic->nb_streams; i++)
- ist_add(o, d, ic->streams[i]);
+ for (int i = 0; i < ic->nb_streams; i++) {
+ ret = ist_add(o, d, ic->streams[i]);
+ if (ret < 0)
+ return ret;
+ }
/* dump the file content */
av_dump_format(ic, f->index, filename, 0);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 08/33] fftools/ffmpeg_mux_init: return errors from create_streams() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (5 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 07/33] fftools/ffmpeg_demux: return errors from ist_add() " Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 09/33] fftools/ffmpeg_mux_init: improve error handling in of_add_attachments() Anton Khirnov
` (25 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_mux_init.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 4d40ceda05..dbc58abea8 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1558,7 +1558,7 @@ static void of_add_attachments(Muxer *mux, const OptionsContext *o)
}
}
-static void create_streams(Muxer *mux, const OptionsContext *o)
+static int create_streams(Muxer *mux, const OptionsContext *o)
{
AVFormatContext *oc = mux->fc;
int auto_disable_v = o->video_disable;
@@ -1616,8 +1616,10 @@ static void create_streams(Muxer *mux, const OptionsContext *o)
if (!oc->nb_streams && !(oc->oformat->flags & AVFMT_NOSTREAMS)) {
av_dump_format(oc, nb_output_files - 1, oc->url, 1);
av_log(mux, AV_LOG_ERROR, "Output file does not contain any stream\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
+
+ return 0;
}
static int setup_sync_queues(Muxer *mux, AVFormatContext *oc, int64_t buf_size_us)
@@ -2426,7 +2428,9 @@ int of_open(const OptionsContext *o, const char *filename)
}
/* create all output streams for this file */
- create_streams(mux, o);
+ err = create_streams(mux, o);
+ if (err < 0)
+ return err;
/* check if all codec options have been used */
validate_enc_avopt(mux, o->g->codec_opts);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 09/33] fftools/ffmpeg_mux_init: improve error handling in of_add_attachments()
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (6 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 08/33] fftools/ffmpeg_mux_init: return errors from create_streams() " Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 10/33] fftools/ffmpeg_mux_init: return error codes from map_*() instead of aborting Anton Khirnov
` (24 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
* return error codes instead of aborting
* avoid leaking the AVIOContext on failure
* check the return code of avio_read()
---
fftools/ffmpeg_mux_init.c | 44 +++++++++++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 9 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index dbc58abea8..47b6f826b6 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1514,7 +1514,7 @@ loop_end:
}
}
-static void of_add_attachments(Muxer *mux, const OptionsContext *o)
+static int of_add_attachments(Muxer *mux, const OptionsContext *o)
{
OutputStream *ost;
int err;
@@ -1528,20 +1528,42 @@ static void of_add_attachments(Muxer *mux, const OptionsContext *o)
if ((err = avio_open2(&pb, o->attachments[i], AVIO_FLAG_READ, &int_cb, NULL)) < 0) {
av_log(mux, AV_LOG_FATAL, "Could not open attachment file %s.\n",
o->attachments[i]);
- exit_program(1);
+ return err;
}
if ((len = avio_size(pb)) <= 0) {
av_log(mux, AV_LOG_FATAL, "Could not get size of the attachment %s.\n",
o->attachments[i]);
- exit_program(1);
+ err = len ? len : AVERROR_INVALIDDATA;
+ goto read_fail;
}
- if (len > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE ||
- !(attachment = av_malloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) {
+ if (len > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) {
av_log(mux, AV_LOG_FATAL, "Attachment %s too large.\n",
o->attachments[i]);
- exit_program(1);
+ err = AVERROR(ERANGE);
+ goto read_fail;
}
- avio_read(pb, attachment, len);
+
+ attachment = av_malloc(len + AV_INPUT_BUFFER_PADDING_SIZE);
+ if (!attachment) {
+ err = AVERROR(ENOMEM);
+ goto read_fail;
+ }
+
+ err = avio_read(pb, attachment, len);
+ if (err < 0)
+ av_log(mux, AV_LOG_FATAL, "Error reading attachment file %s: %s\n",
+ o->attachments[i], av_err2str(err));
+ else if (err != len) {
+ av_log(mux, AV_LOG_FATAL, "Could not read all %"PRId64" bytes for "
+ "attachment file %s\n", len, o->attachments[i]);
+ err = AVERROR(EIO);
+ }
+
+read_fail:
+ avio_closep(&pb);
+ if (err < 0)
+ return err;
+
memset(attachment + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
av_log(mux, AV_LOG_VERBOSE, "Creating attachment stream from file %s\n",
@@ -1554,8 +1576,9 @@ static void of_add_attachments(Muxer *mux, const OptionsContext *o)
p = strrchr(o->attachments[i], '/');
av_dict_set(&ost->st->metadata, "filename", (p && *p) ? p + 1 : o->attachments[i], AV_DICT_DONT_OVERWRITE);
- avio_closep(&pb);
}
+
+ return 0;
}
static int create_streams(Muxer *mux, const OptionsContext *o)
@@ -1565,6 +1588,7 @@ static int create_streams(Muxer *mux, const OptionsContext *o)
int auto_disable_a = o->audio_disable;
int auto_disable_s = o->subtitle_disable;
int auto_disable_d = o->data_disable;
+ int ret;
/* create streams for all unlabeled output pads */
for (int i = 0; i < nb_filtergraphs; i++) {
@@ -1611,7 +1635,9 @@ static int create_streams(Muxer *mux, const OptionsContext *o)
map_manual(mux, o, &o->stream_maps[i]);
}
- of_add_attachments(mux, o);
+ ret = of_add_attachments(mux, o);
+ if (ret < 0)
+ return ret;
if (!oc->nb_streams && !(oc->oformat->flags & AVFMT_NOSTREAMS)) {
av_dump_format(oc, nb_output_files - 1, oc->url, 1);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 10/33] fftools/ffmpeg_mux_init: return error codes from map_*() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (7 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 09/33] fftools/ffmpeg_mux_init: improve error handling in of_add_attachments() Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 11/33] fftools/ffmpeg_mux_init: move allocation out of prologue Anton Khirnov
` (23 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_mux_init.c | 92 +++++++++++++++++++++++----------------
1 file changed, 55 insertions(+), 37 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 47b6f826b6..8e640610cd 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1297,7 +1297,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
return ost;
}
-static void map_auto_video(Muxer *mux, const OptionsContext *o)
+static int map_auto_video(Muxer *mux, const OptionsContext *o)
{
AVFormatContext *oc = mux->fc;
InputStream *best_ist = NULL;
@@ -1306,7 +1306,7 @@ static void map_auto_video(Muxer *mux, const OptionsContext *o)
/* video: highest resolution */
if (av_guess_codec(oc->oformat, NULL, oc->url, NULL, AVMEDIA_TYPE_VIDEO) == AV_CODEC_ID_NONE)
- return;
+ return 0;
qcr = avformat_query_codec(oc->oformat, oc->oformat->video_codec, 0);
for (int j = 0; j < nb_input_files; j++) {
@@ -1346,9 +1346,11 @@ static void map_auto_video(Muxer *mux, const OptionsContext *o)
}
if (best_ist)
ost_add(mux, o, AVMEDIA_TYPE_VIDEO, best_ist, NULL);
+
+ return 0;
}
-static void map_auto_audio(Muxer *mux, const OptionsContext *o)
+static int map_auto_audio(Muxer *mux, const OptionsContext *o)
{
AVFormatContext *oc = mux->fc;
InputStream *best_ist = NULL;
@@ -1356,7 +1358,7 @@ static void map_auto_audio(Muxer *mux, const OptionsContext *o)
/* audio: most channels */
if (av_guess_codec(oc->oformat, NULL, oc->url, NULL, AVMEDIA_TYPE_AUDIO) == AV_CODEC_ID_NONE)
- return;
+ return 0;
for (int j = 0; j < nb_input_files; j++) {
InputFile *ifile = input_files[j];
@@ -1388,9 +1390,11 @@ static void map_auto_audio(Muxer *mux, const OptionsContext *o)
}
if (best_ist)
ost_add(mux, o, AVMEDIA_TYPE_AUDIO, best_ist, NULL);
+
+ return 0;
}
-static void map_auto_subtitle(Muxer *mux, const OptionsContext *o)
+static int map_auto_subtitle(Muxer *mux, const OptionsContext *o)
{
AVFormatContext *oc = mux->fc;
char *subtitle_codec_name = NULL;
@@ -1398,7 +1402,7 @@ static void map_auto_subtitle(Muxer *mux, const OptionsContext *o)
/* subtitles: pick first */
MATCH_PER_TYPE_OPT(codec_names, str, subtitle_codec_name, oc, "s");
if (!avcodec_find_encoder(oc->oformat->subtitle_codec) && !subtitle_codec_name)
- return;
+ return 0;
for (InputStream *ist = ist_iter(NULL); ist; ist = ist_iter(ist))
if (ist->st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE) {
@@ -1426,16 +1430,18 @@ static void map_auto_subtitle(Muxer *mux, const OptionsContext *o)
break;
}
}
+
+ return 0;
}
-static void map_auto_data(Muxer *mux, const OptionsContext *o)
+static int map_auto_data(Muxer *mux, const OptionsContext *o)
{
AVFormatContext *oc = mux->fc;
/* Data only if codec id match */
enum AVCodecID codec_id = av_guess_codec(oc->oformat, NULL, oc->url, NULL, AVMEDIA_TYPE_DATA);
if (codec_id == AV_CODEC_ID_NONE)
- return;
+ return 0;
for (InputStream *ist = ist_iter(NULL); ist; ist = ist_iter(ist)) {
if (ist->user_set_discard == AVDISCARD_ALL)
@@ -1444,14 +1450,16 @@ static void map_auto_data(Muxer *mux, const OptionsContext *o)
ist->st->codecpar->codec_id == codec_id )
ost_add(mux, o, AVMEDIA_TYPE_DATA, ist, NULL);
}
+
+ return 0;
}
-static void map_manual(Muxer *mux, const OptionsContext *o, const StreamMap *map)
+static int map_manual(Muxer *mux, const OptionsContext *o, const StreamMap *map)
{
InputStream *ist;
if (map->disabled)
- return;
+ return 0;
if (map->linklabel) {
FilterGraph *fg;
@@ -1472,7 +1480,7 @@ loop_end:
if (!ofilter) {
av_log(mux, AV_LOG_FATAL, "Output with label '%s' does not exist "
"in any defined filter graph, or was already used elsewhere.\n", map->linklabel);
- exit_program(1);
+ return AVERROR(EINVAL);
}
av_log(mux, AV_LOG_VERBOSE, "Creating output stream from an explicitly "
@@ -1484,16 +1492,16 @@ loop_end:
if (ist->user_set_discard == AVDISCARD_ALL) {
av_log(mux, AV_LOG_FATAL, "Stream #%d:%d is disabled and cannot be mapped.\n",
map->file_index, map->stream_index);
- exit_program(1);
+ return AVERROR(EINVAL);
}
if(o->subtitle_disable && ist->st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
- return;
+ return 0;
if(o-> audio_disable && ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
- return;
+ return 0;
if(o-> video_disable && ist->st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)
- return;
+ return 0;
if(o-> data_disable && ist->st->codecpar->codec_type == AVMEDIA_TYPE_DATA)
- return;
+ return 0;
if (ist->st->codecpar->codec_type == AVMEDIA_TYPE_UNKNOWN &&
!copy_unknown_streams) {
@@ -1505,13 +1513,15 @@ loop_end:
"If you want unsupported types ignored instead "
"of failing, please use the -ignore_unknown option\n"
"If you want them copied, please use -copy_unknown\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
- return;
+ return 0;
}
ost_add(mux, o, ist->st->codecpar->codec_type, ist, NULL);
}
+
+ return 0;
}
static int of_add_attachments(Muxer *mux, const OptionsContext *o)
@@ -1583,11 +1593,21 @@ read_fail:
static int create_streams(Muxer *mux, const OptionsContext *o)
{
+ static const int (*map_func[])(Muxer *mux, const OptionsContext *o) = {
+ [AVMEDIA_TYPE_VIDEO] = map_auto_video,
+ [AVMEDIA_TYPE_AUDIO] = map_auto_audio,
+ [AVMEDIA_TYPE_SUBTITLE] = map_auto_subtitle,
+ [AVMEDIA_TYPE_DATA] = map_auto_data,
+ };
+
AVFormatContext *oc = mux->fc;
- int auto_disable_v = o->video_disable;
- int auto_disable_a = o->audio_disable;
- int auto_disable_s = o->subtitle_disable;
- int auto_disable_d = o->data_disable;
+
+ int auto_disable =
+ o->video_disable * (1 << AVMEDIA_TYPE_VIDEO) |
+ o->audio_disable * (1 << AVMEDIA_TYPE_AUDIO) |
+ o->subtitle_disable * (1 << AVMEDIA_TYPE_SUBTITLE) |
+ o->data_disable * (1 << AVMEDIA_TYPE_DATA);
+
int ret;
/* create streams for all unlabeled output pads */
@@ -1599,11 +1619,7 @@ static int create_streams(Muxer *mux, const OptionsContext *o)
if (ofilter->linklabel || ofilter->ost)
continue;
- switch (ofilter->type) {
- case AVMEDIA_TYPE_VIDEO: auto_disable_v = 1; break;
- case AVMEDIA_TYPE_AUDIO: auto_disable_a = 1; break;
- case AVMEDIA_TYPE_SUBTITLE: auto_disable_s = 1; break;
- }
+ auto_disable |= 1 << ofilter->type;
av_log(mux, AV_LOG_VERBOSE, "Creating output stream from unlabeled "
"output of complex filtergraph %d.", fg->index);
@@ -1620,19 +1636,21 @@ static int create_streams(Muxer *mux, const OptionsContext *o)
av_log(mux, AV_LOG_VERBOSE, "No explicit maps, mapping streams automatically...\n");
/* pick the "best" stream of each type */
- if (!auto_disable_v)
- map_auto_video(mux, o);
- if (!auto_disable_a)
- map_auto_audio(mux, o);
- if (!auto_disable_s)
- map_auto_subtitle(mux, o);
- if (!auto_disable_d)
- map_auto_data(mux, o);
+ for (int i = 0; i < FF_ARRAY_ELEMS(map_func); i++) {
+ if (!map_func[i] || auto_disable & (1 << i))
+ continue;
+ ret = map_func[i](mux, o);
+ if (ret < 0)
+ return ret;
+ }
} else {
av_log(mux, AV_LOG_VERBOSE, "Adding streams from explicit maps...\n");
- for (int i = 0; i < o->nb_stream_maps; i++)
- map_manual(mux, o, &o->stream_maps[i]);
+ for (int i = 0; i < o->nb_stream_maps; i++) {
+ ret = map_manual(mux, o, &o->stream_maps[i]);
+ if (ret < 0)
+ return ret;
+ }
}
ret = of_add_attachments(mux, o);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 11/33] fftools/ffmpeg_mux_init: move allocation out of prologue
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (8 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 10/33] fftools/ffmpeg_mux_init: return error codes from map_*() instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 12/33] fftools/ffmpeg_mux_init: return error codes from ost_add() instead of aborting Anton Khirnov
` (22 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
ost_add() has a very large variable declaration prologue, performing
"active" actions that can fail in there is confusing.
---
fftools/ffmpeg_mux_init.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 8e640610cd..bd4c0a9f97 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -987,13 +987,14 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
MuxStream *ms;
OutputStream *ost;
const AVCodec *enc;
- AVStream *st = avformat_new_stream(oc, NULL);
+ AVStream *st;
int ret = 0;
const char *bsfs = NULL, *time_base = NULL;
char *filters = NULL, *next, *codec_tag = NULL;
double qscale = -1;
int i;
+ st = avformat_new_stream(oc, NULL);
if (!st)
report_and_exit(AVERROR(ENOMEM));
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 12/33] fftools/ffmpeg_mux_init: return error codes from ost_add() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (9 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 11/33] fftools/ffmpeg_mux_init: move allocation out of prologue Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 13/33] fftools/ffmpeg_mux_init: return error codes from copy_meta() " Anton Khirnov
` (21 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_mux_init.c | 88 +++++++++++++++++++++++----------------
1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index bd4c0a9f97..8ad799f951 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -979,9 +979,9 @@ fail:
return ret;
}
-static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
- enum AVMediaType type, InputStream *ist,
- OutputFilter *ofilter)
+static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
+ InputStream *ist, OutputFilter *ofilter,
+ OutputStream **post)
{
AVFormatContext *oc = mux->fc;
MuxStream *ms;
@@ -996,7 +996,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
st = avformat_new_stream(oc, NULL);
if (!st)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
if (oc->nb_streams - 1 < o->nb_streamid_map)
st->id = o->streamid_map[oc->nb_streams - 1];
@@ -1006,11 +1006,11 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ost->par_in = avcodec_parameters_alloc();
if (!ost->par_in)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
ms->muxing_queue = av_fifo_alloc2(8, sizeof(AVPacket*), 0);
if (!ms->muxing_queue)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
ms->last_mux_dts = AV_NOPTS_VALUE;
ost->st = st;
@@ -1022,17 +1022,17 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ret = choose_encoder(o, oc, ost, &enc);
if (ret < 0) {
av_log(ost, AV_LOG_FATAL, "Error selecting an encoder\n");
- exit_program(1);
+ return ret;
}
if (enc) {
ost->enc_ctx = avcodec_alloc_context3(enc);
if (!ost->enc_ctx)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
ret = enc_alloc(&ost->enc, enc);
if (ret < 0)
- report_and_exit(ret);
+ return ret;
av_strlcat(ms->log_name, "/", sizeof(ms->log_name));
av_strlcat(ms->log_name, enc->name, sizeof(ms->log_name));
@@ -1042,7 +1042,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
"Streamcopy requested for output stream fed "
"from a complex filtergraph. Filtering and streamcopy "
"cannot be used together.\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
av_strlcat(ms->log_name, "/copy", sizeof(ms->log_name));
@@ -1063,7 +1063,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ms->pkt = av_packet_alloc();
if (!ms->pkt)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
if (ost->enc_ctx) {
AVCodecContext *enc = ost->enc_ctx;
@@ -1088,7 +1088,8 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
continue;
if (!(arg = strchr(buf, '='))) {
av_log(ost, AV_LOG_FATAL, "Invalid line found in the preset file.\n");
- exit_program(1);
+ ret = AVERROR(EINVAL);
+ break;
}
*arg++ = 0;
av_dict_set(&ost->encoder_opts, buf, arg, AV_DICT_DONT_OVERWRITE);
@@ -1099,7 +1100,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
if (ret) {
av_log(ost, AV_LOG_FATAL,
"Preset %s specified, but could not be opened.\n", preset);
- exit_program(1);
+ return ret;
}
MATCH_PER_STREAM_OPT(enc_stats_pre, str, enc_stats_pre, oc, st);
@@ -1111,7 +1112,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ret = enc_stats_init(ost, &ost->enc_stats_pre, 1, enc_stats_pre, format);
if (ret < 0)
- exit_program(1);
+ return ret;
}
MATCH_PER_STREAM_OPT(enc_stats_post, str, enc_stats_post, oc, st);
@@ -1123,7 +1124,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ret = enc_stats_init(ost, &ost->enc_stats_post, 0, enc_stats_post, format);
if (ret < 0)
- exit_program(1);
+ return ret;
}
MATCH_PER_STREAM_OPT(mux_stats, str, mux_stats, oc, st);
@@ -1135,7 +1136,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ret = enc_stats_init(ost, &ms->stats, 0, mux_stats, format);
if (ret < 0)
- exit_program(1);
+ return ret;
}
MATCH_PER_STREAM_OPT(enc_time_bases, str, enc_time_base, oc, st);
@@ -1144,14 +1145,14 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
if (av_parse_ratio(&q, enc_time_base, INT_MAX, 0, NULL) < 0 ||
q.den <= 0) {
av_log(ost, AV_LOG_FATAL, "Invalid time base: %s\n", enc_time_base);
- exit_program(1);
+ return AVERROR(EINVAL);
}
if (q.num < 0) {
if (!ost->ist) {
av_log(ost, AV_LOG_FATAL,
"Cannot use input stream timebase for encoding - "
"no input stream available\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
q = ost->ist->st->time_base;
}
@@ -1175,7 +1176,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
if (av_parse_ratio(&q, time_base, INT_MAX, 0, NULL) < 0 ||
q.num <= 0 || q.den <= 0) {
av_log(ost, AV_LOG_FATAL, "Invalid time base: %s\n", time_base);
- exit_program(1);
+ return AVERROR(EINVAL);
}
st->time_base = q;
}
@@ -1198,7 +1199,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ret = av_bsf_list_parse_str(bsfs, &ms->bsf_ctx);
if (ret < 0) {
av_log(ost, AV_LOG_ERROR, "Error parsing bitstream filter sequence '%s': %s\n", bsfs, av_err2str(ret));
- exit_program(1);
+ return ret;
}
}
@@ -1258,7 +1259,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
if (type == AVMEDIA_TYPE_VIDEO || type == AVMEDIA_TYPE_AUDIO) {
ret = ost_get_filters(o, oc, ost, &filters);
if (ret < 0)
- exit_program(1);
+ return ret;
}
if (ost->enc &&
@@ -1271,7 +1272,7 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
if (ret < 0) {
av_log(ost, AV_LOG_ERROR,
"Error initializing a simple filtergraph\n");
- exit_program(1);
+ return ret;
}
}
} else if (ost->ist) {
@@ -1279,14 +1280,14 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
if (ret < 0) {
av_log(ost, AV_LOG_ERROR,
"Error binding an input stream\n");
- exit_program(1);
+ return ret;
}
}
if (ost->ist && !ost->enc) {
ret = streamcopy_init(mux, ost);
if (ret < 0)
- exit_program(1);
+ return ret;
}
// copy estimated duration as a hint to the muxer
@@ -1295,7 +1296,10 @@ static OutputStream *ost_add(Muxer *mux, const OptionsContext *o,
ms->stream_duration_tb = ist->st->time_base;
}
- return ost;
+ if (post)
+ *post = ost;
+
+ return 0;
}
static int map_auto_video(Muxer *mux, const OptionsContext *o)
@@ -1346,7 +1350,7 @@ static int map_auto_video(Muxer *mux, const OptionsContext *o)
}
}
if (best_ist)
- ost_add(mux, o, AVMEDIA_TYPE_VIDEO, best_ist, NULL);
+ return ost_add(mux, o, AVMEDIA_TYPE_VIDEO, best_ist, NULL, NULL);
return 0;
}
@@ -1390,7 +1394,7 @@ static int map_auto_audio(Muxer *mux, const OptionsContext *o)
}
}
if (best_ist)
- ost_add(mux, o, AVMEDIA_TYPE_AUDIO, best_ist, NULL);
+ return ost_add(mux, o, AVMEDIA_TYPE_AUDIO, best_ist, NULL, NULL);
return 0;
}
@@ -1427,8 +1431,7 @@ static int map_auto_subtitle(Muxer *mux, const OptionsContext *o)
input_descriptor && output_descriptor &&
(!input_descriptor->props ||
!output_descriptor->props)) {
- ost_add(mux, o, AVMEDIA_TYPE_SUBTITLE, ist, NULL);
- break;
+ return ost_add(mux, o, AVMEDIA_TYPE_SUBTITLE, ist, NULL, NULL);
}
}
@@ -1448,8 +1451,11 @@ static int map_auto_data(Muxer *mux, const OptionsContext *o)
if (ist->user_set_discard == AVDISCARD_ALL)
continue;
if (ist->st->codecpar->codec_type == AVMEDIA_TYPE_DATA &&
- ist->st->codecpar->codec_id == codec_id )
- ost_add(mux, o, AVMEDIA_TYPE_DATA, ist, NULL);
+ ist->st->codecpar->codec_id == codec_id) {
+ int ret = ost_add(mux, o, AVMEDIA_TYPE_DATA, ist, NULL, NULL);
+ if (ret < 0)
+ return ret;
+ }
}
return 0;
@@ -1458,6 +1464,7 @@ static int map_auto_data(Muxer *mux, const OptionsContext *o)
static int map_manual(Muxer *mux, const OptionsContext *o, const StreamMap *map)
{
InputStream *ist;
+ int ret;
if (map->disabled)
return 0;
@@ -1487,7 +1494,9 @@ loop_end:
av_log(mux, AV_LOG_VERBOSE, "Creating output stream from an explicitly "
"mapped complex filtergraph %d, output [%s]\n", fg->index, map->linklabel);
- ost_add(mux, o, ofilter->type, NULL, ofilter);
+ ret = ost_add(mux, o, ofilter->type, NULL, ofilter, NULL);
+ if (ret < 0)
+ return ret;
} else {
ist = input_files[map->file_index]->streams[map->stream_index];
if (ist->user_set_discard == AVDISCARD_ALL) {
@@ -1519,7 +1528,9 @@ loop_end:
return 0;
}
- ost_add(mux, o, ist->st->codecpar->codec_type, ist, NULL);
+ ret = ost_add(mux, o, ist->st->codecpar->codec_type, ist, NULL, NULL);
+ if (ret < 0)
+ return ret;
}
return 0;
@@ -1580,7 +1591,12 @@ read_fail:
av_log(mux, AV_LOG_VERBOSE, "Creating attachment stream from file %s\n",
o->attachments[i]);
- ost = ost_add(mux, o, AVMEDIA_TYPE_ATTACHMENT, NULL, NULL);
+ err = ost_add(mux, o, AVMEDIA_TYPE_ATTACHMENT, NULL, NULL, &ost);
+ if (err < 0) {
+ av_freep(&attachment);
+ return err;
+ }
+
ost->attachment_filename = o->attachments[i];
ost->par_in->extradata = attachment;
ost->par_in->extradata_size = len;
@@ -1629,7 +1645,9 @@ static int create_streams(Muxer *mux, const OptionsContext *o)
av_get_media_type_string(ofilter->type));
av_log(mux, AV_LOG_VERBOSE, "\n");
- ost_add(mux, o, ofilter->type, NULL, ofilter);
+ ret = ost_add(mux, o, ofilter->type, NULL, ofilter, NULL);
+ if (ret < 0)
+ return ret;
}
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 13/33] fftools/ffmpeg_mux_init: return error codes from copy_meta() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (10 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 12/33] fftools/ffmpeg_mux_init: return error codes from ost_add() instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 14/33] fftools/ffmpeg_mux_init: return error codes from parse_forced_key_frames() " Anton Khirnov
` (20 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_mux_init.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 8ad799f951..f85357d8e4 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -2089,7 +2089,7 @@ static int copy_metadata(Muxer *mux, AVFormatContext *ic,
return 0;
}
-static void copy_meta(Muxer *mux, const OptionsContext *o)
+static int copy_meta(Muxer *mux, const OptionsContext *o)
{
OutputFile *of = &mux->of;
AVFormatContext *oc = mux->fc;
@@ -2106,7 +2106,7 @@ static void copy_meta(Muxer *mux, const OptionsContext *o)
if (in_file_index >= nb_input_files) {
av_log(mux, AV_LOG_FATAL, "Invalid input file index %d while "
"processing metadata maps\n", in_file_index);
- exit_program(1);
+ return AVERROR(EINVAL);
}
copy_metadata(mux,
in_file_index >= 0 ? input_files[in_file_index]->ctx : NULL,
@@ -2128,7 +2128,7 @@ static void copy_meta(Muxer *mux, const OptionsContext *o)
} else {
av_log(mux, AV_LOG_FATAL, "Invalid input file index %d in chapter mapping.\n",
chapters_input_file);
- exit_program(1);
+ return AVERROR(EINVAL);
}
}
if (chapters_input_file >= 0)
@@ -2157,6 +2157,8 @@ static void copy_meta(Muxer *mux, const OptionsContext *o)
av_dict_set(&ost->st->metadata, "encoder", NULL, 0);
}
}
+
+ return 0;
}
static int set_dispositions(Muxer *mux, const OptionsContext *o)
@@ -2533,7 +2535,9 @@ int of_open(const OptionsContext *o, const char *filename)
oc->max_delay = (int)(o->mux_max_delay * AV_TIME_BASE);
/* copy metadata and chapters from input files */
- copy_meta(mux, o);
+ err = copy_meta(mux, o);
+ if (err < 0)
+ return err;
of_add_programs(mux, o);
of_add_metadata(of, oc, o);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 14/33] fftools/ffmpeg_mux_init: return error codes from parse_forced_key_frames() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (11 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 13/33] fftools/ffmpeg_mux_init: return error codes from copy_meta() " Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 15/33] fftools/ffmpeg_mux_init: return error codes from validate_enc_avopt() " Anton Khirnov
` (19 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_mux_init.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index f85357d8e4..7a8c935795 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -2245,8 +2245,8 @@ static int compare_int64(const void *a, const void *b)
return FFDIFFSIGN(*(const int64_t *)a, *(const int64_t *)b);
}
-static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
- const char *spec)
+static int parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
+ const char *spec)
{
const char *p;
int n = 1, i, size, index = 0;
@@ -2258,7 +2258,7 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
size = n;
pts = av_malloc_array(size, sizeof(*pts));
if (!pts)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
p = spec;
for (i = 0; i < n; i++) {
@@ -2275,7 +2275,7 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
if (nb_ch > INT_MAX - size ||
!(pts = av_realloc_f(pts, size += nb_ch - 1,
sizeof(*pts))))
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
t = p[8] ? parse_time_or_die("force_key_frames", p + 8, 1) : 0;
for (j = 0; j < nb_ch; j++) {
@@ -2297,6 +2297,8 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux,
qsort(pts, size, sizeof(*pts), compare_int64);
kf->nb_pts = size;
kf->pts = pts;
+
+ return 0;
}
static int process_forced_keyframes(Muxer *mux, const OptionsContext *o)
@@ -2331,7 +2333,9 @@ static int process_forced_keyframes(Muxer *mux, const OptionsContext *o)
} else if (!strcmp(forced_keyframes, "source_no_drop")) {
ost->kf.type = KF_FORCE_SOURCE_NO_DROP;
} else {
- parse_forced_key_frames(&ost->kf, mux, forced_keyframes);
+ int ret = parse_forced_key_frames(&ost->kf, mux, forced_keyframes);
+ if (ret < 0)
+ return ret;
}
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 15/33] fftools/ffmpeg_mux_init: return error codes from validate_enc_avopt() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (12 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 14/33] fftools/ffmpeg_mux_init: return error codes from parse_forced_key_frames() " Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 16/33] fftools/ffmpeg_mux_init: improve of_add_programs() Anton Khirnov
` (18 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_mux_init.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 7a8c935795..3a327048bb 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -2342,7 +2342,7 @@ static int process_forced_keyframes(Muxer *mux, const OptionsContext *o)
return 0;
}
-static void validate_enc_avopt(Muxer *mux, const AVDictionary *codec_avopt)
+static int validate_enc_avopt(Muxer *mux, const AVDictionary *codec_avopt)
{
const AVClass *class = avcodec_get_class();
const AVClass *fclass = avformat_get_class();
@@ -2370,7 +2370,7 @@ static void validate_enc_avopt(Muxer *mux, const AVDictionary *codec_avopt)
if (!(option->flags & AV_OPT_FLAG_ENCODING_PARAM)) {
av_log(mux, AV_LOG_ERROR, "Codec AVOption %s (%s) is not an "
"encoding option.\n", e->key, option->help ? option->help : "");
- exit_program(1);
+ return AVERROR(EINVAL);
}
// gop_timecode is injected by generic code but not always used
@@ -2384,6 +2384,8 @@ static void validate_enc_avopt(Muxer *mux, const AVDictionary *codec_avopt)
"any stream.\n", e->key, option->help ? option->help : "");
}
av_dict_free(&unused_opts);
+
+ return 0;
}
static int init_output_stream_nofilter(OutputStream *ost)
@@ -2502,7 +2504,9 @@ int of_open(const OptionsContext *o, const char *filename)
return err;
/* check if all codec options have been used */
- validate_enc_avopt(mux, o->g->codec_opts);
+ err = validate_enc_avopt(mux, o->g->codec_opts);
+ if (err < 0)
+ return err;
/* check filename in case of an image number is expected */
if (oc->oformat->flags & AVFMT_NEEDNUMBER && !av_filename_number_test(oc->url)) {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 16/33] fftools/ffmpeg_mux_init: improve of_add_programs()
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (13 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 15/33] fftools/ffmpeg_mux_init: return error codes from validate_enc_avopt() " Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 23:30 ` Michael Niedermayer
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 17/33] fftools/ffmpeg_mux_init: return error codes from metadata processing instead of aborting Anton Khirnov
` (17 subsequent siblings)
32 siblings, 1 reply; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
Replace duplicated(!) and broken* custom string parsing with
av_dict_parse_string(). Return error codes instead of aborting.
* e.g. it treats NULL returned from av_get_token() as "separator not
found", when in fact av_get_token() only returns NULL on memory
allocation failure
---
fftools/ffmpeg_mux_init.c | 111 ++++++++++++++++++--------------------
1 file changed, 52 insertions(+), 59 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 3a327048bb..b0befb8924 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1782,76 +1782,66 @@ static int setup_sync_queues(Muxer *mux, AVFormatContext *oc, int64_t buf_size_u
return 0;
}
-static void of_add_programs(Muxer *mux, const OptionsContext *o)
+static int of_add_programs(Muxer *mux, const OptionsContext *o)
{
AVFormatContext *oc = mux->fc;
/* process manually set programs */
for (int i = 0; i < o->nb_program; i++) {
- const char *p = o->program[i].u.str;
- int progid = i+1;
+ AVDictionary *dict = NULL;
+ const AVDictionaryEntry *e;
AVProgram *program;
+ int ret, progid = i + 1;
- while(*p) {
- const char *p2 = av_get_token(&p, ":");
- const char *to_dealloc = p2;
- char *key;
- if (!p2)
- break;
+ ret = av_dict_parse_string(&dict, o->program[i].u.str, "=", ":",
+ AV_DICT_MULTIKEY);
+ if (ret < 0) {
+ av_log(mux, AV_LOG_ERROR, "Error parsing program specification %s\n",
+ o->program[i].u.str);
+ return ret;
+ }
- if(*p) p++;
-
- key = av_get_token(&p2, "=");
- if (!key || !*p2) {
- av_freep(&to_dealloc);
- av_freep(&key);
- break;
- }
- p2++;
-
- if (!strcmp(key, "program_num"))
- progid = strtol(p2, NULL, 0);
- av_freep(&to_dealloc);
- av_freep(&key);
+ e = av_dict_get(dict, "program_num", NULL, 0);
+ if (e) {
+ progid = strtol(e->value, NULL, 0);
+ av_dict_set(&dict, e->key, NULL, 0);
}
program = av_new_program(oc, progid);
- if (!program)
- report_and_exit(AVERROR(ENOMEM));
-
- p = o->program[i].u.str;
- while(*p) {
- const char *p2 = av_get_token(&p, ":");
- const char *to_dealloc = p2;
- char *key;
- if (!p2)
- break;
- if(*p) p++;
-
- key = av_get_token(&p2, "=");
- if (!key) {
- av_log(mux, AV_LOG_FATAL,
- "No '=' character in program string %s.\n",
- p2);
- exit_program(1);
- }
- if (!*p2)
- exit_program(1);
- p2++;
-
- if (!strcmp(key, "title")) {
- av_dict_set(&program->metadata, "title", p2, 0);
- } else if (!strcmp(key, "program_num")) {
- } else if (!strcmp(key, "st")) {
- int st_num = strtol(p2, NULL, 0);
- av_program_add_stream_index(oc, progid, st_num);
- } else {
- av_log(mux, AV_LOG_FATAL, "Unknown program key %s.\n", key);
- exit_program(1);
- }
- av_freep(&to_dealloc);
- av_freep(&key);
+ if (!program) {
+ ret = AVERROR(ENOMEM);
+ goto fail;
}
+
+ e = av_dict_get(dict, "title", NULL, 0);
+ if (e) {
+ av_dict_set(&program->metadata, e->key, e->value, 0);
+ av_dict_set(&dict, e->key, NULL, 0);
+ }
+
+ e = NULL;
+ while (e = av_dict_get(dict, "st", e, 0)) {
+ int st_num = strtol(e->value, NULL, 0);
+ av_program_add_stream_index(oc, progid, st_num);
+ }
+
+ // make sure that nothing but "st" entries are left in the dict
+ e = NULL;
+ while (e = av_dict_iterate(dict, e)) {
+ if (!strcmp(e->key, "st"))
+ continue;
+
+ av_log(mux, AV_LOG_FATAL, "Unknown program key %s.\n", e->key);
+ ret = AVERROR(EINVAL);
+ goto fail;
+ }
+
+fail:
+ av_dict_free(&dict);
+ if (ret < 0)
+ return ret;
}
+
+ return 0;
}
/**
@@ -2547,7 +2537,10 @@ int of_open(const OptionsContext *o, const char *filename)
if (err < 0)
return err;
- of_add_programs(mux, o);
+ err = of_add_programs(mux, o);
+ if (err < 0)
+ return err;
+
of_add_metadata(of, oc, o);
err = set_dispositions(mux, o);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 16/33] fftools/ffmpeg_mux_init: improve of_add_programs()
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 16/33] fftools/ffmpeg_mux_init: improve of_add_programs() Anton Khirnov
@ 2023-07-13 23:30 ` Michael Niedermayer
2023-07-14 9:07 ` Anton Khirnov
0 siblings, 1 reply; 45+ messages in thread
From: Michael Niedermayer @ 2023-07-13 23:30 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 5332 bytes --]
On Thu, Jul 13, 2023 at 12:55:36PM +0200, Anton Khirnov wrote:
> Replace duplicated(!) and broken* custom string parsing with
> av_dict_parse_string(). Return error codes instead of aborting.
>
> * e.g. it treats NULL returned from av_get_token() as "separator not
> found", when in fact av_get_token() only returns NULL on memory
> allocation failure
> ---
> fftools/ffmpeg_mux_init.c | 111 ++++++++++++++++++--------------------
> 1 file changed, 52 insertions(+), 59 deletions(-)
smells like memory corruption
-i ~/videos/01c56b0dc1.ts -copy_unknown -map 0 -c copy -fflags +bitexact -t 3 -y file-copy-unknown.ts
http://samples.mplayerhq.hu/ts/01c56b0dc1.ts
[mpegts @ 0x2ced6980] Could not find codec parameters for stream 6 (Unknown: none ([5][0][0][0] / 0x0005)): unknown codec
Consider increasing the value for the 'analyzeduration' (0) and 'probesize' (5000000) options
Input #0, mpegts, from 'videos/01c56b0dc1.ts':
Duration: 00:00:10.73, start: 40848.136244, bitrate: 8431 kb/s
Program 1201
Stream #0:0[0xfb]: Video: h264 (Main) ([27][0][0][0] / 0x001B), yuv420p(tv, bt709, top first), 1920x1080 [SAR 1:1 DAR 16:9], 25 fps, 50 tbr, 90k tbn
Stream #0:1[0x12d](eng): Audio: aac_latm (HE-AAC) ([17][0][0][0] / 0x0011), 48000 Hz, stereo, fltp
Stream #0:2[0x132](eng): Audio: aac_latm (HE-AAC) ([17][0][0][0] / 0x0011), 48000 Hz, stereo, fltp (visual impaired) (descriptions)
Stream #0:3[0x192](eng): Audio: ac3 ([6][0][0][0] / 0x0006), 48000 Hz, 5.1(side), fltp, 384 kb/s
Stream #0:4[0x3ea]: Unknown: none ([11][0][0][0] / 0x000B)
Stream #0:5[0x401](eng): Subtitle: dvb_subtitle ([6][0][0][0] / 0x0006) (hearing impaired)
Stream #0:6[0x1f40]: Unknown: none ([5][0][0][0] / 0x0005)
==2131== Conditional jump or move depends on uninitialised value(s)
==2131== at 0x3162AF: of_open (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== by 0x31863B: open_files.isra.2 (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== by 0x319BAA: ffmpeg_parse_options (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== by 0x2F7A00: main (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== Uninitialised value was created by a stack allocation
==2131== at 0x3139DD: of_open (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131==
==2131== Invalid free() / delete / delete[] / realloc()
==2131== at 0x4C32D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2131== by 0x315AEF: of_open (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== by 0x31863B: open_files.isra.2 (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== by 0x319BAA: ffmpeg_parse_options (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== by 0x2F7A00: main (in ffmpeg-git/ffmpeg/ffmpeg_g)
==2131== Address 0x22f462840 is not stack'd, malloc'd or (recently) free'd
==2131==
Output #0, mpegts, to 'file-copy-unknown.ts':
Stream #0:0: Video: h264 (Main) ([27][0][0][0] / 0x001B), yuv420p(tv, bt709, top first), 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 25 fps, 50 tbr, 90k tbn
Stream #0:1(eng): Audio: aac_latm (HE-AAC) ([17][0][0][0] / 0x0011), 48000 Hz, stereo, fltp (default)
Stream #0:2(eng): Audio: aac_latm (HE-AAC) ([17][0][0][0] / 0x0011), 48000 Hz, stereo, fltp (visual impaired) (descriptions)
Stream #0:3(eng): Audio: ac3 ([6][0][0][0] / 0x0006), 48000 Hz, 5.1(side), fltp, 384 kb/s
Stream #0:4: Unknown: none ([11][0][0][0] / 0x000B)
Stream #0:5(eng): Subtitle: dvb_subtitle ([6][0][0][0] / 0x0006) (hearing impaired)
Stream #0:6: Unknown: none ([5][0][0][0] / 0x0005)
Stream mapping:
Stream #0:0 -> #0:0 (copy)
Stream #0:1 -> #0:1 (copy)
Stream #0:2 -> #0:2 (copy)
Stream #0:3 -> #0:3 (copy)
Stream #0:4 -> #0:4 (copy)
Stream #0:5 -> #0:5 (copy)
Stream #0:6 -> #0:6 (copy)
Press [q] to stop, [?] for help
[mpegts @ 0x2ced6980] PES packet size mismatche=00:00:02.98 bitrate= 0.0kbits/s speed=5.97x
[mpegts @ 0x2ced6980] Packet corrupt (stream = 1, dts = 3677260813).
[in#0/mpegts @ 0x2ced67c0] corrupt input packet in stream 1
[mpegts @ 0x2ced6980] PES packet size mismatch
[mpegts @ 0x2ced6980] Packet corrupt (stream = 2, dts = 3677250303).
[in#0/mpegts @ 0x2ced67c0] corrupt input packet in stream 2
Last message repeated 3 times
[mpegts @ 0x2ced6980] PES packet size mismatch
[mpegts @ 0x2ced6980] Packet corrupt (stream = 3, dts = 3677239462).
[in#0/mpegts @ 0x2ced67c0] corrupt input packet in stream 3
Last message repeated 1 times
[mpegts @ 0x2ea0edc0] Stream 4, codec none, is muxed as a private data stream and may not be recognized upon reading.
[mpegts @ 0x2ea0edc0] Stream 6, codec none, is muxed as a private data stream and may not be recognized upon reading.
[out#0/mpegts @ 0x2d09f040] video:484kB audio:206kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 6.355365%
frame= 42 fps=0.0 q=-1.0 Lsize= 734kB time=00:00:02.98 bitrate=2011.2kbits/s speed=3.31x
==2131==
==2131== HEAP SUMMARY:
==2131== in use at exit: 49,276 bytes in 240 blocks
==2131== total heap usage: 23,803 allocs, 23,564 frees, 66,233,951 bytes allocated
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 16/33] fftools/ffmpeg_mux_init: improve of_add_programs()
2023-07-13 23:30 ` Michael Niedermayer
@ 2023-07-14 9:07 ` Anton Khirnov
2023-07-14 18:12 ` Michael Niedermayer
0 siblings, 1 reply; 45+ messages in thread
From: Anton Khirnov @ 2023-07-14 9:07 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Michael Niedermayer (2023-07-14 01:30:24)
> On Thu, Jul 13, 2023 at 12:55:36PM +0200, Anton Khirnov wrote:
> > Replace duplicated(!) and broken* custom string parsing with
> > av_dict_parse_string(). Return error codes instead of aborting.
> >
> > * e.g. it treats NULL returned from av_get_token() as "separator not
> > found", when in fact av_get_token() only returns NULL on memory
> > allocation failure
> > ---
> > fftools/ffmpeg_mux_init.c | 111 ++++++++++++++++++--------------------
> > 1 file changed, 52 insertions(+), 59 deletions(-)
>
> smells like memory corruption
>
> -i ~/videos/01c56b0dc1.ts -copy_unknown -map 0 -c copy -fflags +bitexact -t 3 -y file-copy-unknown.ts
I have no idea why are you replying to this specific patch, when this
issue apparently exists in current master.
I'll send a fix for it in a later patchset, but it's completely
unrelated to this set.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [FFmpeg-devel] [PATCH 16/33] fftools/ffmpeg_mux_init: improve of_add_programs()
2023-07-14 9:07 ` Anton Khirnov
@ 2023-07-14 18:12 ` Michael Niedermayer
0 siblings, 0 replies; 45+ messages in thread
From: Michael Niedermayer @ 2023-07-14 18:12 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1352 bytes --]
On Fri, Jul 14, 2023 at 11:07:19AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-14 01:30:24)
> > On Thu, Jul 13, 2023 at 12:55:36PM +0200, Anton Khirnov wrote:
> > > Replace duplicated(!) and broken* custom string parsing with
> > > av_dict_parse_string(). Return error codes instead of aborting.
> > >
> > > * e.g. it treats NULL returned from av_get_token() as "separator not
> > > found", when in fact av_get_token() only returns NULL on memory
> > > allocation failure
> > > ---
> > > fftools/ffmpeg_mux_init.c | 111 ++++++++++++++++++--------------------
> > > 1 file changed, 52 insertions(+), 59 deletions(-)
> >
> > smells like memory corruption
> >
> > -i ~/videos/01c56b0dc1.ts -copy_unknown -map 0 -c copy -fflags +bitexact -t 3 -y file-copy-unknown.ts
>
> I have no idea why are you replying to this specific patch, when this
> issue apparently exists in current master.
git bisect of a segfault lead me to this one
with memory corruption, its quite possible it doesnt always segfault so
git bisect results can be bogus
sorry
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread
* [FFmpeg-devel] [PATCH 17/33] fftools/ffmpeg_mux_init: return error codes from metadata processing instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (14 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 16/33] fftools/ffmpeg_mux_init: improve of_add_programs() Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 18/33] fftools/ffmpeg_mux_init: replace all remaining aborts with returning error codes Anton Khirnov
` (16 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg_mux_init.c | 63 ++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index b0befb8924..2bd152039d 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1851,8 +1851,8 @@ fail:
* @param index for type c/p, chapter/program index is written here
* @param stream_spec for type s, the stream specifier is written here
*/
-static void parse_meta_type(void *logctx, const char *arg,
- char *type, int *index, const char **stream_spec)
+static int parse_meta_type(void *logctx, const char *arg,
+ char *type, int *index, const char **stream_spec)
{
if (*arg) {
*type = *arg;
@@ -1862,7 +1862,7 @@ static void parse_meta_type(void *logctx, const char *arg,
case 's':
if (*(++arg) && *arg != ':') {
av_log(logctx, AV_LOG_FATAL, "Invalid metadata specifier %s.\n", arg);
- exit_program(1);
+ return AVERROR(EINVAL);
}
*stream_spec = *arg == ':' ? arg + 1 : "";
break;
@@ -1873,14 +1873,16 @@ static void parse_meta_type(void *logctx, const char *arg,
break;
default:
av_log(logctx, AV_LOG_FATAL, "Invalid metadata type %c.\n", *arg);
- exit_program(1);
+ return AVERROR(EINVAL);
}
} else
*type = 'g';
+
+ return 0;
}
-static void of_add_metadata(OutputFile *of, AVFormatContext *oc,
- const OptionsContext *o)
+static int of_add_metadata(OutputFile *of, AVFormatContext *oc,
+ const OptionsContext *o)
{
for (int i = 0; i < o->nb_metadata; i++) {
AVDictionary **m;
@@ -1892,11 +1894,14 @@ static void of_add_metadata(OutputFile *of, AVFormatContext *oc,
if (!val) {
av_log(of, AV_LOG_FATAL, "No '=' character in metadata string %s.\n",
o->metadata[i].u.str);
- exit_program(1);
+ return AVERROR(EINVAL);
}
*val++ = 0;
- parse_meta_type(of, o->metadata[i].specifier, &type, &index, &stream_spec);
+ ret = parse_meta_type(of, o->metadata[i].specifier, &type, &index, &stream_spec);
+ if (ret < 0)
+ return ret;
+
if (type == 's') {
for (int j = 0; j < oc->nb_streams; j++) {
OutputStream *ost = of->streams[j];
@@ -1922,7 +1927,7 @@ static void of_add_metadata(OutputFile *of, AVFormatContext *oc,
}
#endif
} else if (ret < 0)
- exit_program(1);
+ return ret;
}
} else {
switch (type) {
@@ -1932,24 +1937,26 @@ static void of_add_metadata(OutputFile *of, AVFormatContext *oc,
case 'c':
if (index < 0 || index >= oc->nb_chapters) {
av_log(of, AV_LOG_FATAL, "Invalid chapter index %d in metadata specifier.\n", index);
- exit_program(1);
+ return AVERROR(EINVAL);
}
m = &oc->chapters[index]->metadata;
break;
case 'p':
if (index < 0 || index >= oc->nb_programs) {
av_log(of, AV_LOG_FATAL, "Invalid program index %d in metadata specifier.\n", index);
- exit_program(1);
+ return AVERROR(EINVAL);
}
m = &oc->programs[index]->metadata;
break;
default:
av_log(of, AV_LOG_FATAL, "Invalid metadata specifier %s.\n", o->metadata[i].specifier);
- exit_program(1);
+ return AVERROR(EINVAL);
}
av_dict_set(m, o->metadata[i].u.str, *val ? val : NULL, 0);
}
}
+
+ return 0;
}
static int copy_chapters(InputFile *ifile, OutputFile *ofile, AVFormatContext *os,
@@ -2008,8 +2015,11 @@ static int copy_metadata(Muxer *mux, AVFormatContext *ic,
const char *istream_spec = NULL, *ostream_spec = NULL;
int idx_in = 0, idx_out = 0;
- parse_meta_type(mux, inspec, &type_in, &idx_in, &istream_spec);
- parse_meta_type(mux, outspec, &type_out, &idx_out, &ostream_spec);
+ ret = parse_meta_type(mux, inspec, &type_in, &idx_in, &istream_spec);
+ if (ret >= 0)
+ ret = parse_meta_type(mux, outspec, &type_out, &idx_out, &ostream_spec);
+ if (ret < 0)
+ return ret;
if (type_in == 'g' || type_out == 'g' || !*outspec)
*metadata_global_manual = 1;
@@ -2026,7 +2036,7 @@ static int copy_metadata(Muxer *mux, AVFormatContext *ic,
if ((index) < 0 || (index) >= (nb_elems)) {\
av_log(mux, AV_LOG_FATAL, "Invalid %s index %d while processing metadata maps.\n",\
(desc), (index));\
- exit_program(1);\
+ return AVERROR(EINVAL);\
}
#define SET_DICT(type, meta, context, index)\
@@ -2057,11 +2067,11 @@ static int copy_metadata(Muxer *mux, AVFormatContext *ic,
meta_in = &ic->streams[i]->metadata;
break;
} else if (ret < 0)
- exit_program(1);
+ return ret;
}
if (!meta_in) {
av_log(mux, AV_LOG_FATAL, "Stream specifier %s does not match any streams.\n", istream_spec);
- exit_program(1);
+ return AVERROR(EINVAL);
}
}
@@ -2071,7 +2081,7 @@ static int copy_metadata(Muxer *mux, AVFormatContext *ic,
meta_out = &oc->streams[i]->metadata;
av_dict_copy(meta_out, *meta_in, AV_DICT_DONT_OVERWRITE);
} else if (ret < 0)
- exit_program(1);
+ return ret;
}
} else
av_dict_copy(meta_out, *meta_in, AV_DICT_DONT_OVERWRITE);
@@ -2087,6 +2097,7 @@ static int copy_meta(Muxer *mux, const OptionsContext *o)
int metadata_global_manual = 0;
int metadata_streams_manual = 0;
int metadata_chapters_manual = 0;
+ int ret;
/* copy metadata */
for (int i = 0; i < o->nb_metadata_map; i++) {
@@ -2098,11 +2109,13 @@ static int copy_meta(Muxer *mux, const OptionsContext *o)
"processing metadata maps\n", in_file_index);
return AVERROR(EINVAL);
}
- copy_metadata(mux,
- in_file_index >= 0 ? input_files[in_file_index]->ctx : NULL,
- o->metadata_map[i].specifier, *p ? p + 1 : p,
- &metadata_global_manual, &metadata_streams_manual,
- &metadata_chapters_manual);
+ ret = copy_metadata(mux,
+ in_file_index >= 0 ? input_files[in_file_index]->ctx : NULL,
+ o->metadata_map[i].specifier, *p ? p + 1 : p,
+ &metadata_global_manual, &metadata_streams_manual,
+ &metadata_chapters_manual);
+ if (ret < 0)
+ return ret;
}
/* copy chapters */
@@ -2541,7 +2554,9 @@ int of_open(const OptionsContext *o, const char *filename)
if (err < 0)
return err;
- of_add_metadata(of, oc, o);
+ err = of_add_metadata(of, oc, o);
+ if (err < 0)
+ return err;
err = set_dispositions(mux, o);
if (err < 0) {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 18/33] fftools/ffmpeg_mux_init: replace all remaining aborts with returning error codes
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (15 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 17/33] fftools/ffmpeg_mux_init: return error codes from metadata processing instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 19/33] fftools/ffmpeg: return an error instead of aborting Anton Khirnov
` (15 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
Mainly concerns new_stream_*() and their callees.
---
fftools/ffmpeg_mux_init.c | 126 ++++++++++++++++++++++++--------------
1 file changed, 79 insertions(+), 47 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 2bd152039d..4b7961b833 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -154,7 +154,8 @@ static char *get_line(AVIOContext *s, AVBPrint *bprint)
av_bprint_chars(bprint, c, 1);
if (!av_bprint_is_complete(bprint))
- report_and_exit(AVERROR(ENOMEM));
+ return NULL;
+
return bprint->str;
}
@@ -462,7 +463,7 @@ static int ost_get_filters(const OptionsContext *o, AVFormatContext *oc,
if (filters_script && filters) {
av_log(ost, AV_LOG_ERROR, "Both -filter and -filter_script set\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
if (filters_script)
@@ -474,7 +475,7 @@ static int ost_get_filters(const OptionsContext *o, AVFormatContext *oc,
return *dst ? 0 : AVERROR(ENOMEM);
}
-static void parse_matrix_coeffs(void *logctx, uint16_t *dest, const char *str)
+static int parse_matrix_coeffs(void *logctx, uint16_t *dest, const char *str)
{
int i;
const char *p = str;
@@ -486,36 +487,39 @@ static void parse_matrix_coeffs(void *logctx, uint16_t *dest, const char *str)
if (!p) {
av_log(logctx, AV_LOG_FATAL,
"Syntax error in matrix \"%s\" at coeff %d\n", str, i);
- exit_program(1);
+ return AVERROR(EINVAL);
}
p++;
}
+
+ return 0;
}
-static void new_stream_video(Muxer *mux, const OptionsContext *o,
- OutputStream *ost)
+static int new_stream_video(Muxer *mux, const OptionsContext *o,
+ OutputStream *ost)
{
AVFormatContext *oc = mux->fc;
AVStream *st;
char *frame_rate = NULL, *max_frame_rate = NULL, *frame_aspect_ratio = NULL;
+ int ret = 0;
st = ost->st;
MATCH_PER_STREAM_OPT(frame_rates, str, frame_rate, oc, st);
if (frame_rate && av_parse_video_rate(&ost->frame_rate, frame_rate) < 0) {
av_log(ost, AV_LOG_FATAL, "Invalid framerate value: %s\n", frame_rate);
- exit_program(1);
+ return AVERROR(EINVAL);
}
MATCH_PER_STREAM_OPT(max_frame_rates, str, max_frame_rate, oc, st);
if (max_frame_rate && av_parse_video_rate(&ost->max_frame_rate, max_frame_rate) < 0) {
av_log(ost, AV_LOG_FATAL, "Invalid maximum framerate value: %s\n", max_frame_rate);
- exit_program(1);
+ return AVERROR(EINVAL);
}
if (frame_rate && max_frame_rate) {
av_log(ost, AV_LOG_ERROR, "Only one of -fpsmax and -r can be set for a stream.\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
MATCH_PER_STREAM_OPT(frame_aspect_ratios, str, frame_aspect_ratio, oc, st);
@@ -524,7 +528,7 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
if (av_parse_ratio(&q, frame_aspect_ratio, 255, 0, NULL) < 0 ||
q.num <= 0 || q.den <= 0) {
av_log(ost, AV_LOG_FATAL, "Invalid aspect ratio: %s\n", frame_aspect_ratio);
- exit_program(1);
+ return AVERROR(EINVAL);
}
ost->frame_aspect_ratio = q;
}
@@ -540,9 +544,12 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
int i;
MATCH_PER_STREAM_OPT(frame_sizes, str, frame_size, oc, st);
- if (frame_size && av_parse_video_size(&video_enc->width, &video_enc->height, frame_size) < 0) {
- av_log(ost, AV_LOG_FATAL, "Invalid frame size: %s.\n", frame_size);
- exit_program(1);
+ if (frame_size) {
+ ret = av_parse_video_size(&video_enc->width, &video_enc->height, frame_size);
+ if (ret < 0) {
+ av_log(ost, AV_LOG_FATAL, "Invalid frame size: %s.\n", frame_size);
+ return AVERROR(EINVAL);
+ }
}
MATCH_PER_STREAM_OPT(frame_pix_fmts, str, frame_pix_fmt, oc, st);
@@ -553,29 +560,36 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
}
if (frame_pix_fmt && (video_enc->pix_fmt = av_get_pix_fmt(frame_pix_fmt)) == AV_PIX_FMT_NONE) {
av_log(ost, AV_LOG_FATAL, "Unknown pixel format requested: %s.\n", frame_pix_fmt);
- exit_program(1);
+ return AVERROR(EINVAL);
}
st->sample_aspect_ratio = video_enc->sample_aspect_ratio;
MATCH_PER_STREAM_OPT(intra_matrices, str, intra_matrix, oc, st);
if (intra_matrix) {
if (!(video_enc->intra_matrix = av_mallocz(sizeof(*video_enc->intra_matrix) * 64)))
- report_and_exit(AVERROR(ENOMEM));
- parse_matrix_coeffs(ost, video_enc->intra_matrix, intra_matrix);
+ return AVERROR(ENOMEM);
+
+ ret = parse_matrix_coeffs(ost, video_enc->intra_matrix, intra_matrix);
+ if (ret < 0)
+ return ret;
}
MATCH_PER_STREAM_OPT(chroma_intra_matrices, str, chroma_intra_matrix, oc, st);
if (chroma_intra_matrix) {
uint16_t *p = av_mallocz(sizeof(*video_enc->chroma_intra_matrix) * 64);
if (!p)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
video_enc->chroma_intra_matrix = p;
- parse_matrix_coeffs(ost, p, chroma_intra_matrix);
+ ret = parse_matrix_coeffs(ost, p, chroma_intra_matrix);
+ if (ret < 0)
+ return ret;
}
MATCH_PER_STREAM_OPT(inter_matrices, str, inter_matrix, oc, st);
if (inter_matrix) {
if (!(video_enc->inter_matrix = av_mallocz(sizeof(*video_enc->inter_matrix) * 64)))
- report_and_exit(AVERROR(ENOMEM));
- parse_matrix_coeffs(ost, video_enc->inter_matrix, inter_matrix);
+ return AVERROR(ENOMEM);
+ ret = parse_matrix_coeffs(ost, video_enc->inter_matrix, inter_matrix);
+ if (ret < 0)
+ return ret;
}
MATCH_PER_STREAM_OPT(rc_overrides, str, p, oc, st);
@@ -584,14 +598,14 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
int e = sscanf(p, "%d,%d,%d", &start, &end, &q);
if (e != 3) {
av_log(ost, AV_LOG_FATAL, "error parsing rc_override\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
video_enc->rc_override =
av_realloc_array(video_enc->rc_override,
i + 1, sizeof(RcOverride));
if (!video_enc->rc_override) {
av_log(ost, AV_LOG_FATAL, "Could not (re)allocate memory for rc_override.\n");
- exit_program(1);
+ return AVERROR(ENOMEM);
}
video_enc->rc_override[i].start_frame = start;
video_enc->rc_override[i].end_frame = end;
@@ -631,7 +645,7 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
MATCH_PER_STREAM_OPT(passlogfiles, str, ost->logfile_prefix, oc, st);
if (ost->logfile_prefix &&
!(ost->logfile_prefix = av_strdup(ost->logfile_prefix)))
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
if (do_pass) {
int ost_idx = -1;
@@ -655,7 +669,7 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
if (!logbuffer) {
av_log(ost, AV_LOG_FATAL, "Error reading log file '%s' for pass-2 encoding\n",
logfilename);
- exit_program(1);
+ return AVERROR(EIO);
}
video_enc->stats_in = logbuffer;
}
@@ -665,7 +679,7 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
av_log(ost, AV_LOG_FATAL,
"Cannot write log file '%s' for pass-1 encoding: %s\n",
logfilename, strerror(errno));
- exit_program(1);
+ return AVERROR(errno);
}
ost->logfile = f;
}
@@ -687,7 +701,7 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
ost->vsync_method == VSYNC_CFR || ost->vsync_method == VSYNC_VSCFR)) {
av_log(ost, AV_LOG_FATAL, "One of -r/-fpsmax was specified "
"together a non-CFR -vsync/-fps_mode. This is contradictory.\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
if (ost->vsync_method == VSYNC_AUTO) {
@@ -715,13 +729,16 @@ static void new_stream_video(Muxer *mux, const OptionsContext *o,
}
ost->is_cfr = (ost->vsync_method == VSYNC_CFR || ost->vsync_method == VSYNC_VSCFR);
}
+
+ return 0;
}
-static void new_stream_audio(Muxer *mux, const OptionsContext *o,
- OutputStream *ost)
+static int new_stream_audio(Muxer *mux, const OptionsContext *o,
+ OutputStream *ost)
{
AVFormatContext *oc = mux->fc;
AVStream *st;
+ int ret = 0;
st = ost->st;
@@ -748,7 +765,7 @@ static void new_stream_audio(Muxer *mux, const OptionsContext *o,
if (!mask) {
#endif
av_log(ost, AV_LOG_FATAL, "Unknown channel layout: %s\n", layout);
- exit_program(1);
+ return AVERROR(EINVAL);
#if FF_API_OLD_CHANNEL_LAYOUT
}
av_log(ost, AV_LOG_WARNING, "Channel layout '%s' uses a deprecated syntax.\n",
@@ -762,7 +779,7 @@ static void new_stream_audio(Muxer *mux, const OptionsContext *o,
if (sample_fmt &&
(audio_enc->sample_fmt = av_get_sample_fmt(sample_fmt)) == AV_SAMPLE_FMT_NONE) {
av_log(ost, AV_LOG_FATAL, "Invalid sample format '%s'\n", sample_fmt);
- exit_program(1);
+ return AVERROR(EINVAL);
}
MATCH_PER_STREAM_OPT(audio_sample_rate, i, audio_enc->sample_rate, oc, st);
@@ -789,11 +806,11 @@ static void new_stream_audio(Muxer *mux, const OptionsContext *o,
}
if (!ist || (ist->file_index == map->file_idx && ist->index == map->stream_idx)) {
- if (av_reallocp_array(&ost->audio_channels_map,
- ost->audio_channels_mapped + 1,
- sizeof(*ost->audio_channels_map)
- ) < 0 )
- report_and_exit(AVERROR(ENOMEM));
+ ret = av_reallocp_array(&ost->audio_channels_map,
+ ost->audio_channels_mapped + 1,
+ sizeof(*ost->audio_channels_map));
+ if (ret < 0)
+ return ret;
ost->audio_channels_map[ost->audio_channels_mapped++] = map->channel_idx;
}
@@ -801,16 +818,19 @@ static void new_stream_audio(Muxer *mux, const OptionsContext *o,
}
#endif
}
+
+ return 0;
}
-static void new_stream_attachment(Muxer *mux, const OptionsContext *o,
- OutputStream *ost)
+static int new_stream_attachment(Muxer *mux, const OptionsContext *o,
+ OutputStream *ost)
{
ost->finished = 1;
+ return 0;
}
-static void new_stream_subtitle(Muxer *mux, const OptionsContext *o,
- OutputStream *ost)
+static int new_stream_subtitle(Muxer *mux, const OptionsContext *o,
+ OutputStream *ost)
{
AVStream *st;
@@ -828,9 +848,12 @@ static void new_stream_subtitle(Muxer *mux, const OptionsContext *o,
char *frame_size = NULL;
MATCH_PER_STREAM_OPT(frame_sizes, str, frame_size, mux->fc, st);
- if (frame_size && av_parse_video_size(&subtitle_enc->width, &subtitle_enc->height, frame_size) < 0) {
- av_log(ost, AV_LOG_FATAL, "Invalid frame size: %s.\n", frame_size);
- exit_program(1);
+ if (frame_size) {
+ int ret = av_parse_video_size(&subtitle_enc->width, &subtitle_enc->height, frame_size);
+ if (ret < 0) {
+ av_log(ost, AV_LOG_FATAL, "Invalid frame size: %s.\n", frame_size);
+ return ret;
+ }
}
if (input_descriptor)
input_props = input_descriptor->props & (AV_CODEC_PROP_TEXT_SUB | AV_CODEC_PROP_BITMAP_SUB);
@@ -840,9 +863,11 @@ static void new_stream_subtitle(Muxer *mux, const OptionsContext *o,
av_log(ost, AV_LOG_ERROR,
"Subtitle encoding currently only possible from text to text "
"or bitmap to bitmap\n");
- exit_program(1);
+ return AVERROR(EINVAL);
}
}
+
+ return 0;
}
static int streamcopy_init(const Muxer *mux, OutputStream *ost)
@@ -1084,6 +1109,11 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
do {
av_bprint_clear(&bprint);
buf = get_line(s, &bprint);
+ if (!buf) {
+ ret = AVERROR(ENOMEM);
+ break;
+ }
+
if (!buf[0] || buf[0] == '#')
continue;
if (!(arg = strchr(buf, '='))) {
@@ -1250,11 +1280,13 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
ms->copy_initial_nonkeyframes, oc, st);
switch (type) {
- case AVMEDIA_TYPE_VIDEO: new_stream_video (mux, o, ost); break;
- case AVMEDIA_TYPE_AUDIO: new_stream_audio (mux, o, ost); break;
- case AVMEDIA_TYPE_SUBTITLE: new_stream_subtitle (mux, o, ost); break;
- case AVMEDIA_TYPE_ATTACHMENT: new_stream_attachment(mux, o, ost); break;
+ case AVMEDIA_TYPE_VIDEO: ret = new_stream_video (mux, o, ost); break;
+ case AVMEDIA_TYPE_AUDIO: ret = new_stream_audio (mux, o, ost); break;
+ case AVMEDIA_TYPE_SUBTITLE: ret = new_stream_subtitle (mux, o, ost); break;
+ case AVMEDIA_TYPE_ATTACHMENT: ret = new_stream_attachment(mux, o, ost); break;
}
+ if (ret < 0)
+ return ret;
if (type == AVMEDIA_TYPE_VIDEO || type == AVMEDIA_TYPE_AUDIO) {
ret = ost_get_filters(o, oc, ost, &filters);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 19/33] fftools/ffmpeg: return an error instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (16 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 18/33] fftools/ffmpeg_mux_init: replace all remaining aborts with returning error codes Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 20/33] fftools/ffmpeg: handle error codes from process_input_packet() Anton Khirnov
` (14 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 96638242f3..68924d21e0 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1094,7 +1094,7 @@ static int process_input(int file_index)
av_log(ifile, AV_LOG_ERROR,
"Error retrieving a packet from demuxer: %s\n", av_err2str(ret));
if (exit_on_error)
- exit_program(1);
+ return ret;
}
for (i = 0; i < ifile->nb_streams; i++) {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 20/33] fftools/ffmpeg: handle error codes from process_input_packet()
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (17 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 19/33] fftools/ffmpeg: return an error instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 21/33] fftools/ffmpeg_mux: return errors from of_streamcopy() instead of aborting Anton Khirnov
` (13 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
None are returned for now, but that will change in future commits.
---
fftools/ffmpeg.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 68924d21e0..dd7cfcf632 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1103,6 +1103,8 @@ static int process_input(int file_index)
ret = process_input_packet(ist, NULL, 0);
if (ret>0)
return 0;
+ else if (ret < 0)
+ return ret;
}
/* mark all outputs that don't go through lavfi as finished */
@@ -1124,11 +1126,11 @@ static int process_input(int file_index)
sub2video_heartbeat(ifile, pkt->pts, pkt->time_base);
- process_input_packet(ist, pkt, 0);
+ ret = process_input_packet(ist, pkt, 0);
av_packet_free(&pkt);
- return 0;
+ return ret < 0 ? ret : 0;
}
/**
@@ -1219,7 +1221,8 @@ static int transcode(int *err_rate_exceeded)
float err_rate;
if (!input_files[ist->file_index]->eof_reached) {
- process_input_packet(ist, NULL, 0);
+ int err = process_input_packet(ist, NULL, 0);
+ ret = err_merge(ret, err);
}
err_rate = (ist->frames_decoded || ist->decode_errors) ?
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 21/33] fftools/ffmpeg_mux: return errors from of_streamcopy() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (18 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 20/33] fftools/ffmpeg: handle error codes from process_input_packet() Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 22/33] fftools/ffmpeg_enc: return errors from enc_subtitle() " Anton Khirnov
` (12 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg.h | 2 +-
fftools/ffmpeg_mux.c | 20 ++++++++++++--------
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 30020cd0f8..fdee20d6b2 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -826,7 +826,7 @@ void of_output_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt);
/**
* @param dts predicted packet dts in AV_TIME_BASE_Q
*/
-void of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts);
+int of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts);
int64_t of_filesize(OutputFile *of);
diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
index 026796f7e6..168e4ec799 100644
--- a/fftools/ffmpeg_mux.c
+++ b/fftools/ffmpeg_mux.c
@@ -383,13 +383,14 @@ fail:
}
-void of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
+int of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
{
OutputFile *of = output_files[ost->file_index];
MuxStream *ms = ms_from_ost(ost);
int64_t start_time = (of->start_time == AV_NOPTS_VALUE) ? 0 : of->start_time;
int64_t ost_tb_start_time = av_rescale_q(start_time, AV_TIME_BASE_Q, ost->mux_timebase);
AVPacket *opkt = ms->pkt;
+ int ret;
av_packet_unref(opkt);
@@ -400,26 +401,27 @@ void of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
// EOF: flush output bitstream filters.
if (!pkt) {
of_output_packet(of, ost, NULL);
- return;
+ return 0;
}
if (!ms->streamcopy_started && !(pkt->flags & AV_PKT_FLAG_KEY) &&
!ms->copy_initial_nonkeyframes)
- return;
+ return 0;
if (!ms->streamcopy_started) {
if (!ms->copy_prior_start &&
(pkt->pts == AV_NOPTS_VALUE ?
dts < ms->ts_copy_start :
pkt->pts < av_rescale_q(ms->ts_copy_start, AV_TIME_BASE_Q, pkt->time_base)))
- return;
+ return 0;
if (of->start_time != AV_NOPTS_VALUE && dts < of->start_time)
- return;
+ return 0;
}
- if (av_packet_ref(opkt, pkt) < 0)
- exit_program(1);
+ ret = av_packet_ref(opkt, pkt);
+ if (ret < 0)
+ return ret;
opkt->time_base = ost->mux_timebase;
@@ -449,13 +451,15 @@ void of_streamcopy(OutputStream *ost, const AVPacket *pkt, int64_t dts)
av_log(NULL, AV_LOG_ERROR,
"Subtitle heartbeat logic failed in %s! (%s)\n",
__func__, av_err2str(ret));
- exit_program(1);
+ return ret;
}
}
of_output_packet(of, ost, opkt);
ms->streamcopy_started = 1;
+
+ return 0;
}
static int thread_stop(Muxer *mux)
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 22/33] fftools/ffmpeg_enc: return errors from enc_subtitle() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (19 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 21/33] fftools/ffmpeg_mux: return errors from of_streamcopy() instead of aborting Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 23/33] fftools/ffmpeg_mux_init: drop an obsolete assignment Anton Khirnov
` (11 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
---
fftools/ffmpeg.h | 2 +-
fftools/ffmpeg_dec.c | 4 +++-
fftools/ffmpeg_enc.c | 16 ++++++++--------
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index fdee20d6b2..bcd9cc584e 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -804,7 +804,7 @@ int enc_alloc(Encoder **penc, const AVCodec *codec);
void enc_free(Encoder **penc);
int enc_open(OutputStream *ost, AVFrame *frame);
-void enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub);
+int enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub);
void enc_frame(OutputStream *ost, AVFrame *frame);
void enc_flush(void);
diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index 85bf8dc536..a1d811f512 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -439,7 +439,9 @@ static int process_subtitle(InputStream *ist, AVFrame *frame)
if (!ost->enc || ost->type != AVMEDIA_TYPE_SUBTITLE)
continue;
- enc_subtitle(output_files[ost->file_index], ost, subtitle);
+ ret = enc_subtitle(output_files[ost->file_index], ost, subtitle);
+ if (ret < 0)
+ return ret;
}
return 0;
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 4029501313..b35dbb6290 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -462,7 +462,7 @@ static int check_recording_time(OutputStream *ost, int64_t ts, AVRational tb)
return 1;
}
-void enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub)
+int enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub)
{
Encoder *e = ost->enc;
int subtitle_out_max_size = 1024 * 1024;
@@ -473,13 +473,11 @@ void enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub)
if (sub->pts == AV_NOPTS_VALUE) {
av_log(ost, AV_LOG_ERROR, "Subtitle packets must have a pts\n");
- if (exit_on_error)
- exit_program(1);
- return;
+ return exit_on_error ? AVERROR(EINVAL) : 0;
}
if (ost->finished ||
(of->start_time != AV_NOPTS_VALUE && sub->pts < of->start_time))
- return;
+ return 0;
enc = ost->enc_ctx;
@@ -501,11 +499,11 @@ void enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub)
AVSubtitle local_sub = *sub;
if (!check_recording_time(ost, pts, AV_TIME_BASE_Q))
- return;
+ return 0;
ret = av_new_packet(pkt, subtitle_out_max_size);
if (ret < 0)
- report_and_exit(AVERROR(ENOMEM));
+ return AVERROR(ENOMEM);
local_sub.pts = pts;
// start_display_time is required to be 0
@@ -525,7 +523,7 @@ void enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub)
subtitle_out_size = avcodec_encode_subtitle(enc, pkt->data, pkt->size, &local_sub);
if (subtitle_out_size < 0) {
av_log(ost, AV_LOG_FATAL, "Subtitle encoding failed\n");
- exit_program(1);
+ return subtitle_out_size;
}
av_shrink_packet(pkt, subtitle_out_size);
@@ -544,6 +542,8 @@ void enc_subtitle(OutputFile *of, OutputStream *ost, const AVSubtitle *sub)
of_output_packet(of, ost, pkt);
}
+
+ return 0;
}
void enc_stats_write(OutputStream *ost, EncStats *es,
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 23/33] fftools/ffmpeg_mux_init: drop an obsolete assignment
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (20 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 22/33] fftools/ffmpeg_enc: return errors from enc_subtitle() " Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 24/33] fftools/ffmpeg_mux_init: handle pixel format endianness Anton Khirnov
` (10 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
This line was added in c30a4489b44 along with
AVStream.sample_aspect_ratio. However, configuring SAR for video
encoding is now done after this code (specifically in enc_open(), which
is called when the first video frame to be encoded is obtained), so this
line cannot have any meaningful effect.
---
fftools/ffmpeg_mux_init.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 4b7961b833..2dba76b82b 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -562,7 +562,6 @@ static int new_stream_video(Muxer *mux, const OptionsContext *o,
av_log(ost, AV_LOG_FATAL, "Unknown pixel format requested: %s.\n", frame_pix_fmt);
return AVERROR(EINVAL);
}
- st->sample_aspect_ratio = video_enc->sample_aspect_ratio;
MATCH_PER_STREAM_OPT(intra_matrices, str, intra_matrix, oc, st);
if (intra_matrix) {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 24/33] fftools/ffmpeg_mux_init: handle pixel format endianness
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (21 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 23/33] fftools/ffmpeg_mux_init: drop an obsolete assignment Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format Anton Khirnov
` (9 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
When -pix_fmt designates a BE/LE pixel format, it gets translated into
the native one by av_get_pix_fmt(). This may not always be the best
choice, as the encoder might only support one endianness. In such a
case, explicitly choose the endianness supported by the encoder.
While this is currently redundant with choose_pixel_fmt() in
ffmpeg_filter.c, the latter code will be removed in following commits.
---
fftools/ffmpeg_mux_init.c | 55 ++++++++++++++++++++++++++++++++++++---
1 file changed, 52 insertions(+), 3 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 2dba76b82b..4434e6b70d 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -495,6 +495,54 @@ static int parse_matrix_coeffs(void *logctx, uint16_t *dest, const char *str)
return 0;
}
+static int fmt_in_list(const int *formats, int format)
+{
+ for (; *formats != -1; formats++)
+ if (*formats == format)
+ return 1;
+ return 0;
+}
+
+static enum AVPixelFormat pix_fmt_parse(OutputStream *ost, const char *name)
+{
+ const enum AVPixelFormat *fmts = ost->enc_ctx->codec->pix_fmts;
+ enum AVPixelFormat fmt;
+
+ fmt = av_get_pix_fmt(name);
+ if (fmt == AV_PIX_FMT_NONE) {
+ av_log(ost, AV_LOG_FATAL, "Unknown pixel format requested: %s.\n", name);
+ return AV_PIX_FMT_NONE;
+ }
+
+ /* when the user specified-format is an alias for an endianness-specific
+ * one (e.g. rgb48 -> rgb48be/le), it gets translated into the native
+ * endianness by av_get_pix_fmt();
+ * the following code handles the case when the native endianness is not
+ * supported by the encoder, but the other one is */
+ if (fmts && !fmt_in_list(fmts, fmt)) {
+ const char *name_canonical = av_get_pix_fmt_name(fmt);
+ int len = strlen(name_canonical);
+
+ if (strcmp(name, name_canonical) &&
+ (!strcmp(name_canonical + len - 2, "le") ||
+ !strcmp(name_canonical + len - 2, "be"))) {
+ char name_other[64];
+ enum AVPixelFormat fmt_other;
+
+ snprintf(name_other, sizeof(name_other), "%s%ce",
+ name, name_canonical[len - 2] == 'l' ? 'b' : 'l');
+ fmt_other = av_get_pix_fmt(name_other);
+ if (fmt_other != AV_PIX_FMT_NONE && fmt_in_list(fmts, fmt_other)) {
+ av_log(ost, AV_LOG_VERBOSE, "Mapping pixel format %s->%s\n",
+ name, name_other);
+ fmt = fmt_other;
+ }
+ }
+ }
+
+ return fmt;
+}
+
static int new_stream_video(Muxer *mux, const OptionsContext *o,
OutputStream *ost)
{
@@ -558,9 +606,10 @@ static int new_stream_video(Muxer *mux, const OptionsContext *o,
if (!*++frame_pix_fmt)
frame_pix_fmt = NULL;
}
- if (frame_pix_fmt && (video_enc->pix_fmt = av_get_pix_fmt(frame_pix_fmt)) == AV_PIX_FMT_NONE) {
- av_log(ost, AV_LOG_FATAL, "Unknown pixel format requested: %s.\n", frame_pix_fmt);
- return AVERROR(EINVAL);
+ if (frame_pix_fmt) {
+ video_enc->pix_fmt = pix_fmt_parse(ost, frame_pix_fmt);
+ if (video_enc->pix_fmt == AV_PIX_FMT_NONE)
+ return AVERROR(EINVAL);
}
MATCH_PER_STREAM_OPT(intra_matrices, str, intra_matrix, oc, st);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (22 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 24/33] fftools/ffmpeg_mux_init: handle pixel format endianness Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 23:11 ` Michael Niedermayer
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 26/33] fftools/ffmpeg_filter: stop accessing encoder from pixfmt selection Anton Khirnov
` (8 subsequent siblings)
32 siblings, 1 reply; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
When the user explicitly specifies a pixel format that is not supported
by the encoder, ffmpeg CLI will currently use some heuristics to pick
another supported format. This is wrong and the correct action here is
to fail.
Surprisingly, a number of FATE tests are affected by this and actually
use a different pixel format than is specified in the makefiles.
---
doc/ffmpeg.texi | 3 +-
fftools/ffmpeg_filter.c | 35 +------------------
tests/fate/fits.mak | 6 ++--
tests/fate/lavf-video.mak | 2 +-
tests/fate/vcodec.mak | 4 +--
.../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
.../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
tests/ref/lavf/gif | 2 +-
8 files changed, 13 insertions(+), 47 deletions(-)
rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 6769f8d305..08b11097b7 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
@item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
Set pixel format. Use @code{-pix_fmts} to show all the supported
pixel formats.
-If the selected pixel format can not be selected, ffmpeg will print a
-warning and select the best pixel format supported by the encoder.
+
If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
if the requested pixel format can not be selected, and automatic conversions
inside filtergraphs are disabled.
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index caf85194c5..0625a9bc92 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -277,38 +277,6 @@ static const enum AVPixelFormat *get_compliance_normal_pix_fmts(const AVCodec *c
}
}
-static enum AVPixelFormat
-choose_pixel_fmt(const AVCodec *codec, enum AVPixelFormat target,
- int strict_std_compliance)
-{
- if (codec && codec->pix_fmts) {
- const enum AVPixelFormat *p = codec->pix_fmts;
- const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(target);
- //FIXME: This should check for AV_PIX_FMT_FLAG_ALPHA after PAL8 pixel format without alpha is implemented
- int has_alpha = desc ? desc->nb_components % 2 == 0 : 0;
- enum AVPixelFormat best= AV_PIX_FMT_NONE;
-
- if (strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
- p = get_compliance_normal_pix_fmts(codec, p);
- }
- for (; *p != AV_PIX_FMT_NONE; p++) {
- best = av_find_best_pix_fmt_of_2(best, *p, target, has_alpha, NULL);
- if (*p == target)
- break;
- }
- if (*p == AV_PIX_FMT_NONE) {
- if (target != AV_PIX_FMT_NONE)
- av_log(NULL, AV_LOG_WARNING,
- "Incompatible pixel format '%s' for codec '%s', auto-selecting format '%s'\n",
- av_get_pix_fmt_name(target),
- codec->name,
- av_get_pix_fmt_name(best));
- return best;
- }
- }
- return target;
-}
-
/* May return NULL (no pixel format found), a static string or a string
* backed by the bprint. Nothing has been written to the AVBPrint in case
* NULL is returned. The AVBPrint provided should be clean. */
@@ -327,8 +295,7 @@ static const char *choose_pix_fmts(OutputFilter *ofilter, AVBPrint *bprint)
return av_get_pix_fmt_name(ost->enc_ctx->pix_fmt);
}
if (ost->enc_ctx->pix_fmt != AV_PIX_FMT_NONE) {
- return av_get_pix_fmt_name(choose_pixel_fmt(enc->codec, enc->pix_fmt,
- ost->enc_ctx->strict_std_compliance));
+ return av_get_pix_fmt_name(enc->pix_fmt);
} else if (enc->codec->pix_fmts) {
const enum AVPixelFormat *p;
diff --git a/tests/fate/fits.mak b/tests/fate/fits.mak
index b9e99d97ee..d85946bc1a 100644
--- a/tests/fate/fits.mak
+++ b/tests/fate/fits.mak
@@ -8,8 +8,8 @@ tests/data/fits-multi.fits: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data
# TODO: Use an actual 64bit input file and fix the gbrp16 test on big-endian
fits-png-map-gray := gray8
fits-png-map-gbrp := rgb24
-fits-png-map-gbrp16 := rgb48
-fits-png-map-gbrap16le := rgba64
+fits-png-map-gbrp16be := rgb48
+fits-png-map-gbrap16be := rgba64
FATE_FITS_DEC-$(call FRAMECRC, FITS, FITS, SCALE_FILTER) += fate-fitsdec-ext_data_min_max
fate-fitsdec-ext_data_min_max: CMD = framecrc -i $(TARGET_SAMPLES)/fits/x0cj010ct_d0h.fit -pix_fmt gray16le -vf scale
@@ -30,7 +30,7 @@ fate-fitsdec-multi: CMD = framecrc -i $(TARGET_PATH)/tests/data/fits-multi.fits
fate-fitsdec%: PIXFMT = $(word 3, $(subst -, ,$(@)))
fate-fitsdec%: CMD = transcode image2 $(TARGET_SAMPLES)/png1/lena-$(fits-png-map-$(PIXFMT)).png fits "-vf scale -pix_fmt $(PIXFMT)"
-FATE_FITS_DEC_PIXFMT = gray gbrp gbrp16 gbrap16le
+FATE_FITS_DEC_PIXFMT = gray gbrp gbrp16be gbrap16be
FATE_FITS_DEC-$(call TRANSCODE, FITS, FITS, IMAGE2_DEMUXER PNG_DECODER SCALE_FILTER) += $(FATE_FITS_DEC_PIXFMT:%=fate-fitsdec-%)
FATE_FITS += $(FATE_FITS_DEC-yes)
diff --git a/tests/fate/lavf-video.mak b/tests/fate/lavf-video.mak
index e73f8f203b..da3b114bc8 100644
--- a/tests/fate/lavf-video.mak
+++ b/tests/fate/lavf-video.mak
@@ -27,7 +27,7 @@ fate-lavf-gbrp.fits: CMD = lavf_video "-pix_fmt gbrp"
fate-lavf-gbrap.fits: CMD = lavf_video "-pix_fmt gbrap"
fate-lavf-gbrp16be.fits: CMD = lavf_video "-pix_fmt gbrp16be"
fate-lavf-gbrap16be.fits: CMD = lavf_video "-pix_fmt gbrap16be"
-fate-lavf-gif: CMD = lavf_video "-pix_fmt rgb24"
+fate-lavf-gif: CMD = lavf_video "-pix_fmt rgb8"
FATE_AVCONV += $(FATE_LAVF_VIDEO)
fate-lavf-video fate-lavf: $(FATE_LAVF_VIDEO)
diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
index 2839e54de8..e32d28c556 100644
--- a/tests/fate/vcodec.mak
+++ b/tests/fate/vcodec.mak
@@ -210,9 +210,9 @@ fate-vsynth%-h263p: ENCOPTS = -qscale 2 -flags +aic -umv 1 -aiv 1 -
FATE_VCODEC_SCALE-$(call ENCDEC, HUFFYUV, AVI) += huffyuv huffyuvbgr24 huffyuvbgra
fate-vsynth%-huffyuv: ENCOPTS = -c:v huffyuv -pix_fmt yuv422p -sws_flags neighbor
fate-vsynth%-huffyuv: DECOPTS = -sws_flags neighbor
-fate-vsynth%-huffyuvbgr24: ENCOPTS = -c:v huffyuv -pix_fmt bgr24 -sws_flags neighbor
+fate-vsynth%-huffyuvbgr24: ENCOPTS = -c:v huffyuv -pix_fmt rgb24 -sws_flags neighbor
fate-vsynth%-huffyuvbgr24: DECOPTS = -sws_flags neighbor
-fate-vsynth%-huffyuvbgra: ENCOPTS = -c:v huffyuv -pix_fmt bgr32 -sws_flags neighbor
+fate-vsynth%-huffyuvbgra: ENCOPTS = -c:v huffyuv -pix_fmt rgb32 -sws_flags neighbor
fate-vsynth%-huffyuvbgra: DECOPTS = -sws_flags neighbor
FATE_VCODEC_SCALE-$(call ENCDEC, JPEGLS, AVI) += jpegls
diff --git a/tests/ref/fate/fitsdec-gbrap16le b/tests/ref/fate/fitsdec-gbrap16be
similarity index 79%
rename from tests/ref/fate/fitsdec-gbrap16le
rename to tests/ref/fate/fitsdec-gbrap16be
index 53ef980b13..1174a0f1d8 100644
--- a/tests/ref/fate/fitsdec-gbrap16le
+++ b/tests/ref/fate/fitsdec-gbrap16be
@@ -1,5 +1,5 @@
-64526d8da12d1fa07ceea5725647076f *tests/data/fate/fitsdec-gbrap16le.fits
-135360 tests/data/fate/fitsdec-gbrap16le.fits
+64526d8da12d1fa07ceea5725647076f *tests/data/fate/fitsdec-gbrap16be.fits
+135360 tests/data/fate/fitsdec-gbrap16be.fits
#tb 0: 1/1
#media_type 0: video
#codec_id 0: rawvideo
diff --git a/tests/ref/fate/fitsdec-gbrp16 b/tests/ref/fate/fitsdec-gbrp16be
similarity index 79%
rename from tests/ref/fate/fitsdec-gbrp16
rename to tests/ref/fate/fitsdec-gbrp16be
index 9250690e9b..ff4ca9e65c 100644
--- a/tests/ref/fate/fitsdec-gbrp16
+++ b/tests/ref/fate/fitsdec-gbrp16be
@@ -1,5 +1,5 @@
-2078208c93ba417d3fe150ba42bf5a30 *tests/data/fate/fitsdec-gbrp16.fits
-103680 tests/data/fate/fitsdec-gbrp16.fits
+2078208c93ba417d3fe150ba42bf5a30 *tests/data/fate/fitsdec-gbrp16be.fits
+103680 tests/data/fate/fitsdec-gbrp16be.fits
#tb 0: 1/1
#media_type 0: video
#codec_id 0: rawvideo
diff --git a/tests/ref/lavf/gif b/tests/ref/lavf/gif
index fc94b9df3d..7f353df286 100644
--- a/tests/ref/lavf/gif
+++ b/tests/ref/lavf/gif
@@ -1,3 +1,3 @@
e35f5ea283bbcb249818e0078ec72664 *tests/data/lavf/lavf.gif
2011766 tests/data/lavf/lavf.gif
-tests/data/lavf/lavf.gif CRC=0x2429faff
+tests/data/lavf/lavf.gif CRC=0x37f4d323
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format Anton Khirnov
@ 2023-07-13 23:11 ` Michael Niedermayer
2023-07-14 9:44 ` Anton Khirnov
0 siblings, 1 reply; 45+ messages in thread
From: Michael Niedermayer @ 2023-07-13 23:11 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2550 bytes --]
On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
> When the user explicitly specifies a pixel format that is not supported
> by the encoder, ffmpeg CLI will currently use some heuristics to pick
> another supported format. This is wrong and the correct action here is
> to fail.
>
> Surprisingly, a number of FATE tests are affected by this and actually
> use a different pixel format than is specified in the makefiles.
> ---
> doc/ffmpeg.texi | 3 +-
> fftools/ffmpeg_filter.c | 35 +------------------
> tests/fate/fits.mak | 6 ++--
> tests/fate/lavf-video.mak | 2 +-
> tests/fate/vcodec.mak | 4 +--
> .../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
> .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
> tests/ref/lavf/gif | 2 +-
> 8 files changed, 13 insertions(+), 47 deletions(-)
> rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
> rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
>
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 6769f8d305..08b11097b7 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
> @item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
> Set pixel format. Use @code{-pix_fmts} to show all the supported
> pixel formats.
> -If the selected pixel format can not be selected, ffmpeg will print a
> -warning and select the best pixel format supported by the encoder.
> +
> If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
> if the requested pixel format can not be selected, and automatic conversions
> inside filtergraphs are disabled.
The commit message makes this sound like a bugfix, while really this is
removing a documented feature.
It also breaks some scripts (fate is an example as it requires changes)
If the removial of that feature is intended, it should be argued somewhere
that this feature is never usefull.
To me as a lazy person it surely feels usefull to be able to ask for
both "exactly rgb" as well as something close to rgb (like bgr or gbrp)
without needing to know what each individual codec uses to return R,G,B
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Never trust a computer, one day, it may think you are the virus. -- Compn
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-13 23:11 ` Michael Niedermayer
@ 2023-07-14 9:44 ` Anton Khirnov
2023-07-14 10:20 ` Timo Rothenpieler
2023-07-14 15:47 ` Michael Niedermayer
0 siblings, 2 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-14 9:44 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Michael Niedermayer (2023-07-14 01:11:07)
> On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
> > When the user explicitly specifies a pixel format that is not supported
> > by the encoder, ffmpeg CLI will currently use some heuristics to pick
> > another supported format. This is wrong and the correct action here is
> > to fail.
> >
> > Surprisingly, a number of FATE tests are affected by this and actually
> > use a different pixel format than is specified in the makefiles.
> > ---
> > doc/ffmpeg.texi | 3 +-
> > fftools/ffmpeg_filter.c | 35 +------------------
> > tests/fate/fits.mak | 6 ++--
> > tests/fate/lavf-video.mak | 2 +-
> > tests/fate/vcodec.mak | 4 +--
> > .../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
> > .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
> > tests/ref/lavf/gif | 2 +-
> > 8 files changed, 13 insertions(+), 47 deletions(-)
> > rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
> > rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
> >
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index 6769f8d305..08b11097b7 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
> > @item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
> > Set pixel format. Use @code{-pix_fmts} to show all the supported
> > pixel formats.
> > -If the selected pixel format can not be selected, ffmpeg will print a
> > -warning and select the best pixel format supported by the encoder.
> > +
> > If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
> > if the requested pixel format can not be selected, and automatic conversions
> > inside filtergraphs are disabled.
>
> The commit message makes this sound like a bugfix, while really this is
> removing a documented feature.
It is a bugfix in my eyes. When you explicitly tell a program to perform
a specific action, and the program just decides to do something else,
then that program is broken.
As far as I can tell, this "feature" was added by you in 89f86379797
with no explanation or documentation beyond 'fix regression with png'.
It was later documented in a largely-unrelated commit that added
something else.
I see no argument whatsoever for why we should have such a "smart"
selection for pixel formats, but nothing else. It goes entirely against
the way any other option works.
> It also breaks some scripts (fate is an example as it requires changes)
No, its removal reveals already broken FATE tests, as I see no
indication that those tests intended to use a different format than
indicated on the commandline. It seems far more likely that this
misfeature confused the tests' authors as to what format is actually
used. That is IMO yet another argument for removing this.
> If the removial of that feature is intended, it should be argued somewhere
> that this feature is never usefull.
https://xkcd.com/1172/
> To me as a lazy person it surely feels usefull to be able to ask for
> both "exactly rgb" as well as something close to rgb (like bgr or gbrp)
> without needing to know what each individual codec uses to return R,G,B
1) This code does not give you the ability to specify "something close to rgb".
You specify a precise pixel format, and this code gives you
something. That something might be what you asked for, or something
close to it, or something completely unrelated.
E.g.
ffmpeg -i in.mkv -map 0:v -c:v libx264 -pix_fmt pal8 -t 1 out.mkv
produces yuv444p. How is it close to pal8?
2) If you want syntax for fuzzy format specification, patches are
welcome. But it should not interfere with specifying an exact format.
3) No other option in ffmpeg CLI works like this. If you specify
something, you get that or an error.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-14 9:44 ` Anton Khirnov
@ 2023-07-14 10:20 ` Timo Rothenpieler
2023-07-14 15:47 ` Michael Niedermayer
1 sibling, 0 replies; 45+ messages in thread
From: Timo Rothenpieler @ 2023-07-14 10:20 UTC (permalink / raw)
To: ffmpeg-devel
On 14/07/2023 11:44, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-14 01:11:07)
>> On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
>>> When the user explicitly specifies a pixel format that is not supported
>>> by the encoder, ffmpeg CLI will currently use some heuristics to pick
>>> another supported format. This is wrong and the correct action here is
>>> to fail.
>>>
>>> Surprisingly, a number of FATE tests are affected by this and actually
>>> use a different pixel format than is specified in the makefiles.
>>> ---
>>> doc/ffmpeg.texi | 3 +-
>>> fftools/ffmpeg_filter.c | 35 +------------------
>>> tests/fate/fits.mak | 6 ++--
>>> tests/fate/lavf-video.mak | 2 +-
>>> tests/fate/vcodec.mak | 4 +--
>>> .../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
>>> .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
>>> tests/ref/lavf/gif | 2 +-
>>> 8 files changed, 13 insertions(+), 47 deletions(-)
>>> rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
>>> rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
>>>
>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>>> index 6769f8d305..08b11097b7 100644
>>> --- a/doc/ffmpeg.texi
>>> +++ b/doc/ffmpeg.texi
>>> @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
>>> @item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
>>> Set pixel format. Use @code{-pix_fmts} to show all the supported
>>> pixel formats.
>>> -If the selected pixel format can not be selected, ffmpeg will print a
>>> -warning and select the best pixel format supported by the encoder.
>>> +
>>> If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
>>> if the requested pixel format can not be selected, and automatic conversions
>>> inside filtergraphs are disabled.
>>
>> The commit message makes this sound like a bugfix, while really this is
>> removing a documented feature.
>
> It is a bugfix in my eyes. When you explicitly tell a program to perform
> a specific action, and the program just decides to do something else,
> then that program is broken.
>
> As far as I can tell, this "feature" was added by you in 89f86379797
> with no explanation or documentation beyond 'fix regression with png'.
> It was later documented in a largely-unrelated commit that added
> something else.
>
> I see no argument whatsoever for why we should have such a "smart"
> selection for pixel formats, but nothing else. It goes entirely against
> the way any other option works.
>
>> It also breaks some scripts (fate is an example as it requires changes)
>
> No, its removal reveals already broken FATE tests, as I see no
> indication that those tests intended to use a different format than
> indicated on the commandline. It seems far more likely that this
> misfeature confused the tests' authors as to what format is actually
> used. That is IMO yet another argument for removing this.
>
>> If the removial of that feature is intended, it should be argued somewhere
>> that this feature is never usefull.
>
> https://xkcd.com/1172/
>
>> To me as a lazy person it surely feels usefull to be able to ask for
>> both "exactly rgb" as well as something close to rgb (like bgr or gbrp)
>> without needing to know what each individual codec uses to return R,G,B
>
> 1) This code does not give you the ability to specify "something close to rgb".
> You specify a precise pixel format, and this code gives you
> something. That something might be what you asked for, or something
> close to it, or something completely unrelated.
> E.g.
> ffmpeg -i in.mkv -map 0:v -c:v libx264 -pix_fmt pal8 -t 1 out.mkv
> produces yuv444p. How is it close to pal8?
> 2) If you want syntax for fuzzy format specification, patches are
> welcome. But it should not interfere with specifying an exact format.
> 3) No other option in ffmpeg CLI works like this. If you specify
> something, you get that or an error.
I completely agree. I didn't even realize this behaviour was a thing.
Getting a more or less random other pixel format than requested seems
very surprising and undesireable.
However, this is unarguably an API break and can and will break existing
scripts and applications.
So it should go through the usual deprecation cycle.
Maybe intensify the warning a bit, make it hang executing for a few
seconds so people take notice.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-14 9:44 ` Anton Khirnov
2023-07-14 10:20 ` Timo Rothenpieler
@ 2023-07-14 15:47 ` Michael Niedermayer
2023-07-14 17:06 ` Anton Khirnov
1 sibling, 1 reply; 45+ messages in thread
From: Michael Niedermayer @ 2023-07-14 15:47 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 5782 bytes --]
On Fri, Jul 14, 2023 at 11:44:07AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-14 01:11:07)
> > On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
> > > When the user explicitly specifies a pixel format that is not supported
> > > by the encoder, ffmpeg CLI will currently use some heuristics to pick
> > > another supported format. This is wrong and the correct action here is
> > > to fail.
> > >
> > > Surprisingly, a number of FATE tests are affected by this and actually
> > > use a different pixel format than is specified in the makefiles.
> > > ---
> > > doc/ffmpeg.texi | 3 +-
> > > fftools/ffmpeg_filter.c | 35 +------------------
> > > tests/fate/fits.mak | 6 ++--
> > > tests/fate/lavf-video.mak | 2 +-
> > > tests/fate/vcodec.mak | 4 +--
> > > .../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
> > > .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
> > > tests/ref/lavf/gif | 2 +-
> > > 8 files changed, 13 insertions(+), 47 deletions(-)
> > > rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
> > > rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
> > >
> > > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > > index 6769f8d305..08b11097b7 100644
> > > --- a/doc/ffmpeg.texi
> > > +++ b/doc/ffmpeg.texi
> > > @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
> > > @item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
> > > Set pixel format. Use @code{-pix_fmts} to show all the supported
> > > pixel formats.
> > > -If the selected pixel format can not be selected, ffmpeg will print a
> > > -warning and select the best pixel format supported by the encoder.
> > > +
> > > If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
> > > if the requested pixel format can not be selected, and automatic conversions
> > > inside filtergraphs are disabled.
> >
> > The commit message makes this sound like a bugfix, while really this is
> > removing a documented feature.
>
> It is a bugfix in my eyes. When you explicitly tell a program to perform
> a specific action, and the program just decides to do something else,
> then that program is broken.
>
> As far as I can tell, this "feature" was added by you in 89f86379797
> with no explanation or documentation beyond 'fix regression with png'.
> It was later documented in a largely-unrelated commit that added
> something else.
>
> I see no argument whatsoever for why we should have such a "smart"
As said previously,
The user cannot be expected to know if a implementation uses planar
or packed rgb, bgr or rgb.
This is not a inherent part of the file/stream/input in many cases
its not a problem for you because you are a FFmpeg developer and work
with this every day but it is a inconvenience for users
[...]
> > To me as a lazy person it surely feels usefull to be able to ask for
> > both "exactly rgb" as well as something close to rgb (like bgr or gbrp)
> > without needing to know what each individual codec uses to return R,G,B
>
> 1) This code does not give you the ability to specify "something close to rgb".
> You specify a precise pixel format, and this code gives you
> something. That something might be what you asked for, or something
> close to it, or something completely unrelated.
> E.g.
> ffmpeg -i in.mkv -map 0:v -c:v libx264 -pix_fmt pal8 -t 1 out.mkv
> produces yuv444p. How is it close to pal8?
(it is close because it can represent pal8 with little loss i think but)
your patch fixes this and adjusts the threshold to not accept formats
that are too different ?
i agree that would be a bugfix and also it should be quite easy to do
as teh code already computes what the differences are and computes a score
> 2) If you want syntax for fuzzy format specification, patches are
> welcome. But it should not interfere with specifying an exact format.
the code is there already
you found a case where it behaves unreasonable, so change the threshold
at which it accepts differences, you can even specify what differences
it accepts
theres also get_pix_fmt_score() that takes 2 pixel formats and gives
you a bitmask discribing their differences together with a numeric score
And once the threshold / mask is adjusted to accept what is considered
similar enough for the pixfmt ffmpeg command line use case.
You can move this to whatever syntax you prefer. You are the expert when
it comes to the ffmpeg command line.
Simply removing the code and calling for someone to add it back in
is a bit odd.
> 3) No other option in ffmpeg CLI works like this. If you specify
> something, you get that or an error.
iam not sure thats true
i think width and height and even vs odd have their fuzzyness at places and
so probably does the aspect ratio. Its not failing if it has to be rounded
to a close value
you could try
./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -aspect 512:511 test.m4v
there are only 8:8 bit so 512:511 cant be stored nothing fails you just
dont get 512:511
and iam pretty sure there are many more examples where "close" values
are taken silently
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-14 15:47 ` Michael Niedermayer
@ 2023-07-14 17:06 ` Anton Khirnov
2023-07-15 8:59 ` Paul B Mahol
2023-07-15 20:01 ` Michael Niedermayer
0 siblings, 2 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-14 17:06 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Michael Niedermayer (2023-07-14 17:47:19)
> On Fri, Jul 14, 2023 at 11:44:07AM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2023-07-14 01:11:07)
> > > On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
> > > > When the user explicitly specifies a pixel format that is not supported
> > > > by the encoder, ffmpeg CLI will currently use some heuristics to pick
> > > > another supported format. This is wrong and the correct action here is
> > > > to fail.
> > > >
> > > > Surprisingly, a number of FATE tests are affected by this and actually
> > > > use a different pixel format than is specified in the makefiles.
> > > > ---
> > > > doc/ffmpeg.texi | 3 +-
> > > > fftools/ffmpeg_filter.c | 35 +------------------
> > > > tests/fate/fits.mak | 6 ++--
> > > > tests/fate/lavf-video.mak | 2 +-
> > > > tests/fate/vcodec.mak | 4 +--
> > > > .../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
> > > > .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
> > > > tests/ref/lavf/gif | 2 +-
> > > > 8 files changed, 13 insertions(+), 47 deletions(-)
> > > > rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
> > > > rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
> > > >
> > > > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > > > index 6769f8d305..08b11097b7 100644
> > > > --- a/doc/ffmpeg.texi
> > > > +++ b/doc/ffmpeg.texi
> > > > @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
> > > > @item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
> > > > Set pixel format. Use @code{-pix_fmts} to show all the supported
> > > > pixel formats.
> > > > -If the selected pixel format can not be selected, ffmpeg will print a
> > > > -warning and select the best pixel format supported by the encoder.
> > > > +
> > > > If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
> > > > if the requested pixel format can not be selected, and automatic conversions
> > > > inside filtergraphs are disabled.
> > >
> > > The commit message makes this sound like a bugfix, while really this is
> > > removing a documented feature.
> >
> > It is a bugfix in my eyes. When you explicitly tell a program to perform
> > a specific action, and the program just decides to do something else,
> > then that program is broken.
> >
> > As far as I can tell, this "feature" was added by you in 89f86379797
> > with no explanation or documentation beyond 'fix regression with png'.
> > It was later documented in a largely-unrelated commit that added
> > something else.
> >
> > I see no argument whatsoever for why we should have such a "smart"
>
> As said previously,
> The user cannot be expected to know if a implementation uses planar
> or packed rgb, bgr or rgb.
Which is why
* libavfilter will by default convert to a format supported by the
encoder
* libavcodec will now helpfully print a list of formats supported by the
encoder if the caller gives it a wrong one
> This is not a inherent part of the file/stream/input in many cases
> its not a problem for you because you are a FFmpeg developer and work
> with this every day but it is a inconvenience for users
Should we then replace any failing commandline with something that will
not fail? Ignore any options with incorrect values? All in the name of
convenience? Maybe you should try web development.
Programs that try to second-guess user's explicit instructions are
broken by design. This "convenience" argument is entirely specious:
* users who do not know what they want get something that works by
default
* users who specify a wrong format get a list of correct formats they
can just pick from; that is as convenient as it gets for this kind of
a program
* users who require yet more convenience and/or handholding can use a
graphical program such as Handbrake; we should not try to be
Handbrake, they are better at it than us
> > > To me as a lazy person it surely feels usefull to be able to ask for
> > > both "exactly rgb" as well as something close to rgb (like bgr or gbrp)
> > > without needing to know what each individual codec uses to return R,G,B
> >
>
> > 1) This code does not give you the ability to specify "something close to rgb".
> > You specify a precise pixel format, and this code gives you
> > something. That something might be what you asked for, or something
> > close to it, or something completely unrelated.
> > E.g.
> > ffmpeg -i in.mkv -map 0:v -c:v libx264 -pix_fmt pal8 -t 1 out.mkv
> > produces yuv444p. How is it close to pal8?
>
> (it is close because it can represent pal8 with little loss i think but)
pal8 and yuv444p are close? I really wonder which pair of formats would
be not close for you then. If the set is non-empty, I'm sure I can craft
a commandline that produces one instead of the other.
> your patch fixes this and adjusts the threshold to not accept formats
> that are too different ?
> i agree that would be a bugfix and also it should be quite easy to do
> as teh code already computes what the differences are and computes a score
>
>
> > 2) If you want syntax for fuzzy format specification, patches are
> > welcome. But it should not interfere with specifying an exact format.
>
> the code is there already
> you found a case where it behaves unreasonable, so change the threshold
> at which it accepts differences, you can even specify what differences
> it accepts
> theres also get_pix_fmt_score() that takes 2 pixel formats and gives
> you a bitmask discribing their differences together with a numeric score
>
> And once the threshold / mask is adjusted to accept what is considered
> similar enough for the pixfmt ffmpeg command line use case.
> You can move this to whatever syntax you prefer. You are the expert when
> it comes to the ffmpeg command line.
> Simply removing the code and calling for someone to add it back in
> is a bit odd.
Then my expert opinion is this - this code:
* is broken by design
* is actively harmful
* solves no actual problem
* adds unnecessary complexity
It should be removed in its entirety.
> > 3) No other option in ffmpeg CLI works like this. If you specify
> > something, you get that or an error.
>
> iam not sure thats true
> i think width and height and even vs odd have their fuzzyness at places and
> so probably does the aspect ratio. Its not failing if it has to be rounded
> to a close value
>
> you could try
> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -aspect 512:511 test.m4v
> there are only 8:8 bit so 512:511 cant be stored nothing fails you just
> dont get 512:511
>
> and iam pretty sure there are many more examples where "close" values
> are taken silently
ffmpeg -i in.mkv -map 0:v -s 512x511 -c:v libx264 -f null -
[...]
[libx264 @ 0x55f8029a71c0] height not divisible by 2 (512x511)
[vost#0:0/libx264 @ 0x55f802a61840] Error while opening encoder - maybe incorrect parameters such as bit_rate, rate, width or height.
Besides, you keep talking about "close" when the code in question makes
no guarantee whatsoever that the result is in any way "close" (whatever
that might even mean).
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-14 17:06 ` Anton Khirnov
@ 2023-07-15 8:59 ` Paul B Mahol
2023-07-15 20:01 ` Michael Niedermayer
1 sibling, 0 replies; 45+ messages in thread
From: Paul B Mahol @ 2023-07-15 8:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On Fri, Jul 14, 2023 at 7:06 PM Anton Khirnov <anton@khirnov.net> wrote:
> Quoting Michael Niedermayer (2023-07-14 17:47:19)
> > On Fri, Jul 14, 2023 at 11:44:07AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2023-07-14 01:11:07)
> > > > On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
> > > > > When the user explicitly specifies a pixel format that is not
> supported
> > > > > by the encoder, ffmpeg CLI will currently use some heuristics to
> pick
> > > > > another supported format. This is wrong and the correct action
> here is
> > > > > to fail.
> > > > >
> > > > > Surprisingly, a number of FATE tests are affected by this and
> actually
> > > > > use a different pixel format than is specified in the makefiles.
> > > > > ---
> > > > > doc/ffmpeg.texi | 3 +-
> > > > > fftools/ffmpeg_filter.c | 35
> +------------------
> > > > > tests/fate/fits.mak | 6 ++--
> > > > > tests/fate/lavf-video.mak | 2 +-
> > > > > tests/fate/vcodec.mak | 4 +--
> > > > > .../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
> > > > > .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
> > > > > tests/ref/lavf/gif | 2 +-
> > > > > 8 files changed, 13 insertions(+), 47 deletions(-)
> > > > > rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be}
> (79%)
> > > > > rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
> > > > >
> > > > > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > > > > index 6769f8d305..08b11097b7 100644
> > > > > --- a/doc/ffmpeg.texi
> > > > > +++ b/doc/ffmpeg.texi
> > > > > @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
> > > > > @item -pix_fmt[:@var{stream_specifier}] @var{format}
> (@emph{input/output,per-stream})
> > > > > Set pixel format. Use @code{-pix_fmts} to show all the supported
> > > > > pixel formats.
> > > > > -If the selected pixel format can not be selected, ffmpeg will
> print a
> > > > > -warning and select the best pixel format supported by the encoder.
> > > > > +
> > > > > If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with
> an error
> > > > > if the requested pixel format can not be selected, and automatic
> conversions
> > > > > inside filtergraphs are disabled.
> > > >
> > > > The commit message makes this sound like a bugfix, while really this
> is
> > > > removing a documented feature.
> > >
> > > It is a bugfix in my eyes. When you explicitly tell a program to
> perform
> > > a specific action, and the program just decides to do something else,
> > > then that program is broken.
> > >
> > > As far as I can tell, this "feature" was added by you in 89f86379797
> > > with no explanation or documentation beyond 'fix regression with png'.
> > > It was later documented in a largely-unrelated commit that added
> > > something else.
> > >
> > > I see no argument whatsoever for why we should have such a "smart"
> >
> > As said previously,
> > The user cannot be expected to know if a implementation uses planar
> > or packed rgb, bgr or rgb.
>
> Which is why
> * libavfilter will by default convert to a format supported by the
> encoder
> * libavcodec will now helpfully print a list of formats supported by the
> encoder if the caller gives it a wrong one
>
> > This is not a inherent part of the file/stream/input in many cases
> > its not a problem for you because you are a FFmpeg developer and work
> > with this every day but it is a inconvenience for users
>
> Should we then replace any failing commandline with something that will
> not fail? Ignore any options with incorrect values? All in the name of
> convenience? Maybe you should try web development.
>
> Programs that try to second-guess user's explicit instructions are
> broken by design. This "convenience" argument is entirely specious:
> * users who do not know what they want get something that works by
> default
> * users who specify a wrong format get a list of correct formats they
> can just pick from; that is as convenient as it gets for this kind of
> a program
> * users who require yet more convenience and/or handholding can use a
> graphical program such as Handbrake; we should not try to be
> Handbrake, they are better at it than us
>
> > > > To me as a lazy person it surely feels usefull to be able to ask for
> > > > both "exactly rgb" as well as something close to rgb (like bgr or
> gbrp)
> > > > without needing to know what each individual codec uses to return
> R,G,B
> > >
> >
> > > 1) This code does not give you the ability to specify "something close
> to rgb".
> > > You specify a precise pixel format, and this code gives you
> > > something. That something might be what you asked for, or something
> > > close to it, or something completely unrelated.
> > > E.g.
> > > ffmpeg -i in.mkv -map 0:v -c:v libx264 -pix_fmt pal8 -t 1 out.mkv
> > > produces yuv444p. How is it close to pal8?
> >
> > (it is close because it can represent pal8 with little loss i think but)
>
> pal8 and yuv444p are close? I really wonder which pair of formats would
> be not close for you then. If the set is non-empty, I'm sure I can craft
> a commandline that produces one instead of the other.
>
> > your patch fixes this and adjusts the threshold to not accept formats
> > that are too different ?
> > i agree that would be a bugfix and also it should be quite easy to do
> > as teh code already computes what the differences are and computes a
> score
> >
> >
> > > 2) If you want syntax for fuzzy format specification, patches are
> > > welcome. But it should not interfere with specifying an exact
> format.
> >
> > the code is there already
> > you found a case where it behaves unreasonable, so change the threshold
> > at which it accepts differences, you can even specify what differences
> > it accepts
> > theres also get_pix_fmt_score() that takes 2 pixel formats and gives
> > you a bitmask discribing their differences together with a numeric score
> >
> > And once the threshold / mask is adjusted to accept what is considered
> > similar enough for the pixfmt ffmpeg command line use case.
> > You can move this to whatever syntax you prefer. You are the expert when
> > it comes to the ffmpeg command line.
> > Simply removing the code and calling for someone to add it back in
> > is a bit odd.
>
> Then my expert opinion is this - this code:
> * is broken by design
> * is actively harmful
> * solves no actual problem
> * adds unnecessary complexity
> It should be removed in its entirety.
>
> > > 3) No other option in ffmpeg CLI works like this. If you specify
> > > something, you get that or an error.
> >
> > iam not sure thats true
> > i think width and height and even vs odd have their fuzzyness at places
> and
> > so probably does the aspect ratio. Its not failing if it has to be
> rounded
> > to a close value
> >
> > you could try
> > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -aspect 512:511 test.m4v
> > there are only 8:8 bit so 512:511 cant be stored nothing fails you just
> > dont get 512:511
> >
> > and iam pretty sure there are many more examples where "close" values
> > are taken silently
>
> ffmpeg -i in.mkv -map 0:v -s 512x511 -c:v libx264 -f null -
> [...]
> [libx264 @ 0x55f8029a71c0] height not divisible by 2 (512x511)
> [vost#0:0/libx264 @ 0x55f802a61840] Error while opening encoder - maybe
> incorrect parameters such as bit_rate, rate, width or height.
>
> Besides, you keep talking about "close" when the code in question makes
> no guarantee whatsoever that the result is in any way "close" (whatever
> that might even mean).
>
Agreed, this patch is good to go into repo.
>
> --
> Anton Khirnov
> _______________________________________________
> 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".
>
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format
2023-07-14 17:06 ` Anton Khirnov
2023-07-15 8:59 ` Paul B Mahol
@ 2023-07-15 20:01 ` Michael Niedermayer
1 sibling, 0 replies; 45+ messages in thread
From: Michael Niedermayer @ 2023-07-15 20:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 8738 bytes --]
Hi
On Fri, Jul 14, 2023 at 07:06:15PM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-14 17:47:19)
> > On Fri, Jul 14, 2023 at 11:44:07AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2023-07-14 01:11:07)
> > > > On Thu, Jul 13, 2023 at 12:55:45PM +0200, Anton Khirnov wrote:
> > > > > When the user explicitly specifies a pixel format that is not supported
> > > > > by the encoder, ffmpeg CLI will currently use some heuristics to pick
> > > > > another supported format. This is wrong and the correct action here is
> > > > > to fail.
> > > > >
> > > > > Surprisingly, a number of FATE tests are affected by this and actually
> > > > > use a different pixel format than is specified in the makefiles.
> > > > > ---
> > > > > doc/ffmpeg.texi | 3 +-
> > > > > fftools/ffmpeg_filter.c | 35 +------------------
> > > > > tests/fate/fits.mak | 6 ++--
> > > > > tests/fate/lavf-video.mak | 2 +-
> > > > > tests/fate/vcodec.mak | 4 +--
> > > > > .../{fitsdec-gbrap16le => fitsdec-gbrap16be} | 4 +--
> > > > > .../fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} | 4 +--
> > > > > tests/ref/lavf/gif | 2 +-
> > > > > 8 files changed, 13 insertions(+), 47 deletions(-)
> > > > > rename tests/ref/fate/{fitsdec-gbrap16le => fitsdec-gbrap16be} (79%)
> > > > > rename tests/ref/fate/{fitsdec-gbrp16 => fitsdec-gbrp16be} (79%)
> > > > >
> > > > > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > > > > index 6769f8d305..08b11097b7 100644
> > > > > --- a/doc/ffmpeg.texi
> > > > > +++ b/doc/ffmpeg.texi
> > > > > @@ -1014,8 +1014,7 @@ Disable autoscale at your own risk.
> > > > > @item -pix_fmt[:@var{stream_specifier}] @var{format} (@emph{input/output,per-stream})
> > > > > Set pixel format. Use @code{-pix_fmts} to show all the supported
> > > > > pixel formats.
> > > > > -If the selected pixel format can not be selected, ffmpeg will print a
> > > > > -warning and select the best pixel format supported by the encoder.
> > > > > +
> > > > > If @var{pix_fmt} is prefixed by a @code{+}, ffmpeg will exit with an error
> > > > > if the requested pixel format can not be selected, and automatic conversions
> > > > > inside filtergraphs are disabled.
> > > >
> > > > The commit message makes this sound like a bugfix, while really this is
> > > > removing a documented feature.
> > >
> > > It is a bugfix in my eyes. When you explicitly tell a program to perform
> > > a specific action, and the program just decides to do something else,
> > > then that program is broken.
> > >
> > > As far as I can tell, this "feature" was added by you in 89f86379797
> > > with no explanation or documentation beyond 'fix regression with png'.
> > > It was later documented in a largely-unrelated commit that added
> > > something else.
> > >
> > > I see no argument whatsoever for why we should have such a "smart"
> >
> > As said previously,
> > The user cannot be expected to know if a implementation uses planar
> > or packed rgb, bgr or rgb.
>
> Which is why
> * libavfilter will by default convert to a format supported by the
> encoder
If the user uses -pix_fmt she likely doesnt want "any" format but has a
preferrance for some reason like 8bit or rgb for example
> * libavcodec will now helpfully print a list of formats supported by the
> encoder if the caller gives it a wrong one
Thats good, but its a case per case adjustment.
>
> > This is not a inherent part of the file/stream/input in many cases
> > its not a problem for you because you are a FFmpeg developer and work
> > with this every day but it is a inconvenience for users
>
> Should we then replace any failing commandline with something that will
> not fail? Ignore any options with incorrect values? All in the name of
> convenience? Maybe you should try web development.
-pix_fmt is not "any command line"
https://en.wikipedia.org/wiki/Straw_man
>
> Programs that try to second-guess user's explicit instructions are
> broken by design.
Maybe but the pix_fmt has 2 syntaxes one for explicit instructions
and one for a close one. Your patch modifies the "pick a close one" path
> This "convenience" argument is entirely specious:
> * users who do not know what they want get something that works by
> default
Thats not true, 16bit yuv might not work for example for the users case
> * users who specify a wrong format get a list of correct formats they
> can just pick from; that is as convenient as it gets for this kind of
> a program
it works probably for most cases but its an extra step for the user
and a change in command line syntax
> * users who require yet more convenience and/or handholding can use a
> graphical program such as Handbrake; we should not try to be
> Handbrake, they are better at it than us
I dont understand what you are trying to say here
"require yet more convenience" is a very strange wording
I dont require convenience, i can use intels documentation, teh ELF
docs and a hex editor. But I instead use a compiler. Similarly
I surely can manually tyoe out the number of pixels for each frame
used but instead i expect ffmpeg to do that for me from the width and
height. Why should i not be able to tell FFmpeg to use a 8bit RGB format?
and instead receive a error message with a list of which format the
implementation supports than search the RGB variant be that RGB, BGR or GBRP
and write that back to ffmpeg in a 2nd call ?
This is not related to GUI vs command line interface. a GUI can show
that list too.
>
> > > > To me as a lazy person it surely feels usefull to be able to ask for
> > > > both "exactly rgb" as well as something close to rgb (like bgr or gbrp)
> > > > without needing to know what each individual codec uses to return R,G,B
> > >
> >
> > > 1) This code does not give you the ability to specify "something close to rgb".
> > > You specify a precise pixel format, and this code gives you
> > > something. That something might be what you asked for, or something
> > > close to it, or something completely unrelated.
> > > E.g.
> > > ffmpeg -i in.mkv -map 0:v -c:v libx264 -pix_fmt pal8 -t 1 out.mkv
> > > produces yuv444p. How is it close to pal8?
> >
> > (it is close because it can represent pal8 with little loss i think but)
>
> pal8 and yuv444p are close? I really wonder which pair of formats would
> be not close for you then. If the set is non-empty, I'm sure I can craft
> a commandline that produces one instead of the other.
You are misunderstanding what i meant
pal8 -> yuv444p relatively little loss of information
yuv444p -> pal8 not "close" because substantial loss of information
[...]
> > > 3) No other option in ffmpeg CLI works like this. If you specify
> > > something, you get that or an error.
> >
> > iam not sure thats true
> > i think width and height and even vs odd have their fuzzyness at places and
> > so probably does the aspect ratio. Its not failing if it has to be rounded
> > to a close value
> >
> > you could try
> > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -aspect 512:511 test.m4v
> > there are only 8:8 bit so 512:511 cant be stored nothing fails you just
> > dont get 512:511
> >
> > and iam pretty sure there are many more examples where "close" values
> > are taken silently
>
> ffmpeg -i in.mkv -map 0:v -s 512x511 -c:v libx264 -f null -
> [...]
> [libx264 @ 0x55f8029a71c0] height not divisible by 2 (512x511)
> [vost#0:0/libx264 @ 0x55f802a61840] Error while opening encoder - maybe incorrect parameters such as bit_rate, rate, width or height.
maybe you want to remove "force_divisible_by" too and let the user
specify the value explicitly
>
> Besides, you keep talking about "close" when the code in question makes
> no guarantee whatsoever that the result is in any way "close" (whatever
> that might even mean).
The code picks the "closest" format.
That can also be seen in your example of pal8 ->yuv444p. Where the encoder
supports nothing closer.
Noone seems to have been bothered before by the fact that the code makes
such choices if its fed by an impossible target.
As said previously, you can just adjust the value at which it hard fails
Do you want me to look at how that can be done and send a patch doing that ?
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No snowflake in an avalanche ever feels responsible. -- Voltaire
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread
* [FFmpeg-devel] [PATCH 26/33] fftools/ffmpeg_filter: stop accessing encoder from pixfmt selection
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (23 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 25/33] fftools/ffmpeg_filter: stop disregarding user-specified pixel format Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 27/33] fftools/ffmpeg: rework initializing encoders with no frames Anton Khirnov
` (7 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
ffmpeg CLI pixel format selection for filtering currently special-cases
MJPEG encoding, where it will restrict the supported list of pixel
formats depending on the value of the -strict option. In order to get
that value it will apply it from the options dict into the encoder
context, which is a highly invasive action even now, and would become a
race once encoding is moved to its own thread.
The ugliness of this code can be much reduced by moving the special
handling of MJPEG into ofilter_bind_ost(), which is called from encoder
init and is thus synchronized with it. There is also no need to write
anything to the encoder context, we can evaluate the option into our
stack variable.
There is also no need to access AVCodec at all during pixel format
selection, as the pixel formats array is already stored in
OutputFilterPriv.
---
fftools/ffmpeg_filter.c | 65 ++++++++++++++++++++---------------------
1 file changed, 32 insertions(+), 33 deletions(-)
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 0625a9bc92..67a5f48245 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -262,47 +262,22 @@ static void sub2video_update(InputFilterPriv *ifp, int64_t heartbeat_pts,
ifp->sub2video.initialize = 0;
}
-// FIXME: YUV420P etc. are actually supported with full color range,
-// yet the latter information isn't available here.
-static const enum AVPixelFormat *get_compliance_normal_pix_fmts(const AVCodec *codec, const enum AVPixelFormat default_formats[])
-{
- static const enum AVPixelFormat mjpeg_formats[] =
- { AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
- AV_PIX_FMT_NONE };
-
- if (!strcmp(codec->name, "mjpeg")) {
- return mjpeg_formats;
- } else {
- return default_formats;
- }
-}
-
/* May return NULL (no pixel format found), a static string or a string
* backed by the bprint. Nothing has been written to the AVBPrint in case
* NULL is returned. The AVBPrint provided should be clean. */
static const char *choose_pix_fmts(OutputFilter *ofilter, AVBPrint *bprint)
{
+ OutputFilterPriv *ofp = ofp_from_ofilter(ofilter);
OutputStream *ost = ofilter->ost;
- AVCodecContext *enc = ost->enc_ctx;
- const AVDictionaryEntry *strict_dict = av_dict_get(ost->encoder_opts, "strict", NULL, 0);
- if (strict_dict)
- // used by choose_pixel_fmt() and below
- av_opt_set(ost->enc_ctx, "strict", strict_dict->value, 0);
- if (ost->keep_pix_fmt) {
- if (ost->enc_ctx->pix_fmt == AV_PIX_FMT_NONE)
- return NULL;
- return av_get_pix_fmt_name(ost->enc_ctx->pix_fmt);
+ if (ost->keep_pix_fmt) {
+ return ofp->format == AV_PIX_FMT_NONE ? NULL :
+ av_get_pix_fmt_name(ofp->format);
}
- if (ost->enc_ctx->pix_fmt != AV_PIX_FMT_NONE) {
- return av_get_pix_fmt_name(enc->pix_fmt);
- } else if (enc->codec->pix_fmts) {
- const enum AVPixelFormat *p;
-
- p = enc->codec->pix_fmts;
- if (ost->enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
- p = get_compliance_normal_pix_fmts(enc->codec, p);
- }
+ if (ofp->format != AV_PIX_FMT_NONE) {
+ return av_get_pix_fmt_name(ofp->format);
+ } else if (ofp->formats) {
+ const enum AVPixelFormat *p = ofp->formats;
for (; *p != AV_PIX_FMT_NONE; p++) {
const char *name = av_get_pix_fmt_name(*p);
@@ -661,6 +636,30 @@ void ofilter_bind_ost(OutputFilter *ofilter, OutputStream *ost)
ofp->format = ost->enc_ctx->pix_fmt;
} else {
ofp->formats = c->pix_fmts;
+
+ // MJPEG encoder exports a full list of supported pixel formats,
+ // but the full-range ones are experimental-only.
+ // Restrict the auto-conversion list unless -strict experimental
+ // has been specified.
+ if (!strcmp(c->name, "mjpeg")) {
+ // FIXME: YUV420P etc. are actually supported with full color range,
+ // yet the latter information isn't available here.
+ static const enum AVPixelFormat mjpeg_formats[] =
+ { AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
+ AV_PIX_FMT_NONE };
+
+ const AVDictionaryEntry *strict = av_dict_get(ost->encoder_opts, "strict", NULL, 0);
+ int strict_val = ost->enc_ctx->strict_std_compliance;
+
+ if (strict) {
+ const AVOption *o = av_opt_find(ost->enc_ctx, strict->key, NULL, 0, 0);
+ av_assert0(o);
+ av_opt_eval_int(ost->enc_ctx, o, strict->value, &strict_val);
+ }
+
+ if (strict_val > FF_COMPLIANCE_UNOFFICIAL)
+ ofp->formats = mjpeg_formats;
+ }
}
fgp->disable_conversions |= ost->keep_pix_fmt;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 27/33] fftools/ffmpeg: rework initializing encoders with no frames
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (24 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 26/33] fftools/ffmpeg_filter: stop accessing encoder from pixfmt selection Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 28/33] fftools/ffmpeg_filter: only flush vsync code if encoding actually started Anton Khirnov
` (6 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
When no frames were passed from a filtergraph to an encoder, but the
filtergraph is configured (i.e. has output parameters), encoder flush
code will use those parameters to initialize the encoder in a last-ditch
effort to produce some useful output.
Rework this process so that it is triggered by the filtergraph, which
now sends a dummy frame with parameters, but no data, to the encoder,
rather than the encoder reaching backwards into the filter.
This approach is more in line with the natural data flow from filters to
encoders and will allow to reduce encoder-filter interactions in
following commits.
This code is tested by fate-adpcm-ima-cunning-trunc-t2-track1, which (as
confirmed by Zane) is supposed to produce empty output.
---
fftools/ffmpeg_enc.c | 22 ++---------------
fftools/ffmpeg_filter.c | 54 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 52 insertions(+), 24 deletions(-)
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index b35dbb6290..390f1b3b57 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -1142,26 +1142,8 @@ void enc_flush(void)
AVCodecContext *enc = ost->enc_ctx;
OutputFile *of = output_files[ost->file_index];
- if (!enc)
- continue;
-
- // Try to enable encoding with no input frames.
- // Maybe we should just let encoding fail instead.
- if (!e->opened) {
- FilterGraph *fg = ost->filter->graph;
-
- av_log(ost, AV_LOG_WARNING,
- "Finishing stream without any data written to it.\n");
-
- if (!fg->graph)
- continue;
-
- ret = enc_open(ost, NULL);
- if (ret < 0)
- exit_program(1);
- }
-
- if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != AVMEDIA_TYPE_AUDIO)
+ if (!enc || !e->opened ||
+ (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != AVMEDIA_TYPE_AUDIO))
continue;
ret = submit_encode_frame(of, ost, NULL);
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 67a5f48245..f8e64ce6cc 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -143,11 +143,17 @@ typedef struct OutputFilterPriv {
int sample_rate;
AVChannelLayout ch_layout;
+ AVRational time_base;
+ AVRational sample_aspect_ratio;
+
// those are only set if no format is specified and the encoder gives us multiple options
// They point directly to the relevant lists of the encoder.
const int *formats;
const AVChannelLayout *ch_layouts;
const int *sample_rates;
+
+ // set to 1 after at least one frame passed through this output
+ int got_frame;
} OutputFilterPriv;
static OutputFilterPriv *ofp_from_ofilter(OutputFilter *ofilter)
@@ -1576,6 +1582,9 @@ static int configure_filtergraph(FilterGraph *fg)
ofp->width = av_buffersink_get_w(sink);
ofp->height = av_buffersink_get_h(sink);
+ ofp->time_base = av_buffersink_get_time_base(sink);
+ ofp->sample_aspect_ratio = av_buffersink_get_sample_aspect_ratio(sink);
+
ofp->sample_rate = av_buffersink_get_sample_rate(sink);
av_channel_layout_uninit(&ofp->ch_layout);
ret = av_buffersink_get_ch_layout(sink, &ofp->ch_layout);
@@ -1688,6 +1697,7 @@ int reap_filters(int flush)
{
/* Reap all buffers present in the buffer sinks */
for (OutputStream *ost = ost_iter(NULL); ost; ost = ost_iter(ost)) {
+ OutputFilterPriv *ofp;
FilterGraphPriv *fgp;
AVFrame *filtered_frame;
AVFilterContext *filter;
@@ -1697,6 +1707,7 @@ int reap_filters(int flush)
continue;
filter = ost->filter->filter;
fgp = fgp_from_fg(ost->filter->graph);
+ ofp = ofp_from_ofilter(ost->filter);
filtered_frame = fgp->frame;
@@ -1749,6 +1760,7 @@ int reap_filters(int flush)
enc_frame(ost, filtered_frame);
av_frame_unref(filtered_frame);
+ ofp->got_frame = 1;
}
}
@@ -1961,6 +1973,7 @@ int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame, int keep_reference)
int fg_transcode_step(FilterGraph *graph, InputStream **best_ist)
{
+ FilterGraphPriv *fgp = fgp_from_fg(graph);
int i, ret;
int nb_requests, nb_requests_max = 0;
InputStream *ist;
@@ -1988,10 +2001,43 @@ int fg_transcode_step(FilterGraph *graph, InputStream **best_ist)
return reap_filters(0);
if (ret == AVERROR_EOF) {
- ret = reap_filters(1);
- for (i = 0; i < graph->nb_outputs; i++)
- close_output_stream(graph->outputs[i]->ost);
- return ret;
+ reap_filters(1);
+ for (int i = 0; i < graph->nb_outputs; i++) {
+ OutputFilter *ofilter = graph->outputs[i];
+ OutputFilterPriv *ofp = ofp_from_ofilter(ofilter);
+
+ // we are finished and no frames were ever seen at this output,
+ // at least initialize the encoder with a dummy frame
+ if (!ofp->got_frame) {
+ AVFrame *frame = fgp->frame;
+
+ frame->time_base = ofp->time_base;
+ frame->format = ofp->format;
+
+ frame->width = ofp->width;
+ frame->height = ofp->height;
+ frame->sample_aspect_ratio = ofp->sample_aspect_ratio;
+
+ frame->sample_rate = ofp->sample_rate;
+ if (ofp->ch_layout.nb_channels) {
+ ret = av_channel_layout_copy(&frame->ch_layout, &ofp->ch_layout);
+ if (ret < 0)
+ return ret;
+ }
+
+ av_assert0(!frame->buf[0]);
+
+ av_log(ofilter->ost, AV_LOG_WARNING,
+ "No filtered frames for output stream, trying to "
+ "initialize anyway.\n");
+
+ enc_open(ofilter->ost, frame);
+ av_frame_unref(frame);
+ }
+
+ close_output_stream(ofilter->ost);
+ }
+ return 0;
}
if (ret != AVERROR(EAGAIN))
return ret;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 28/33] fftools/ffmpeg_filter: only flush vsync code if encoding actually started
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (25 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 27/33] fftools/ffmpeg: rework initializing encoders with no frames Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 29/33] fftools/ffmpeg_enc: initialize audio/video encoders from frame parameters Anton Khirnov
` (5 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
Otherwise this has no effect.
Will be useful in following commits.
---
fftools/ffmpeg_filter.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index f8e64ce6cc..4955fe38dd 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -1720,10 +1720,10 @@ int reap_filters(int flush)
if (ret != AVERROR(EAGAIN) && ret != AVERROR_EOF) {
av_log(fgp, AV_LOG_WARNING,
"Error in av_buffersink_get_frame_flags(): %s\n", av_err2str(ret));
- } else if (flush && ret == AVERROR_EOF) {
- if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_VIDEO)
- enc_frame(ost, NULL);
- }
+ } else if (flush && ret == AVERROR_EOF && ofp->got_frame &&
+ av_buffersink_get_type(filter) == AVMEDIA_TYPE_VIDEO)
+ enc_frame(ost, NULL);
+
break;
}
if (ost->finished) {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 29/33] fftools/ffmpeg_enc: initialize audio/video encoders from frame parameters
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (26 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 28/33] fftools/ffmpeg_filter: only flush vsync code if encoding actually started Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 30/33] fftools/ffmpeg_filter: make OutputFilter.filter private Anton Khirnov
` (4 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
This is possible now that enc_open() is always called with a non-NULL
frame for audio/video.
Previously the code would directly reach into the buffersink, which is a
layering violation.
---
fftools/ffmpeg_enc.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 390f1b3b57..88c62124a9 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -34,8 +34,6 @@
#include "libavutil/rational.h"
#include "libavutil/timestamp.h"
-#include "libavfilter/buffersink.h"
-
#include "libavcodec/avcodec.h"
// FIXME private header, used for mid_pred()
@@ -198,12 +196,21 @@ int enc_open(OutputStream *ost, AVFrame *frame)
AVCodecContext *dec_ctx = NULL;
const AVCodec *enc = enc_ctx->codec;
OutputFile *of = output_files[ost->file_index];
- FrameData *fd = frame ? frame_data(frame) : NULL;
+ FrameData *fd;
int ret;
if (e->opened)
return 0;
+ // frame is always non-NULL for audio and video
+ av_assert0(frame || (enc->type != AVMEDIA_TYPE_VIDEO && enc->type != AVMEDIA_TYPE_AUDIO));
+
+ if (frame) {
+ fd = frame_data(frame);
+ if (!fd)
+ return AVERROR(ENOMEM);
+ }
+
set_encoder_id(output_files[ost->file_index], ost);
if (ist) {
@@ -212,15 +219,15 @@ int enc_open(OutputStream *ost, AVFrame *frame)
switch (enc_ctx->codec_type) {
case AVMEDIA_TYPE_AUDIO:
- enc_ctx->sample_fmt = av_buffersink_get_format(ost->filter->filter);
- enc_ctx->sample_rate = av_buffersink_get_sample_rate(ost->filter->filter);
- ret = av_buffersink_get_ch_layout(ost->filter->filter, &enc_ctx->ch_layout);
+ enc_ctx->sample_fmt = frame->format;
+ enc_ctx->sample_rate = frame->sample_rate;
+ ret = av_channel_layout_copy(&enc_ctx->ch_layout, &frame->ch_layout);
if (ret < 0)
return ret;
if (ost->bits_per_raw_sample)
enc_ctx->bits_per_raw_sample = ost->bits_per_raw_sample;
- else if (fd)
+ else
enc_ctx->bits_per_raw_sample = FFMIN(fd->bits_per_raw_sample,
av_get_bytes_per_sample(enc_ctx->sample_fmt) << 3);
@@ -231,7 +238,7 @@ int enc_open(OutputStream *ost, AVFrame *frame)
case AVMEDIA_TYPE_VIDEO: {
AVRational fr = ost->frame_rate;
- if (!fr.num && fd)
+ if (!fr.num)
fr = fd->frame_rate_filter;
if (!fr.num && !ost->max_frame_rate.num) {
fr = (AVRational){25, 1};
@@ -261,7 +268,7 @@ int enc_open(OutputStream *ost, AVFrame *frame)
av_inv_q(fr);
if (!(enc_ctx->time_base.num && enc_ctx->time_base.den))
- enc_ctx->time_base = av_buffersink_get_time_base(ost->filter->filter);
+ enc_ctx->time_base = frame->time_base;
if ( av_q2d(enc_ctx->time_base) < 0.001 && ost->vsync_method != VSYNC_PASSTHROUGH
&& (ost->vsync_method == VSYNC_CFR || ost->vsync_method == VSYNC_VSCFR ||
(ost->vsync_method == VSYNC_AUTO && !(of->format->flags & AVFMT_VARIABLE_FPS)))){
@@ -270,18 +277,18 @@ int enc_open(OutputStream *ost, AVFrame *frame)
"setting vsync/fps_mode to vfr\n");
}
- enc_ctx->width = av_buffersink_get_w(ost->filter->filter);
- enc_ctx->height = av_buffersink_get_h(ost->filter->filter);
+ enc_ctx->width = frame->width;
+ enc_ctx->height = frame->height;
enc_ctx->sample_aspect_ratio = ost->st->sample_aspect_ratio =
ost->frame_aspect_ratio.num ? // overridden by the -aspect cli option
av_mul_q(ost->frame_aspect_ratio, (AVRational){ enc_ctx->height, enc_ctx->width }) :
- av_buffersink_get_sample_aspect_ratio(ost->filter->filter);
+ frame->sample_aspect_ratio;
- enc_ctx->pix_fmt = av_buffersink_get_format(ost->filter->filter);
+ enc_ctx->pix_fmt = frame->format;
if (ost->bits_per_raw_sample)
enc_ctx->bits_per_raw_sample = ost->bits_per_raw_sample;
- else if (fd)
+ else
enc_ctx->bits_per_raw_sample = FFMIN(fd->bits_per_raw_sample,
av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth);
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 30/33] fftools/ffmpeg_filter: make OutputFilter.filter private
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (27 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 29/33] fftools/ffmpeg_enc: initialize audio/video encoders from frame parameters Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 31/33] fftools/ffmpeg: add more structure to FrameData Anton Khirnov
` (3 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
It should not be accessed from outside of filtering code.
---
fftools/ffmpeg.h | 1 -
fftools/ffmpeg_filter.c | 18 ++++++++++--------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index bcd9cc584e..bd6b0ed6d1 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -283,7 +283,6 @@ typedef struct InputFilter {
} InputFilter;
typedef struct OutputFilter {
- AVFilterContext *filter;
struct OutputStream *ost;
struct FilterGraph *graph;
uint8_t *name;
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 4955fe38dd..e14b8f0f3c 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -137,6 +137,8 @@ static InputFilterPriv *ifp_from_ifilter(InputFilter *ifilter)
typedef struct OutputFilterPriv {
OutputFilter ofilter;
+ AVFilterContext *filter;
+
/* desired output stream properties */
int format;
int width, height;
@@ -1060,7 +1062,7 @@ static int configure_output_video_filter(FilterGraph *fg, OutputFilter *ofilter,
char name[255];
snprintf(name, sizeof(name), "out_%d_%d", ost->file_index, ost->index);
- ret = avfilter_graph_create_filter(&ofilter->filter,
+ ret = avfilter_graph_create_filter(&ofp->filter,
avfilter_get_by_name("buffersink"),
name, NULL, NULL, fg->graph);
@@ -1116,7 +1118,7 @@ static int configure_output_video_filter(FilterGraph *fg, OutputFilter *ofilter,
return ret;
- if ((ret = avfilter_link(last_filter, pad_idx, ofilter->filter, 0)) < 0)
+ if ((ret = avfilter_link(last_filter, pad_idx, ofp->filter, 0)) < 0)
return ret;
return 0;
@@ -1134,12 +1136,12 @@ static int configure_output_audio_filter(FilterGraph *fg, OutputFilter *ofilter,
int ret;
snprintf(name, sizeof(name), "out_%d_%d", ost->file_index, ost->index);
- ret = avfilter_graph_create_filter(&ofilter->filter,
+ ret = avfilter_graph_create_filter(&ofp->filter,
avfilter_get_by_name("abuffersink"),
name, NULL, NULL, fg->graph);
if (ret < 0)
return ret;
- if ((ret = av_opt_set_int(ofilter->filter, "all_channel_counts", 1, AV_OPT_SEARCH_CHILDREN)) < 0)
+ if ((ret = av_opt_set_int(ofp->filter, "all_channel_counts", 1, AV_OPT_SEARCH_CHILDREN)) < 0)
return ret;
#define AUTO_INSERT_FILTER(opt_name, filter_name, arg) do { \
@@ -1222,7 +1224,7 @@ static int configure_output_audio_filter(FilterGraph *fg, OutputFilter *ofilter,
if (ret < 0)
goto fail;
- if ((ret = avfilter_link(last_filter, pad_idx, ofilter->filter, 0)) < 0)
+ if ((ret = avfilter_link(last_filter, pad_idx, ofp->filter, 0)) < 0)
goto fail;
fail:
av_bprint_finalize(&args, NULL);
@@ -1470,7 +1472,7 @@ static void cleanup_filtergraph(FilterGraph *fg)
{
int i;
for (i = 0; i < fg->nb_outputs; i++)
- fg->outputs[i]->filter = (AVFilterContext *)NULL;
+ ofp_from_ofilter(fg->outputs[i])->filter = NULL;
for (i = 0; i < fg->nb_inputs; i++)
ifp_from_ifilter(fg->inputs[i])->filter = NULL;
avfilter_graph_free(&fg->graph);
@@ -1575,7 +1577,7 @@ static int configure_filtergraph(FilterGraph *fg)
for (i = 0; i < fg->nb_outputs; i++) {
OutputFilter *ofilter = fg->outputs[i];
OutputFilterPriv *ofp = ofp_from_ofilter(ofilter);
- AVFilterContext *sink = ofilter->filter;
+ AVFilterContext *sink = ofp->filter;
ofp->format = av_buffersink_get_format(sink);
@@ -1705,9 +1707,9 @@ int reap_filters(int flush)
if (!ost->filter || !ost->filter->graph->graph)
continue;
- filter = ost->filter->filter;
fgp = fgp_from_fg(ost->filter->graph);
ofp = ofp_from_ofilter(ost->filter);
+ filter = ofp->filter;
filtered_frame = fgp->frame;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 31/33] fftools/ffmpeg: add more structure to FrameData
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (28 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 30/33] fftools/ffmpeg_filter: make OutputFilter.filter private Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 32/33] fftools/ffmpeg: rework -enc_time_base handling Anton Khirnov
` (2 subsequent siblings)
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
It now contains data from multiple sources, so group those items that
always come from the decoder. Also, initialize them to invalid values,
so that frames that did not originate from a decoder can be
distinguished.
---
fftools/ffmpeg.c | 8 +++++++-
fftools/ffmpeg.h | 10 +++++++---
fftools/ffmpeg_dec.c | 6 +++---
fftools/ffmpeg_enc.c | 6 +++---
4 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index dd7cfcf632..64778bc0f3 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -434,9 +434,15 @@ InputStream *ist_iter(InputStream *prev)
FrameData *frame_data(AVFrame *frame)
{
if (!frame->opaque_ref) {
- frame->opaque_ref = av_buffer_allocz(sizeof(FrameData));
+ FrameData *fd;
+
+ frame->opaque_ref = av_buffer_allocz(sizeof(*fd));
if (!frame->opaque_ref)
return NULL;
+ fd = (FrameData*)frame->opaque_ref->data;
+
+ fd->dec.frame_num = UINT64_MAX;
+ fd->dec.pts = AV_NOPTS_VALUE;
}
return (FrameData*)frame->opaque_ref->data;
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index bd6b0ed6d1..97aa4d716e 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -626,9 +626,13 @@ typedef struct OutputFile {
// optionally attached as opaque_ref to decoded AVFrames
typedef struct FrameData {
- uint64_t idx;
- int64_t pts;
- AVRational tb;
+ // properties that come from the decoder
+ struct {
+ uint64_t frame_num;
+
+ int64_t pts;
+ AVRational tb;
+ } dec;
AVRational frame_rate_filter;
diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index a1d811f512..5c1b8888e9 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -612,9 +612,9 @@ static int packet_decode(InputStream *ist, const AVPacket *pkt, AVFrame *frame)
av_frame_unref(frame);
return AVERROR(ENOMEM);
}
- fd->pts = frame->pts;
- fd->tb = dec->pkt_timebase;
- fd->idx = dec->frame_num - 1;
+ fd->dec.pts = frame->pts;
+ fd->dec.tb = dec->pkt_timebase;
+ fd->dec.frame_num = dec->frame_num - 1;
fd->bits_per_raw_sample = dec->bits_per_raw_sample;
frame->time_base = dec->pkt_timebase;
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 88c62124a9..0e2285c5a2 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -569,8 +569,8 @@ void enc_stats_write(OutputStream *ost, EncStats *es,
if ((frame && frame->opaque_ref) || (pkt && pkt->opaque_ref)) {
fd = (const FrameData*)(frame ? frame->opaque_ref->data : pkt->opaque_ref->data);
- tbi = fd->tb;
- ptsi = fd->pts;
+ tbi = fd->dec.tb;
+ ptsi = fd->dec.pts;
}
for (size_t i = 0; i < es->nb_components; i++) {
@@ -588,7 +588,7 @@ void enc_stats_write(OutputStream *ost, EncStats *es,
case ENC_STATS_PTS_TIME_IN: avio_printf(io, "%g", ptsi == INT64_MAX ?
INFINITY : ptsi * av_q2d(tbi)); continue;
case ENC_STATS_FRAME_NUM: avio_printf(io, "%"PRIu64, frame_num); continue;
- case ENC_STATS_FRAME_NUM_IN: avio_printf(io, "%"PRIu64, fd ? fd->idx : -1); continue;
+ case ENC_STATS_FRAME_NUM_IN: avio_printf(io, "%"PRIu64, fd ? fd->dec.frame_num : -1); continue;
}
if (frame) {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 32/33] fftools/ffmpeg: rework -enc_time_base handling
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (29 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 31/33] fftools/ffmpeg: add more structure to FrameData Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 33/33] doc/ffmpeg: fix -enc_time_base documentation Anton Khirnov
2023-07-13 12:01 ` [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting "zhilizhao(赵志立)"
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
Read the timebase from FrameData rather than the input stream. This
should fix #10393 and generally be more reliable.
Replace the use of '-1' to indicate demuxing timebase with the string
'demux'. Also allow to request filter timebase with
'-enc_time_base filter'.
---
doc/ffmpeg.texi | 7 ++++---
fftools/ffmpeg.h | 6 ++++++
fftools/ffmpeg_enc.c | 16 ++++++++++++++--
fftools/ffmpeg_mux_init.c | 31 +++++++++++++++++++------------
tests/fate/video.mak | 2 +-
5 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 08b11097b7..b41e4d9272 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1811,10 +1811,11 @@ Assign a default value according to the media type.
For video - use 1/framerate, for audio - use 1/samplerate.
-@item -1
-Use the input stream timebase when possible.
+@item demux
+Use the timebase from the demuxer.
-If an input stream is not available, the default timebase will be used.
+@item filter
+Use the timebase from the filtergraph.
@item >0
Use the provided number as the timebase.
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 97aa4d716e..f45ddf33b2 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -56,6 +56,7 @@
#define FFMPEG_ROTATION_METADATA 1
#define FFMPEG_OPT_QPHIST 1
#define FFMPEG_OPT_ADRIFT_THRESHOLD 1
+#define FFMPEG_OPT_ENC_TIME_BASE_NUM 1
enum VideoSyncMethod {
VSYNC_AUTO = -1,
@@ -66,6 +67,11 @@ enum VideoSyncMethod {
VSYNC_DROP,
};
+enum EncTimeBase {
+ ENC_TIME_BASE_DEMUX = -1,
+ ENC_TIME_BASE_FILTER = -2,
+};
+
#define MAX_STREAMS 1024 /* arbitrary sanity check value */
enum HWAccelID {
diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index 0e2285c5a2..1489b2f179 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -264,8 +264,20 @@ int enc_open(OutputStream *ost, AVFrame *frame)
fr.num, fr.den, 65535);
}
- enc_ctx->time_base = ost->enc_timebase.num > 0 ? ost->enc_timebase :
- av_inv_q(fr);
+ if (ost->enc_timebase.num == ENC_TIME_BASE_DEMUX) {
+ if (fd->dec.tb.num <= 0 || fd->dec.tb.den <= 0) {
+ av_log(ost, AV_LOG_ERROR,
+ "Demuxing timebase not available - cannot use it for encoding\n");
+ return AVERROR(EINVAL);
+ }
+
+ enc_ctx->time_base = fd->dec.tb;
+ } else if (ost->enc_timebase.num == ENC_TIME_BASE_FILTER) {
+ enc_ctx->time_base = frame->time_base;
+ } else {
+ enc_ctx->time_base = ost->enc_timebase.num > 0 ? ost->enc_timebase :
+ av_inv_q(fr);
+ }
if (!(enc_ctx->time_base.num && enc_ctx->time_base.den))
enc_ctx->time_base = frame->time_base;
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 4434e6b70d..2d45fa7c7c 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -1220,20 +1220,27 @@ static int ost_add(Muxer *mux, const OptionsContext *o, enum AVMediaType type,
MATCH_PER_STREAM_OPT(enc_time_bases, str, enc_time_base, oc, st);
if (enc_time_base) {
AVRational q;
- if (av_parse_ratio(&q, enc_time_base, INT_MAX, 0, NULL) < 0 ||
- q.den <= 0) {
- av_log(ost, AV_LOG_FATAL, "Invalid time base: %s\n", enc_time_base);
- return AVERROR(EINVAL);
- }
- if (q.num < 0) {
- if (!ost->ist) {
- av_log(ost, AV_LOG_FATAL,
- "Cannot use input stream timebase for encoding - "
- "no input stream available\n");
- return AVERROR(EINVAL);
+ if (!strcmp(enc_time_base, "demux")) {
+ q = (AVRational){ ENC_TIME_BASE_DEMUX, 0 };
+ } else if (!strcmp(enc_time_base, "filter")) {
+ q = (AVRational){ ENC_TIME_BASE_FILTER, 0 };
+ } else {
+ ret = av_parse_ratio(&q, enc_time_base, INT_MAX, 0, NULL);
+ if (ret < 0 || q.den <= 0
+#if !FFMPEG_OPT_ENC_TIME_BASE_NUM
+ || q.num < 0
+#endif
+ ) {
+ av_log(ost, AV_LOG_FATAL, "Invalid time base: %s\n", enc_time_base);
+ return ret < 0 ? ret : AVERROR(EINVAL);
}
- q = ost->ist->st->time_base;
+#if FFMPEG_OPT_ENC_TIME_BASE_NUM
+ if (q.num < 0)
+ av_log(ost, AV_LOG_WARNING, "-enc_time_base -1 is deprecated,"
+ " use -enc_timebase demux\n");
+#endif
}
+
ost->enc_timebase = q;
}
} else {
diff --git a/tests/fate/video.mak b/tests/fate/video.mak
index a2011d0dad..4e7a77537f 100644
--- a/tests/fate/video.mak
+++ b/tests/fate/video.mak
@@ -270,7 +270,7 @@ FATE_VIDEO-$(call FRAMECRC, MXG, MXPEG) += fate-mxpeg
fate-mxpeg: CMD = framecrc -idct simple -flags +bitexact -i $(TARGET_SAMPLES)/mxpeg/m1.mxg -an
FATE_NUV += fate-nuv-rtjpeg
-fate-nuv-rtjpeg: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/nuv/Today.nuv -an -enc_time_base -1
+fate-nuv-rtjpeg: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/nuv/Today.nuv -an -enc_time_base demux
FATE_NUV += fate-nuv-rtjpeg-fh
fate-nuv-rtjpeg-fh: CMD = framecrc -idct simple -i $(TARGET_SAMPLES)/nuv/rtjpeg_frameheader.nuv -an
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* [FFmpeg-devel] [PATCH 33/33] doc/ffmpeg: fix -enc_time_base documentation
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (30 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 32/33] fftools/ffmpeg: rework -enc_time_base handling Anton Khirnov
@ 2023-07-13 10:55 ` Anton Khirnov
2023-07-13 12:01 ` [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting "zhilizhao(赵志立)"
32 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 10:55 UTC (permalink / raw)
To: ffmpeg-devel
Stop claiming the argument is always a floating point number, which
* confuses floating point and decimal numbers
* is not always true even accounting for the above point
---
doc/ffmpeg.texi | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index b41e4d9272..a6ef5590c7 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1802,8 +1802,7 @@ Try to make the choice automatically, in order to generate a sane output.
Default value is -1.
@item -enc_time_base[:@var{stream_specifier}] @var{timebase} (@emph{output,per-stream})
-Set the encoder timebase. @var{timebase} is a floating point number,
-and can assume one of the following values:
+Set the encoder timebase. @var{timebase} can assume one of the following values:
@table @option
@item 0
@@ -1817,11 +1816,11 @@ Use the timebase from the demuxer.
@item filter
Use the timebase from the filtergraph.
-@item >0
+@item a positive number
Use the provided number as the timebase.
This field can be provided as a ratio of two integers (e.g. 1:24, 1:48000)
-or as a floating point number (e.g. 0.04166, 2.0833e-5)
+or as a decimal number (e.g. 0.04166, 2.0833e-5)
@end table
Default value is 0.
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting
2023-07-13 10:55 [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting Anton Khirnov
` (31 preceding siblings ...)
2023-07-13 10:55 ` [FFmpeg-devel] [PATCH 33/33] doc/ffmpeg: fix -enc_time_base documentation Anton Khirnov
@ 2023-07-13 12:01 ` "zhilizhao(赵志立)"
2023-07-13 13:01 ` Anton Khirnov
32 siblings, 1 reply; 45+ messages in thread
From: "zhilizhao(赵志立)" @ 2023-07-13 12:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
> On Jul 13, 2023, at 18:55, Anton Khirnov <anton@khirnov.net> wrote:
>
> ---
> fftools/ffmpeg_mux_init.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
> index 6ab541d7c5..8a3e7b98cf 100644
> --- a/fftools/ffmpeg_mux_init.c
> +++ b/fftools/ffmpeg_mux_init.c
> @@ -2380,7 +2380,7 @@ int of_open(const OptionsContext *o, const char *filename)
> int64_t start_time = o->start_time == AV_NOPTS_VALUE ? 0 : o->start_time;
> if (stop_time <= start_time) {
> av_log(mux, AV_LOG_ERROR, "-to value smaller than -ss; aborting.\n");
> - exit_program(1);
> + return AVERROR(EINVAL);
The log message still says ‘aborting’.
> } else {
> recording_time = stop_time - start_time;
> }
> @@ -2401,7 +2401,7 @@ int of_open(const OptionsContext *o, const char *filename)
> if (!oc) {
> av_log(mux, AV_LOG_FATAL, "Error initializing the muxer for %s: %s\n",
> filename, av_err2str(err));
> - exit_program(1);
> + return err;
> }
> mux->fc = oc;
>
> @@ -2437,7 +2437,7 @@ int of_open(const OptionsContext *o, const char *filename)
> "Output filename '%s' does not contain a numeric pattern like "
> "'%%d', which is required by output format '%s'.\n",
> oc->url, oc->oformat->name);
> - exit_program(1);
> + return AVERROR(EINVAL);
> }
>
> if (!(oc->oformat->flags & AVFMT_NOFILE)) {
> @@ -2450,7 +2450,7 @@ int of_open(const OptionsContext *o, const char *filename)
> &mux->opts)) < 0) {
> av_log(mux, AV_LOG_FATAL, "Error opening output %s: %s\n",
> filename, av_err2str(err));
> - exit_program(1);
> + return err;
> }
> } else if (strcmp(oc->oformat->name, "image2")==0 && !av_filename_number_test(filename))
> assert_file_overwrite(filename);
> @@ -2469,7 +2469,7 @@ int of_open(const OptionsContext *o, const char *filename)
> err = set_dispositions(mux, o);
> if (err < 0) {
> av_log(mux, AV_LOG_FATAL, "Error setting output stream dispositions\n");
> - exit_program(1);
> + return err;
> }
>
> // parse forced keyframe specifications;
> @@ -2477,13 +2477,13 @@ int of_open(const OptionsContext *o, const char *filename)
> err = process_forced_keyframes(mux, o);
> if (err < 0) {
> av_log(mux, AV_LOG_FATAL, "Error processing forced keyframes\n");
> - exit_program(1);
> + return err;
> }
>
> err = setup_sync_queues(mux, oc, o->shortest_buf_duration * AV_TIME_BASE);
> if (err < 0) {
> av_log(mux, AV_LOG_FATAL, "Error setting up output sync queues\n");
> - exit_program(1);
> + return err;
> }
>
> of->url = filename;
> @@ -2500,7 +2500,7 @@ int of_open(const OptionsContext *o, const char *filename)
>
> err = init_output_stream_nofilter(ost);
> if (err < 0)
> - report_and_exit(err);
> + return err;
> }
>
> /* write the header for files with no streams */
> --
> 2.40.1
>
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting
2023-07-13 12:01 ` [FFmpeg-devel] [PATCH 01/33] fftools/ffmpeg_mux_init: return errors from of_open() instead of aborting "zhilizhao(赵志立)"
@ 2023-07-13 13:01 ` Anton Khirnov
0 siblings, 0 replies; 45+ messages in thread
From: Anton Khirnov @ 2023-07-13 13:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting zhilizhao(赵志立) (2023-07-13 14:01:48)
>
>
> > On Jul 13, 2023, at 18:55, Anton Khirnov <anton@khirnov.net> wrote:
> >
> > ---
> > fftools/ffmpeg_mux_init.c | 16 ++++++++--------
> > 1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
> > index 6ab541d7c5..8a3e7b98cf 100644
> > --- a/fftools/ffmpeg_mux_init.c
> > +++ b/fftools/ffmpeg_mux_init.c
> > @@ -2380,7 +2380,7 @@ int of_open(const OptionsContext *o, const char *filename)
> > int64_t start_time = o->start_time == AV_NOPTS_VALUE ? 0 : o->start_time;
> > if (stop_time <= start_time) {
> > av_log(mux, AV_LOG_ERROR, "-to value smaller than -ss; aborting.\n");
> > - exit_program(1);
> > + return AVERROR(EINVAL);
>
> The log message still says ‘aborting’.
From the user's POV it does abort the transcoding, so the message is IMO
still accurate. The change is only in how precisely the abort is done.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 45+ messages in thread