* [FFmpeg-devel] [PATCH] Better printing of errors on format negotiation failure (PR #20851)
@ 2025-11-06 17:11 Niklas Haas via ffmpeg-devel
0 siblings, 0 replies; only message in thread
From: Niklas Haas via ffmpeg-devel @ 2025-11-06 17:11 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Niklas Haas
PR #20851 opened by Niklas Haas (haasn)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20851
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20851.patch
The current code only ever printed the pixel format; even when the negotiation error was in a completely different format.
>From c1717cb6669f986a8bd04c8bfc5e37927c12769d Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Thu, 6 Nov 2025 17:26:19 +0100
Subject: [PATCH 1/5] avfilter/format: add print_list() to
AVFilterFormatsMerger
So that the generic code can correctly print format lists for failing mergers.
---
libavfilter/formats.c | 41 +++++++++++++++++++++++++++++++++++++++++
libavfilter/formats.h | 3 +++
2 files changed, 44 insertions(+)
diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index 847199dda1..b9487fe0cf 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -20,6 +20,7 @@
*/
#include "libavutil/avassert.h"
+#include "libavutil/bprint.h"
#include "libavutil/channel_layout.h"
#include "libavutil/common.h"
#include "libavutil/mem.h"
@@ -345,6 +346,39 @@ static int merge_generic(void *a, void *b)
return merge_generic_internal(a, b, 0);
}
+#define PRINT_NAME(type, type_name) \
+static void print_##type_name(AVBPrint *bp, const void *fmtsp) \
+{ \
+ const AVFilterFormats *fmts = fmtsp; \
+ for (int i = 0; i < fmts->nb_formats; i++) { \
+ const char *name = av_##type_name(fmts->formats[i]); \
+ av_bprint_chars(bp, ' ', i ? 1 : 0); \
+ av_bprint_append_data(bp, name, name ? strlen(name) : 0); \
+ } \
+}
+
+PRINT_NAME(enum AVSampleFormat, get_sample_fmt_name)
+PRINT_NAME(enum AVPixelFormat, get_pix_fmt_name)
+PRINT_NAME(enum AVColorSpace, color_space_name)
+PRINT_NAME(enum AVColorRange, color_range_name)
+PRINT_NAME(enum AVAlphaMode, alpha_mode_name)
+
+static void print_channel_layout_desc(AVBPrint *bp, const void *layoutsp)
+{
+ const AVFilterChannelLayouts *layouts = layoutsp;
+ for (int i = 0; i < layouts->nb_channel_layouts; i++) {
+ av_bprint_chars(bp, ' ', i ? 1 : 0);
+ av_channel_layout_describe_bprint(&layouts->channel_layouts[i], bp);
+ }
+}
+
+static void print_sample_rate(AVBPrint *bp, const void *ratesp)
+{
+ const AVFilterFormats *rates = ratesp;
+ for (int i = 0; i < rates->nb_formats; i++)
+ av_bprintf(bp, "%s%d", i ? " " : "", rates->formats[i]);
+}
+
#define CONVERSION_FILTER_SWSCALE \
.conversion_filter = "scale", \
.conversion_opts_offset = offsetof(AVFilterGraph, scale_sws_opts),
@@ -358,24 +392,28 @@ static const AVFilterFormatsMerger mergers_video[] = {
.offset = offsetof(AVFilterFormatsConfig, formats),
.merge = merge_pix_fmts,
.can_merge = can_merge_pix_fmts,
+ .print_list = print_get_pix_fmt_name,
CONVERSION_FILTER_SWSCALE
},
{
.offset = offsetof(AVFilterFormatsConfig, color_spaces),
.merge = merge_generic,
.can_merge = can_merge_generic,
+ .print_list = print_color_space_name,
CONVERSION_FILTER_SWSCALE
},
{
.offset = offsetof(AVFilterFormatsConfig, color_ranges),
.merge = merge_generic,
.can_merge = can_merge_generic,
+ .print_list = print_color_range_name,
CONVERSION_FILTER_SWSCALE
},
{
.offset = offsetof(AVFilterFormatsConfig, alpha_modes),
.merge = merge_generic,
.can_merge = can_merge_generic,
+ .print_list = print_alpha_mode_name,
.conversion_filter = "premultiply_dynamic",
},
};
@@ -385,18 +423,21 @@ static const AVFilterFormatsMerger mergers_audio[] = {
.offset = offsetof(AVFilterFormatsConfig, channel_layouts),
.merge = merge_channel_layouts,
.can_merge = can_merge_channel_layouts,
+ .print_list = print_channel_layout_desc,
CONVERSION_FILTER_ARESAMPLE
},
{
.offset = offsetof(AVFilterFormatsConfig, samplerates),
.merge = merge_samplerates,
.can_merge = can_merge_samplerates,
+ .print_list = print_sample_rate,
CONVERSION_FILTER_ARESAMPLE
},
{
.offset = offsetof(AVFilterFormatsConfig, formats),
.merge = merge_sample_fmts,
.can_merge = can_merge_sample_fmts,
+ .print_list = print_get_sample_fmt_name,
CONVERSION_FILTER_ARESAMPLE
},
};
diff --git a/libavfilter/formats.h b/libavfilter/formats.h
index 0c92ecad3f..d0b2a0b96a 100644
--- a/libavfilter/formats.h
+++ b/libavfilter/formats.h
@@ -513,10 +513,13 @@ int ff_formats_check_color_ranges(void *log, const AVFilterFormats *fmts);
*/
int ff_formats_check_alpha_modes(void *log, const AVFilterFormats *fmts);
+struct AVBPrint;
+
typedef struct AVFilterFormatMerger {
unsigned offset;
int (*merge)(void *a, void *b);
int (*can_merge)(const void *a, const void *b);
+ void (*print_list)(struct AVBPrint *bp, const void *fmts);
const char *conversion_filter;
unsigned conversion_opts_offset;
} AVFilterFormatsMerger;
--
2.49.1
>From f5bc9704edb3254e65c108509052f19eb77622c1 Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Thu, 6 Nov 2025 17:48:32 +0100
Subject: [PATCH 2/5] avfilter/formats: constify ff_filter_get_negotiation
---
libavfilter/formats.c | 2 +-
libavfilter/formats.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index b9487fe0cf..36e45c53c1 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -452,7 +452,7 @@ static const AVFilterNegotiation negotiate_audio = {
.mergers = mergers_audio,
};
-const AVFilterNegotiation *ff_filter_get_negotiation(AVFilterLink *link)
+const AVFilterNegotiation *ff_filter_get_negotiation(const AVFilterLink *link)
{
switch (link->type) {
case AVMEDIA_TYPE_VIDEO: return &negotiate_video;
diff --git a/libavfilter/formats.h b/libavfilter/formats.h
index d0b2a0b96a..4fe96187ba 100644
--- a/libavfilter/formats.h
+++ b/libavfilter/formats.h
@@ -614,6 +614,6 @@ typedef struct AVFilterNegotiation {
const AVFilterFormatsMerger *mergers;
} AVFilterNegotiation;
-const AVFilterNegotiation *ff_filter_get_negotiation(AVFilterLink *link);
+const AVFilterNegotiation *ff_filter_get_negotiation(const AVFilterLink *link);
#endif /* AVFILTER_FORMATS_H */
--
2.49.1
>From ad5b151f88aed49b4c36e764186214c13e0788ac Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Thu, 6 Nov 2025 17:58:10 +0100
Subject: [PATCH 3/5] avfilter/formats: add name field to AVFilterFormatMerger
Needed to properly print format lists on format configuration failure.
---
libavfilter/formats.c | 7 +++++++
libavfilter/formats.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/libavfilter/formats.c b/libavfilter/formats.c
index 36e45c53c1..5ba961f059 100644
--- a/libavfilter/formats.c
+++ b/libavfilter/formats.c
@@ -389,6 +389,7 @@ static void print_sample_rate(AVBPrint *bp, const void *ratesp)
static const AVFilterFormatsMerger mergers_video[] = {
{
+ .name = "Pixel formats",
.offset = offsetof(AVFilterFormatsConfig, formats),
.merge = merge_pix_fmts,
.can_merge = can_merge_pix_fmts,
@@ -396,6 +397,7 @@ static const AVFilterFormatsMerger mergers_video[] = {
CONVERSION_FILTER_SWSCALE
},
{
+ .name = "Color spaces",
.offset = offsetof(AVFilterFormatsConfig, color_spaces),
.merge = merge_generic,
.can_merge = can_merge_generic,
@@ -403,6 +405,7 @@ static const AVFilterFormatsMerger mergers_video[] = {
CONVERSION_FILTER_SWSCALE
},
{
+ .name = "Color ranges",
.offset = offsetof(AVFilterFormatsConfig, color_ranges),
.merge = merge_generic,
.can_merge = can_merge_generic,
@@ -410,6 +413,7 @@ static const AVFilterFormatsMerger mergers_video[] = {
CONVERSION_FILTER_SWSCALE
},
{
+ .name = "Alpha modes",
.offset = offsetof(AVFilterFormatsConfig, alpha_modes),
.merge = merge_generic,
.can_merge = can_merge_generic,
@@ -420,6 +424,7 @@ static const AVFilterFormatsMerger mergers_video[] = {
static const AVFilterFormatsMerger mergers_audio[] = {
{
+ .name = "Channel layouts",
.offset = offsetof(AVFilterFormatsConfig, channel_layouts),
.merge = merge_channel_layouts,
.can_merge = can_merge_channel_layouts,
@@ -427,6 +432,7 @@ static const AVFilterFormatsMerger mergers_audio[] = {
CONVERSION_FILTER_ARESAMPLE
},
{
+ .name = "Sample rates",
.offset = offsetof(AVFilterFormatsConfig, samplerates),
.merge = merge_samplerates,
.can_merge = can_merge_samplerates,
@@ -434,6 +440,7 @@ static const AVFilterFormatsMerger mergers_audio[] = {
CONVERSION_FILTER_ARESAMPLE
},
{
+ .name = "Sample formats",
.offset = offsetof(AVFilterFormatsConfig, formats),
.merge = merge_sample_fmts,
.can_merge = can_merge_sample_fmts,
diff --git a/libavfilter/formats.h b/libavfilter/formats.h
index 4fe96187ba..969bb230f1 100644
--- a/libavfilter/formats.h
+++ b/libavfilter/formats.h
@@ -516,6 +516,7 @@ int ff_formats_check_alpha_modes(void *log, const AVFilterFormats *fmts);
struct AVBPrint;
typedef struct AVFilterFormatMerger {
+ const char *name;
unsigned offset;
int (*merge)(void *a, void *b);
int (*can_merge)(const void *a, const void *b);
--
2.49.1
>From 7b564e2efc295ca205642c5c2ddfe5d8e77ecf08 Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Thu, 6 Nov 2025 18:01:37 +0100
Subject: [PATCH 4/5] avfilter/avfiltergraph: print all format lists on config
failure
Instead of just printing the pixel/sample formats.
---
libavfilter/avfiltergraph.c | 87 +++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 37 deletions(-)
diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index 4c80204f01..15978fc7a8 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -443,44 +443,33 @@ static int formats_declared(AVFilterContext *f)
return 1;
}
-static void print_formats(void *log_ctx, int level, enum AVMediaType type,
- const AVFilterFormats *formats)
-{
- AVBPrint bp;
- av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
-
- switch (type) {
- case AVMEDIA_TYPE_VIDEO:
- for (unsigned i = 0; i < formats->nb_formats; i++)
- av_bprintf(&bp, "%s%s", bp.len ? " " : "", av_get_pix_fmt_name(formats->formats[i]));
- break;
- case AVMEDIA_TYPE_AUDIO:
- for (unsigned i = 0; i < formats->nb_formats; i++)
- av_bprintf(&bp, "%s%s", bp.len ? " " : "", av_get_sample_fmt_name(formats->formats[i]));
- break;
- default:
- av_bprintf(&bp, "(unknown)");
- break;
- }
-
- if (av_bprint_is_complete(&bp)) {
- av_log(log_ctx, level, "%s\n", bp.str);
- } else {
- av_log(log_ctx, level, "(out of memory)\n");
- }
- av_bprint_finalize(&bp, NULL);
-}
-
static void print_link_formats(void *log_ctx, int level, const AVFilterLink *l)
{
if (av_log_get_level() < level)
return;
- av_log(log_ctx, level, "Link '%s.%s' -> '%s.%s':\n"
- " src: ", l->src->name, l->srcpad->name, l->dst->name, l->dstpad->name);
- print_formats(log_ctx, level, l->type, l->incfg.formats);
- av_log(log_ctx, level, " dst: ");
- print_formats(log_ctx, level, l->type, l->outcfg.formats);
+ AVBPrint bp;
+ av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
+
+ av_log(log_ctx, level, "Link '%s.%s' -> '%s.%s':\n",
+ l->src->name, l->srcpad->name, l->dst->name, l->dstpad->name);
+
+ const AVFilterNegotiation *neg = ff_filter_get_negotiation(l);
+ for (unsigned i = 0; i < neg->nb_mergers; i++) {
+ const AVFilterFormatsMerger *m = &neg->mergers[i];
+ av_log(log_ctx, level, " %s:\n", m->name);
+ m->print_list(&bp, FF_FIELD_AT(void *, m->offset, l->incfg));
+ if (av_bprint_is_complete(&bp))
+ av_log(log_ctx, level, " src: %s\n", bp.str);
+ av_bprint_clear(&bp);
+
+ m->print_list(&bp, FF_FIELD_AT(void *, m->offset, l->outcfg));
+ if (av_bprint_is_complete(&bp))
+ av_log(log_ctx, level, " dst: %s\n", bp.str);
+ av_bprint_clear(&bp);
+ }
+
+ av_bprint_finalize(&bp, NULL);
}
static void print_filter_formats(void *log_ctx, int level, const AVFilterContext *f)
@@ -488,15 +477,39 @@ static void print_filter_formats(void *log_ctx, int level, const AVFilterContext
if (av_log_get_level() < level)
return;
+ AVBPrint bp;
+ av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
+
av_log(log_ctx, level, "Filter '%s' formats:\n", f->name);
for (int i = 0; i < f->nb_inputs; i++) {
- av_log(log_ctx, level, " in[%d] '%s': ", i, f->input_pads[i].name);
- print_formats(log_ctx, level, f->inputs[i]->type, f->inputs[i]->outcfg.formats);
+ const AVFilterLink *in = f->inputs[i];
+ const AVFilterNegotiation *neg = ff_filter_get_negotiation(in);
+ av_log(log_ctx, level, " in[%d] '%s':", i, f->input_pads[i].name);
+
+ for (unsigned i = 0; i < neg->nb_mergers; i++) {
+ const AVFilterFormatsMerger *m = &neg->mergers[i];
+ m->print_list(&bp, FF_FIELD_AT(void *, m->offset, in->outcfg));
+ if (av_bprint_is_complete(&bp))
+ av_log(log_ctx, level, " %s: %s", m->name, bp.str);
+ av_bprint_clear(&bp);
+ }
}
+
for (int i = 0; i < f->nb_outputs; i++) {
- av_log(log_ctx, level, " out[%d] '%s': ", i, f->output_pads[i].name);
- print_formats(log_ctx, level, f->outputs[i]->type, f->outputs[i]->incfg.formats);
+ const AVFilterLink *out = f->outputs[i];
+ const AVFilterNegotiation *neg = ff_filter_get_negotiation(out);
+ av_log(log_ctx, level, " out[%d] '%s':", i, f->output_pads[i].name);
+
+ for (unsigned i = 0; i < neg->nb_mergers; i++) {
+ const AVFilterFormatsMerger *m = &neg->mergers[i];
+ m->print_list(&bp, FF_FIELD_AT(void *, m->offset, out->incfg));
+ if (av_bprint_is_complete(&bp))
+ av_log(log_ctx, level, " %s: %s", m->name, bp.str);
+ av_bprint_clear(&bp);
+ }
}
+
+ av_bprint_finalize(&bp, NULL);
}
/**
--
2.49.1
>From 6c3a63112be927526b08c0154b2df786fa7bce3d Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Thu, 6 Nov 2025 18:09:36 +0100
Subject: [PATCH 5/5] avfilter/avfiltergraph: only print format lists for
failing mergers
Instead of printing all format lists on a link negotiation error, just print
the relevant/failing format lists.
---
libavfilter/avfiltergraph.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index 15978fc7a8..285a701d09 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -443,7 +443,9 @@ static int formats_declared(AVFilterContext *f)
return 1;
}
-static void print_link_formats(void *log_ctx, int level, const AVFilterLink *l)
+static void print_link_formats(void *log_ctx, int level, const AVFilterLink *l,
+ const AVFilterFormatsMerger *mergers[],
+ int nb_mergers)
{
if (av_log_get_level() < level)
return;
@@ -454,9 +456,8 @@ static void print_link_formats(void *log_ctx, int level, const AVFilterLink *l)
av_log(log_ctx, level, "Link '%s.%s' -> '%s.%s':\n",
l->src->name, l->srcpad->name, l->dst->name, l->dstpad->name);
- const AVFilterNegotiation *neg = ff_filter_get_negotiation(l);
- for (unsigned i = 0; i < neg->nb_mergers; i++) {
- const AVFilterFormatsMerger *m = &neg->mergers[i];
+ for (unsigned i = 0; i < nb_mergers; i++) {
+ const AVFilterFormatsMerger *m = mergers[i];
av_log(log_ctx, level, " %s:\n", m->name);
m->print_list(&bp, FF_FIELD_AT(void *, m->offset, l->incfg));
if (av_bprint_is_complete(&bp))
@@ -554,8 +555,9 @@ retry:
AVFilterLink *link = filter->inputs[j];
const AVFilterNegotiation *neg;
AVFilterContext *conv[4];
+ const AVFilterFormatsMerger *mergers[4]; /* triggered mergers */
const char *conv_filters[4], *conv_opts[4] = {0};
- unsigned neg_step, num_conv = 0;
+ unsigned neg_step, num_conv = 0, num_mergers = 0;
if (!link)
continue;
@@ -578,6 +580,8 @@ retry:
conv_opts[num_conv] = FF_FIELD_AT(char *, m->conversion_opts_offset, *graph);
num_conv++;
}
+ av_assert1(num_mergers < FF_ARRAY_ELEMS(mergers));
+ mergers[num_mergers++] = m;
}
}
for (neg_step = 0; neg_step < neg->nb_mergers; neg_step++) {
@@ -594,6 +598,7 @@ retry:
if (ret < 0)
return ret;
if (!ret) {
+ mergers[num_mergers++] = m;
conv_filters[num_conv] = m->conversion_filter;
if (m->conversion_opts_offset)
conv_opts[num_conv] = FF_FIELD_AT(char *, m->conversion_opts_offset, *graph);
@@ -616,7 +621,7 @@ retry:
"The filters '%s' and '%s' do not have a common format "
"and automatic conversion is disabled.\n",
link->src->name, link->dst->name);
- print_link_formats(log_ctx, AV_LOG_ERROR, link);
+ print_link_formats(log_ctx, AV_LOG_ERROR, link, mergers, num_mergers);
return AVERROR(EINVAL);
}
@@ -624,7 +629,7 @@ retry:
av_log(log_ctx, AV_LOG_ERROR,
"'%s' filter not present, cannot convert formats.\n",
conv_filters[k]);
- print_link_formats(log_ctx, AV_LOG_ERROR, link);
+ print_link_formats(log_ctx, AV_LOG_ERROR, link, mergers, num_mergers);
return AVERROR(EINVAL);
}
snprintf(inst_name, sizeof(inst_name), "auto_%s_%d",
@@ -687,7 +692,7 @@ retry:
av_log(log_ctx, AV_LOG_ERROR,
"Impossible to convert between the formats supported by the filter "
"'%s' and the filter '%s'\n", link->src->name, link->dst->name);
- print_link_formats(log_ctx, AV_LOG_ERROR, link);
+ print_link_formats(log_ctx, AV_LOG_ERROR, link, &m, 1);
return AVERROR(ENOSYS);
} else {
count_merged += 2;
--
2.49.1
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2025-11-06 17:12 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-06 17:11 [FFmpeg-devel] [PATCH] Better printing of errors on format negotiation failure (PR #20851) Niklas Haas via ffmpeg-devel
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git