diff mbox

[FFmpeg-devel,PATCHv2] fate: Use a oneoff test for the tremolo filter

Message ID 20191211074312.26817-1-martin@martin.st
State New
Headers show

Commit Message

Martin Storsjö Dec. 11, 2019, 7:43 a.m. UTC 
The tremolo filter uses floating point internally, and uses
multiplication factors derived from sin(fmod()), neither of
which is bitexact for use with framecrc.

This fixes running this test with mingw/x86_32 binaries when run
in wine on linux (unsure if the same issue is present when run
on actual windows).

In this case, a 1 ulp difference in the output from fmod() would
end up in an output from the filter that differs by 1 ulp, but
which makes the lrint() in swresample/audioconvert.c round in a
different direction.

---
Updated with the existing reference file removed.
---
 tests/fate/filter-audio.mak   |  5 ++++-
 tests/ref/fate/filter-tremolo | 26 --------------------------
 2 files changed, 4 insertions(+), 27 deletions(-)
 delete mode 100644 tests/ref/fate/filter-tremolo

Comments

James Almer Dec. 11, 2019, 2:21 p.m. UTC | #1 
On 12/11/2019 4:43 AM, Martin Storsjö wrote:
> The tremolo filter uses floating point internally, and uses
> multiplication factors derived from sin(fmod()), neither of
> which is bitexact for use with framecrc.
> 
> This fixes running this test with mingw/x86_32 binaries when run
> in wine on linux (unsure if the same issue is present when run
> on actual windows).
> 
> In this case, a 1 ulp difference in the output from fmod() would
> end up in an output from the filter that differs by 1 ulp, but
> which makes the lrint() in swresample/audioconvert.c round in a
> different direction.
> 
> ---
> Updated with the existing reference file removed.
> ---
>  tests/fate/filter-audio.mak   |  5 ++++-
>  tests/ref/fate/filter-tremolo | 26 --------------------------
>  2 files changed, 4 insertions(+), 27 deletions(-)
>  delete mode 100644 tests/ref/fate/filter-tremolo
> 
> diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak
> index fed2644ccf..b1dcb9787a 100644
> --- a/tests/fate/filter-audio.mak
> +++ b/tests/fate/filter-audio.mak
> @@ -189,7 +189,10 @@ fate-filter-stereotools: CMD = framecrc -i $(SRC) -frames:a 20 -af stereotools=m
>  FATE_AFILTER-$(call FILTERDEMDECENCMUX, TREMOLO, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-tremolo

Needs to be FATE_AFILTER_SAMPLES now that it's using a reference sample
rather than only the autogenerated asynth-44100-2.wav.

>  fate-filter-tremolo: tests/data/asynth-44100-2.wav
>  fate-filter-tremolo: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
> -fate-filter-tremolo: CMD = framecrc -i $(SRC) -frames:a 20 -af tremolo
> +fate-filter-tremolo: CMD = ffmpeg -i $(SRC) -frames:a 20 -af tremolo -f wav -f s16le -
> +fate-filter-tremolo: REF = $(SAMPLES)/filter/tremolo.pcm

Created and uploaded it, so feel free to push this patch after you fix
the above and confirm the ref sample is ok.

> +fate-filter-tremolo: CMP = oneoff
> +fate-filter-tremolo: CMP_UNIT = s16
>  
>  FATE_AFILTER-$(call FILTERDEMDECENCMUX, COMPAND, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-compand
>  fate-filter-compand: tests/data/asynth-44100-2.wav
> diff --git a/tests/ref/fate/filter-tremolo b/tests/ref/fate/filter-tremolo
> deleted file mode 100644
> index c6cff52c0e..0000000000
> --- a/tests/ref/fate/filter-tremolo
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -#tb 0: 1/44100
> -#media_type 0: audio
> -#codec_id 0: pcm_s16le
> -#sample_rate 0: 44100
> -#channel_layout 0: 3
> -#channel_layout_name 0: stereo
> -0,          0,          0,     1024,     4096, 0x5d3be907
> -0,       1024,       1024,     1024,     4096, 0xea151fbe
> -0,       2048,       2048,     1024,     4096, 0xa5bc19f4
> -0,       3072,       3072,     1024,     4096, 0x8706ec6d
> -0,       4096,       4096,     1024,     4096, 0x334ff275
> -0,       5120,       5120,     1024,     4096, 0xcd0ff7ad
> -0,       6144,       6144,     1024,     4096, 0x29a1e9c9
> -0,       7168,       7168,     1024,     4096, 0x1d41e77f
> -0,       8192,       8192,     1024,     4096, 0x99e7fe07
> -0,       9216,       9216,     1024,     4096, 0x4bbf09ce
> -0,      10240,      10240,     1024,     4096, 0x94600236
> -0,      11264,      11264,     1024,     4096, 0xc8af0c9e
> -0,      12288,      12288,     1024,     4096, 0x70eef88f
> -0,      13312,      13312,     1024,     4096, 0xb222ec47
> -0,      14336,      14336,     1024,     4096, 0x1071ee27
> -0,      15360,      15360,     1024,     4096, 0x7c390bd2
> -0,      16384,      16384,     1024,     4096, 0x68bdf655
> -0,      17408,      17408,     1024,     4096, 0x810cfacb
> -0,      18432,      18432,     1024,     4096, 0x9639e41f
> -0,      19456,      19456,     1024,     4096, 0xa30be70f
>
Martin Storsjö Dec. 11, 2019, 2:25 p.m. UTC | #2 
On Wed, 11 Dec 2019, James Almer wrote:

> On 12/11/2019 4:43 AM, Martin Storsjö wrote:
>> The tremolo filter uses floating point internally, and uses
>> multiplication factors derived from sin(fmod()), neither of
>> which is bitexact for use with framecrc.
>> 
>> This fixes running this test with mingw/x86_32 binaries when run
>> in wine on linux (unsure if the same issue is present when run
>> on actual windows).
>> 
>> In this case, a 1 ulp difference in the output from fmod() would
>> end up in an output from the filter that differs by 1 ulp, but
>> which makes the lrint() in swresample/audioconvert.c round in a
>> different direction.
>> 
>> ---
>> Updated with the existing reference file removed.
>> ---
>>  tests/fate/filter-audio.mak   |  5 ++++-
>>  tests/ref/fate/filter-tremolo | 26 --------------------------
>>  2 files changed, 4 insertions(+), 27 deletions(-)
>>  delete mode 100644 tests/ref/fate/filter-tremolo
>> 
>> diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak
>> index fed2644ccf..b1dcb9787a 100644
>> --- a/tests/fate/filter-audio.mak
>> +++ b/tests/fate/filter-audio.mak
>> @@ -189,7 +189,10 @@ fate-filter-stereotools: CMD = framecrc -i $(SRC) -frames:a 20 -af stereotools=m
>>  FATE_AFILTER-$(call FILTERDEMDECENCMUX, TREMOLO, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-tremolo
>
> Needs to be FATE_AFILTER_SAMPLES now that it's using a reference sample
> rather than only the autogenerated asynth-44100-2.wav.

Ah, good catch - amended locally

>>  fate-filter-tremolo: tests/data/asynth-44100-2.wav
>>  fate-filter-tremolo: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
>> -fate-filter-tremolo: CMD = framecrc -i $(SRC) -frames:a 20 -af tremolo
>> +fate-filter-tremolo: CMD = ffmpeg -i $(SRC) -frames:a 20 -af tremolo -f wav -f s16le -
>> +fate-filter-tremolo: REF = $(SAMPLES)/filter/tremolo.pcm
>
> Created and uploaded it, so feel free to push this patch after you fix
> the above and confirm the ref sample is ok.

Thanks! Seems to work fine for me, so I'll push this one a bit later then.

// Martin
diff mbox

Patch

diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak
index fed2644ccf..b1dcb9787a 100644
--- a/tests/fate/filter-audio.mak
+++ b/tests/fate/filter-audio.mak
@@ -189,7 +189,10 @@  fate-filter-stereotools: CMD = framecrc -i $(SRC) -frames:a 20 -af stereotools=m
 FATE_AFILTER-$(call FILTERDEMDECENCMUX, TREMOLO, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-tremolo
 fate-filter-tremolo: tests/data/asynth-44100-2.wav
 fate-filter-tremolo: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
-fate-filter-tremolo: CMD = framecrc -i $(SRC) -frames:a 20 -af tremolo
+fate-filter-tremolo: CMD = ffmpeg -i $(SRC) -frames:a 20 -af tremolo -f wav -f s16le -
+fate-filter-tremolo: REF = $(SAMPLES)/filter/tremolo.pcm
+fate-filter-tremolo: CMP = oneoff
+fate-filter-tremolo: CMP_UNIT = s16
 
 FATE_AFILTER-$(call FILTERDEMDECENCMUX, COMPAND, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-compand
 fate-filter-compand: tests/data/asynth-44100-2.wav
diff --git a/tests/ref/fate/filter-tremolo b/tests/ref/fate/filter-tremolo
deleted file mode 100644
index c6cff52c0e..0000000000
--- a/tests/ref/fate/filter-tremolo
+++ /dev/null
@@ -1,26 +0,0 @@ 
-#tb 0: 1/44100
-#media_type 0: audio
-#codec_id 0: pcm_s16le
-#sample_rate 0: 44100
-#channel_layout 0: 3
-#channel_layout_name 0: stereo
-0,          0,          0,     1024,     4096, 0x5d3be907
-0,       1024,       1024,     1024,     4096, 0xea151fbe
-0,       2048,       2048,     1024,     4096, 0xa5bc19f4
-0,       3072,       3072,     1024,     4096, 0x8706ec6d
-0,       4096,       4096,     1024,     4096, 0x334ff275
-0,       5120,       5120,     1024,     4096, 0xcd0ff7ad
-0,       6144,       6144,     1024,     4096, 0x29a1e9c9
-0,       7168,       7168,     1024,     4096, 0x1d41e77f
-0,       8192,       8192,     1024,     4096, 0x99e7fe07
-0,       9216,       9216,     1024,     4096, 0x4bbf09ce
-0,      10240,      10240,     1024,     4096, 0x94600236
-0,      11264,      11264,     1024,     4096, 0xc8af0c9e
-0,      12288,      12288,     1024,     4096, 0x70eef88f
-0,      13312,      13312,     1024,     4096, 0xb222ec47
-0,      14336,      14336,     1024,     4096, 0x1071ee27
-0,      15360,      15360,     1024,     4096, 0x7c390bd2
-0,      16384,      16384,     1024,     4096, 0x68bdf655
-0,      17408,      17408,     1024,     4096, 0x810cfacb
-0,      18432,      18432,     1024,     4096, 0x9639e41f
-0,      19456,      19456,     1024,     4096, 0xa30be70f