[FFmpeg-devel] [PATCH] avfilter/avfiltergraph: always retry format negotiation after auto-filters (PR #21100)

[FFmpeg-devel] [PATCH] avfilter/avfiltergraph: always retry format negotiation after auto-filters (PR #21100)

[Niklas Haas via ffmpeg-devel]

PR #21100 opened by Niklas Haas (haasn)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21100
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21100.patch

There is an edge case not covered by the current logic: If there is only
a single auto-filter inserted, but the auto-inserted filter is incompatible
with a *different* format attribute (after settling the previous formats),
we may need a second auto-filter (e.g. `scale`) to settle the newly introduced
incompatibility.

A regression test demonstrating the issue is added.


>From 60a3b0848e038d75a59dd942c547787c6e68869a Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Thu, 4 Dec 2025 13:25:03 +0100
Subject: [PATCH 1/2] avfilter/avfiltergraph: always retry format negotiation
 after auto-filters

There is an edge case not covered by the current logic: If there is only
a single auto-filter inserted, but the auto-inserted filter is incompatible
with a *different* format attribute (after settling the previous formats),
we may need a second auto-filter (e.g. `scale`) to settle the newly introduced
incompatibility.

A regression test demonstrating the issue is added.
---
 libavfilter/avfiltergraph.c             |  9 +++++----
 tests/fate/filter-video.mak             |  3 +++
 tests/ref/fate/filter-scale-premultiply | 15 +++++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 tests/ref/fate/filter-scale-premultiply

diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index d5c2ef54e6..c15d95b91e 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -701,10 +701,11 @@ retry:
                 }
             }
 
-            /* if there is more than one auto filter, we may need another round
-             * to fully settle formats due to possible cross-incompatibilities
-             * between the auto filters themselves */
-            if (num_conv > 1)
+            /* if there is an auto filter, we may need another round to fully
+             * settle formats due to possible cross-incompatibilities between
+             * the auto filters themselves, or between the auto filters and
+             * a different attribute of the filter they are modifying */
+            if (num_conv)
                 goto retry;
         }
     }
diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
index 9f3c92a395..35b2687b27 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -836,6 +836,9 @@ FATE_FILTER-$(call ALLYES, TESTSRC2_FILTER SPLIT_FILTER AVGBLUR_FILTER        \
                            METADATA_FILTER WRAPPED_AVFRAME_ENCODER NULL_MUXER \
                            PIPE_PROTOCOL) += $(FATE_FILTER_REFCMP_METADATA-yes)
 
+FATE_FILTER-$(call FILTERFRAMECRC, TESTSRC SCALE PREMULTIPLY, LAVFI_INDEV) += fate-filter-scale-premultiply
+fate-filter-scale-premultiply: CMD = framecrc -auto_conversion_filters -lavfi "testsrc,format=rgba,setparams=alpha_mode=premultiplied,format=rgba:alpha_modes=straight" -frames:v 10
+
 FATE_SAMPLES_FFPROBE += $(FATE_METADATA_FILTER-yes)
 FATE_SAMPLES_FFMPEG += $(FATE_FILTER_SAMPLES-yes)
 FATE_FFPROBE += $(FATE_FILTER_FFPROBE-yes)
diff --git a/tests/ref/fate/filter-scale-premultiply b/tests/ref/fate/filter-scale-premultiply
new file mode 100644
index 0000000000..13fd79310a
--- /dev/null
+++ b/tests/ref/fate/filter-scale-premultiply
@@ -0,0 +1,15 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: rawvideo
+#dimensions 0: 320x240
+#sar 0: 1/1
+0,          0,          0,        1,   307200, 0x73a3b71f
+0,          1,          1,        1,   307200, 0x3f05f047
+0,          2,          2,        1,   307200, 0x50382370
+0,          3,          3,        1,   307200, 0x41a15136
+0,          4,          4,        1,   307200, 0xbc8c78ee
+0,          5,          5,        1,   307200, 0x19839e1b
+0,          6,          6,        1,   307200, 0x8545b93b
+0,          7,          7,        1,   307200, 0x4a4acf07
+0,          8,          8,        1,   307200, 0xefa1dee4
+0,          9,          9,        1,   307200, 0x22dae9ca
-- 
2.49.1


>From 73f67a23d292cf4018114dc820879d7e640d29c4 Mon Sep 17 00:00:00 2001
From: Niklas Haas <git@haasn.dev>
Date: Thu, 4 Dec 2025 13:10:47 +0100
Subject: [PATCH 2/2] avfilter/avfiltergraph: add missing newlines to format
 printing

---
 libavfilter/avfiltergraph.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
index c15d95b91e..ad81d91bbc 100644
--- a/libavfilter/avfiltergraph.c
+++ b/libavfilter/avfiltergraph.c
@@ -485,13 +485,13 @@ static void print_filter_formats(void *log_ctx, int level, const AVFilterContext
     for (int i = 0; i < f->nb_inputs; i++) {
         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);
+        av_log(log_ctx, level, "  in[%d] '%s':\n", 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_log(log_ctx, level, "    %s: %s\n", m->name, bp.str);
             av_bprint_clear(&bp);
         }
     }
@@ -499,13 +499,13 @@ static void print_filter_formats(void *log_ctx, int level, const AVFilterContext
     for (int i = 0; i < f->nb_outputs; i++) {
         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);
+        av_log(log_ctx, level, "  out[%d] '%s':\n", 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_log(log_ctx, level, "    %s: %s\n", m->name, bp.str);
             av_bprint_clear(&bp);
         }
     }
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avfilter/avfiltergraph: always retry format negotiation after auto-filters (PR #21100)

[Nicolas George via ffmpeg-devel]

Niklas Haas via ffmpeg-devel (HE12025-12-04):
> PR #21100 opened by Niklas Haas (haasn)
> URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21100
> Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/21100.patch
> 
> There is an edge case not covered by the current logic: If there is only
> a single auto-filter inserted, but the auto-inserted filter is incompatible
> with a *different* format attribute (after settling the previous formats),
> we may need a second auto-filter (e.g. `scale`) to settle the newly introduced
> incompatibility.
> 
> A regression test demonstrating the issue is added.

Sorry, it has already been five days, I need to find time to look at
that.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avfilter/avfiltergraph: always retry format negotiation after auto-filters (PR #21100)

[Nicolas George via ffmpeg-devel]

Nicolas George via ffmpeg-devel (HE12025-12-09):
> Sorry, it has already been five days, I need to find time to look at
> that.

It has only been five days.

YOU NEED TO STOP DOING THAT.

This is not how high-quality is maintained. High quality is maintained
by letting people with knowledge of the code share their insight. And
that includes waiting.

-- 
   Nicolas George
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

[FFmpeg-devel] Re: [PATCH] avfilter/avfiltergraph: always retry format negotiation after auto-filters (PR #21100)

[Nicolas George via ffmpeg-devel]

Niklas Haas via ffmpeg-devel (HE12025-12-04):
> >From 60a3b0848e038d75a59dd942c547787c6e68869a Mon Sep 17 00:00:00 2001
> From: Niklas Haas <git@haasn.dev>
> Date: Thu, 4 Dec 2025 13:25:03 +0100
> Subject: [PATCH 1/2] avfilter/avfiltergraph: always retry format negotiation
>  after auto-filters
> 
> There is an edge case not covered by the current logic: If there is only
> a single auto-filter inserted, but the auto-inserted filter is incompatible
> with a *different* format attribute (after settling the previous formats),
> we may need a second auto-filter (e.g. `scale`) to settle the newly introduced
> incompatibility.

I have strong doubts about this and the code it is trying to fix.

And since it was pushed too soon without review, I reserve the right to
revert, but let us discuss first.

The original negotiation code is designed for one single conversion
filter that does everything: if it is possible to convert at all, then
inserting the right conversion filter for the media type (scale for
video, aresample for audio) would be able to make the conversion.

This is not theoretical: unlike video, audio had multiple dimensions in
the negotiation from the start, and over time multiple subtle
adjustments had to be made to avoid doing multiple conversions due to
apparent incompatibilities.

And it is not a purely technical choice, it is also a choice for user
experience: if they want to control finely where the conversion happens,
the do not have to also worry about what conversion happens. They can,
but they do not have to.

But then you added the alpha mode negotiation, and you broke that.

So yes, this is not a proper fix.

The proper fix would be to make sure scale can convert between straight
and premultiplied alpha.

Please fix your fix.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org