diff mbox series

[FFmpeg-devel] fate: Convert the musepack8 test to an oneoff test

Message ID 20201111101953.16366-1-martin@martin.st
State Accepted
Headers show
Series [FFmpeg-devel] fate: Convert the musepack8 test to an oneoff test
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate fail Make fate failed
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate warning Make fate failed

Commit Message

Martin Storsjö Nov. 11, 2020, 10:19 a.m. UTC
This fixes tests if built for x86 with x87 FPU.
---
This requires someone to upload a reference file.
---
 tests/fate/mpc.mak | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Nov. 12, 2020, 4:49 a.m. UTC | #1
Martin Storsjö:
> This fixes tests if built for x86 with x87 FPU.
> ---
> This requires someone to upload a reference file.
> ---
>  tests/fate/mpc.mak | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/fate/mpc.mak b/tests/fate/mpc.mak
> index bb1c03d250..cde6e55177 100644
> --- a/tests/fate/mpc.mak
> +++ b/tests/fate/mpc.mak
> @@ -12,7 +12,9 @@ fate-musepack7: REF = $(SAMPLES)/musepack/inside-mp7.pcm
>  FATE_MPC-$(call ALLYES, FILE_PROTOCOL MPC8_DEMUXER MPC8_DECODER  \
>                          ARESAMPLE_FILTER PCM_S16LE_ENCODER  \
>                          FRAMECRC_MUXER PIPE_PROTOCOL) += fate-musepack8
> -fate-musepack8: CMD = framecrc -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample -c:a pcm_s16le
> +fate-musepack8: CMD = pcm -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample
> +fate-musepack8: CMP = oneoff
> +fate-musepack8: REF = $(SAMPLES)/musepack/inside-mp8.pcm
>  
>  FATE_SAMPLES_AVCONV += $(FATE_MPC-yes)
>  fate-mpc: $(FATE_MPC-yes)
> 
To quote myself:
"The test uses the framecrc output, because Musepack SV8 is an encoder
that returns multiple frames for a single packet, so that timing
information in the test output is valueable. Output seeking has been
used in order to limit the size of the ref file as well as to test this
codepath for the first time."
The timing information would be lost with your patch; output seeking
would only be implicitly tested (by looking at the first sample that is
retained). And of course your ref file would be way bigger. So is there
no better way?

- Andreas
Martin Storsjö Nov. 12, 2020, 7:27 a.m. UTC | #2
On Thu, 12 Nov 2020, Andreas Rheinhardt wrote:

> Martin Storsjö:
>> This fixes tests if built for x86 with x87 FPU.
>> ---
>> This requires someone to upload a reference file.
>> ---
>>  tests/fate/mpc.mak | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/fate/mpc.mak b/tests/fate/mpc.mak
>> index bb1c03d250..cde6e55177 100644
>> --- a/tests/fate/mpc.mak
>> +++ b/tests/fate/mpc.mak
>> @@ -12,7 +12,9 @@ fate-musepack7: REF = $(SAMPLES)/musepack/inside-mp7.pcm
>>  FATE_MPC-$(call ALLYES, FILE_PROTOCOL MPC8_DEMUXER MPC8_DECODER  \
>>                          ARESAMPLE_FILTER PCM_S16LE_ENCODER  \
>>                          FRAMECRC_MUXER PIPE_PROTOCOL) += fate-musepack8
>> -fate-musepack8: CMD = framecrc -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample -c:a pcm_s16le
>> +fate-musepack8: CMD = pcm -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample
>> +fate-musepack8: CMP = oneoff
>> +fate-musepack8: REF = $(SAMPLES)/musepack/inside-mp8.pcm
>>
>>  FATE_SAMPLES_AVCONV += $(FATE_MPC-yes)
>>  fate-mpc: $(FATE_MPC-yes)
>> 
> To quote myself:
> "The test uses the framecrc output, because Musepack SV8 is an encoder
> that returns multiple frames for a single packet, so that timing
> information in the test output is valueable. Output seeking has been
> used in order to limit the size of the ref file as well as to test this
> codepath for the first time."
> The timing information would be lost with your patch; output seeking
> would only be implicitly tested (by looking at the first sample that is
> retained). And of course your ref file would be way bigger. So is there
> no better way?

Well one way would be to convert the floating point bits of the decoder to 
fixed point, or create some combination of pcm/oneoff + framecrc for 
timing info but without the actual crcs. Or build something like framecrc 
with actual output data instead of hashes, with a matching oneoff test 
tool to compare the outputs...

But as things are right now, any crc tests for anything involving floats 
(at least, anything that does more than one single trivial arithmetic 
operation on floats before storing them) are bound to break.

// Martin
Martin Storsjö Nov. 17, 2020, 9:56 p.m. UTC | #3
On Thu, 12 Nov 2020, Martin Storsjö wrote:

> On Thu, 12 Nov 2020, Andreas Rheinhardt wrote:
>
>> Martin Storsjö:
>>> This fixes tests if built for x86 with x87 FPU.
>>> ---
>>> This requires someone to upload a reference file.
>>> ---
>>>  tests/fate/mpc.mak | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/tests/fate/mpc.mak b/tests/fate/mpc.mak
>>> index bb1c03d250..cde6e55177 100644
>>> --- a/tests/fate/mpc.mak
>>> +++ b/tests/fate/mpc.mak
>>> @@ -12,7 +12,9 @@ fate-musepack7: REF = $(SAMPLES)/musepack/inside-mp7.pcm
>>>  FATE_MPC-$(call ALLYES, FILE_PROTOCOL MPC8_DEMUXER MPC8_DECODER  \
>>>                          ARESAMPLE_FILTER PCM_S16LE_ENCODER  \
>>>                          FRAMECRC_MUXER PIPE_PROTOCOL) += fate-musepack8
>>> -fate-musepack8: CMD = framecrc -i 
>>> $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample -c:a 
>>> pcm_s16le
>>> +fate-musepack8: CMD = pcm -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc 
>>> -ss 8.4 -af aresample
>>> +fate-musepack8: CMP = oneoff
>>> +fate-musepack8: REF = $(SAMPLES)/musepack/inside-mp8.pcm
>>>
>>>  FATE_SAMPLES_AVCONV += $(FATE_MPC-yes)
>>>  fate-mpc: $(FATE_MPC-yes)
>>> 
>> To quote myself:
>> "The test uses the framecrc output, because Musepack SV8 is an encoder
>> that returns multiple frames for a single packet, so that timing
>> information in the test output is valueable. Output seeking has been
>> used in order to limit the size of the ref file as well as to test this
>> codepath for the first time."
>> The timing information would be lost with your patch; output seeking
>> would only be implicitly tested (by looking at the first sample that is
>> retained). And of course your ref file would be way bigger. So is there
>> no better way?
>
> Well one way would be to convert the floating point bits of the decoder to 
> fixed point, or create some combination of pcm/oneoff + framecrc for timing 
> info but without the actual crcs. Or build something like framecrc with 
> actual output data instead of hashes, with a matching oneoff test tool to 
> compare the outputs...
>
> But as things are right now, any crc tests for anything involving floats (at 
> least, anything that does more than one single trivial arithmetic operation 
> on floats before storing them) are bound to break.

Concluded on irc that this probably is the only sensible option, and a 
sample has been uploaded a couple hours ago, thus pushing this one.

// Martin
diff mbox series

Patch

diff --git a/tests/fate/mpc.mak b/tests/fate/mpc.mak
index bb1c03d250..cde6e55177 100644
--- a/tests/fate/mpc.mak
+++ b/tests/fate/mpc.mak
@@ -12,7 +12,9 @@  fate-musepack7: REF = $(SAMPLES)/musepack/inside-mp7.pcm
 FATE_MPC-$(call ALLYES, FILE_PROTOCOL MPC8_DEMUXER MPC8_DECODER  \
                         ARESAMPLE_FILTER PCM_S16LE_ENCODER  \
                         FRAMECRC_MUXER PIPE_PROTOCOL) += fate-musepack8
-fate-musepack8: CMD = framecrc -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample -c:a pcm_s16le
+fate-musepack8: CMD = pcm -i $(TARGET_SAMPLES)/musepack/inside-mp8.mpc -ss 8.4 -af aresample
+fate-musepack8: CMP = oneoff
+fate-musepack8: REF = $(SAMPLES)/musepack/inside-mp8.pcm
 
 FATE_SAMPLES_AVCONV += $(FATE_MPC-yes)
 fate-mpc: $(FATE_MPC-yes)