[FFmpeg-devel] Re: [PATCH 1/3] Revert "avfilter/avfiltergraph: always retry format negotiation after auto-filters"

[Niklas Haas via ffmpeg-devel]

On Sat, Apr 25, 2026, at 7:59 PM, Nicolas George via ffmpeg-devel wrote:
> This reverts commit 978a0821eeb909d7b2cb777bed03a8fc380e9503.
>
> The stability of the negotiation process has never been studied
> with multiple rounds.
>
> It has always been documented to users that scale was the filter
> to insert to manage format conversions. A second filter for
> a specific type of conversion should never have been added:
> the proper fix for the issue that this commit tried to fix is
> to give scale the ability to handle premultiplication too.
>
> Signed-off-by: Nicolas George <geo...@nsup.org>
> ---
>  libavfilter/avfiltergraph.c             |  9 ++++-----
>  tests/fate/filter-video.mak             |  3 ---
>  tests/ref/fate/filter-scale-premultiply | 15 ---------------
>  3 files changed, 4 insertions(+), 23 deletions(-)
>  delete mode 100644 tests/ref/fate/filter-scale-premultiply
>
>
> Out of courtesy, I refrain from writing in the commit message that this
> patch was pushed despite my objections and after only a review by
> somebody who does not know the code.
>
>
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index ad81d91bbc..f1abcc80b4 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -701,11 +701,10 @@ retry:
>                  }
>              }
> 
> -            /* 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)
> +            /* 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)
>                  goto retry;
>          }
>      }
> diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
> index 5016ea9d4d..d11de0ce86 100644
> --- a/tests/fate/filter-video.mak
> +++ b/tests/fate/filter-video.mak
> @@ -849,9 +849,6 @@ 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
> deleted file mode 100644
> index 13fd79310a..0000000000
> --- a/tests/ref/fate/filter-scale-premultiply
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -#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

This change breaks the FATE test in question. It was added for a reason. If you 
want to implement a "proper fix", as you say, then please do so *before* 
reverting this for no reason and reintroducing broken behavior.

You also do not explain what bug this revert is solving.

> -- 
> 2.53.0
>
> _______________________________________________
> ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
> To unsubscribe send an email to ffmpeg-devel-le...@ffmpeg.org
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-le...@ffmpeg.org