diff mbox

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

Message ID 20191210201101.22386-1-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö Dec. 10, 2019, 8:11 p.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.

---
This change requires uploading a new reference file.
---
 tests/fate/filter-audio.mak | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

James Almer Dec. 10, 2019, 11:06 p.m. UTC | #1
On 12/10/2019 5:11 PM, 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.
> 
> ---
> This change requires uploading a new reference file.

I can create and upload it, if you want. Will use an x86_32 mingw-w64
build with cpuflags 0 to do it.

> ---
>  tests/fate/filter-audio.mak | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> 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

Missing removing tests/ref/fate/filter-tremolo.
Martin Storsjö Dec. 11, 2019, 7:40 a.m. UTC | #2
On Tue, 10 Dec 2019, James Almer wrote:

> On 12/10/2019 5:11 PM, 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.
>> 
>> ---
>> This change requires uploading a new reference file.
>
> I can create and upload it, if you want. Will use an x86_32 mingw-w64
> build with cpuflags 0 to do it.

Thanks, that'd be appreciated. I presume the existing test pass for you, 
so in that case the generated reference will be close to what everyone 
else normally produce.

>> ---
>>  tests/fate/filter-audio.mak | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> 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
>
> Missing removing tests/ref/fate/filter-tremolo.

Oops, thanks for catching it, will resend a new patch.

// 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