[FFmpeg-devel] [PATCH 2/2] fate: disable automatic conversions on most filter tests.

[Mark Thompson]

On 14/08/2020 18:14, Nicolas George wrote:
> Explicitly insert the scale or aresample filter where it would
> have been inserted by the negotiation.
> 
> If a conversion is needed in a test, we want to know about it.
> If the negotiation changes and makes new conversion necessary,
> we want to know about it even more.
> 
> Signed-off-by: Nicolas George <george at nsup.org>
> ---
>   tests/fate-run.sh           |   4 +-
>   tests/fate/filter-audio.mak | 112 ++++++------
>   tests/fate/filter-video.mak | 350 ++++++++++++++++++------------------
>   3 files changed, 235 insertions(+), 231 deletions(-)

I like it, this is a good idea.

> diff --git a/tests/fate-run.sh b/tests/fate-run.sh
> index 414ac6f825..7456216e9c 100755
> --- a/tests/fate-run.sh
> +++ b/tests/fate-run.sh
> @@ -388,7 +388,7 @@ video_filter(){
>       label=${test#filter-}
>       raw_src="${target_path}/tests/vsynth1/%02d.pgm"
>       printf '%-20s' $label
> -    ffmpeg $DEC_OPTS -f image2 -vcodec pgmyuv -i $raw_src \
> +    ffmpeg $DEC_OPTS -disable_all_auto_conversion_filters -f image2 -vcodec pgmyuv -i $raw_src \
>           $FLAGS $ENC_OPTS -vf "$filters" -vcodec rawvideo -frames:v 5 $* -f nut md5:
>   }
>   
> @@ -416,7 +416,7 @@ pixfmts(){
>       outertest=$test
>       for pix_fmt in $pix_fmts; do
>           test=$pix_fmt
> -        video_filter "${prefilter_chain}format=$pix_fmt,$filter=$filter_args" -pix_fmt $pix_fmt -frames:v $nframes
> +        video_filter "${prefilter_chain}scale,format=$pix_fmt,$filter=$filter_args" -pix_fmt $pix_fmt -frames:v $nframes
>       done
>   
>       rm $in_fmts $scale_in_fmts $scale_out_fmts $scale_exclude_fmts
> diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak
> index 79b1536df0..5dfafcccad 100644
> --- a/tests/fate/filter-audio.mak
> +++ b/tests/fate/filter-audio.mak
> @@ -1,54 +1,56 @@
> +NOCONV=-disable_all_auto_conversion_filters

Adding this variable to the command line of large numbers of tests is not very fun, because it's not relevant to the test definition itself.

Is it possible that it might be better to split some of the commands into multiple forms if there are some tests which still want autoconversion?  Depending on which variant is more common, "framecrc_autoconv" or "framecrc_noconv".

(Alternatively: how many tests /do/ want autoconversion?  The option could be added in the top-level ffmpeg() function and then disabled selectively in tests.)

> +
>   FATE_AFILTER-$(call FILTERDEMDECENCMUX, ADELAY, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-adelay
>   fate-filter-adelay: tests/data/asynth-44100-2.wav
>   fate-filter-adelay: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
> -fate-filter-adelay: CMD = framecrc -i $(SRC) -af adelay=42
> +fate-filter-adelay: CMD = framecrc $(NOCONV) -i $(SRC) -af aresample,adelay=42,aresample
>   
>   FATE_AFILTER-$(call FILTERDEMDECENCMUX, AECHO, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-aecho
>   fate-filter-aecho: tests/data/asynth-44100-2.wav
>   fate-filter-aecho: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
> -fate-filter-aecho: CMD = framecrc -i $(SRC) -af aecho=0.5:0.5:32:0.5
> +fate-filter-aecho: CMD = framecrc $(NOCONV) -i $(SRC) -af aresample,aecho=0.5:0.5:32:0.5,aresample
>   
>   FATE_FILTER_AEMPHASIS += fate-filter-aemphasis-50fm
>   fate-filter-aemphasis-50fm: tests/data/asynth-44100-2.wav
>   fate-filter-aemphasis-50fm: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
> -fate-filter-aemphasis-50fm: CMD = framecrc -i $(SRC) -af aemphasis=1:5:reproduction:50fm
> +fate-filter-aemphasis-50fm: CMD = framecrc $(NOCONV) -i $(SRC) -af aresample,aemphasis=1:5:reproduction:50fm,aresample
>   
>   FATE_FILTER_AEMPHASIS += fate-filter-aemphasis-75kf
>   fate-filter-aemphasis-75kf: tests/data/asynth-44100-2.wav
>   fate-filter-aemphasis-75kf: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
> -fate-filter-aemphasis-75kf: CMD = framecrc -i $(SRC) -af aemphasis=2:8:reproduction:75kf
> +fate-filter-aemphasis-75kf: CMD = framecrc $(NOCONV) -i $(SRC) -af aresample,aemphasis=2:8:reproduction:75kf,aresample

(Not something to change in this patch, but the way that some of these tests are resampling on both input and output is rather suggesting that they could be written more nicely.)

> ...

Thanks,

- Mark