[FFmpeg-devel] [PATCH 1/2] fate: add more DCA tests

[James Almer]

On 7/31/2017 8:58 PM, foo86 wrote:
> On Mon, Jul 31, 2017 at 06:50:44PM -0300, James Almer wrote:
>> On 7/31/2017 5:33 PM, foo86 wrote:
>>> ---
>>>  tests/fate/dca.mak           |  36 ++
>>>  tests/ref/fate/dca-core-14be |   1 +
>>>  tests/ref/fate/dca-core-14le |   1 +
>>>  tests/ref/fate/dca-core-16be |   1 +
>>>  tests/ref/fate/dca-core-16le |   1 +
>>>  tests/ref/fate/dca-core-dmix |   1 +
>>>  tests/ref/fate/dca-core-size |   1 +
>>>  tests/ref/fate/dca-core-x96  |   1 +
>>>  tests/ref/fate/dca-core-xch  |   1 +
>>>  tests/ref/fate/dca-core-xxch |   1 +
>>>  tests/ref/fate/dca-xll-ltrt  |   1 +
>>>  tests/ref/fate/dca-xll-pbr   | 836 +++++++++++++++++++++++++++++++++++++++++++
>>>  tests/ref/fate/dca-xll-read  | 510 ++++++++++++++++++++++++++
>>>  tests/ref/fate/dca-xll-sync  | 810 +++++++++++++++++++++++++++++++++++++++++
>>>  14 files changed, 2202 insertions(+)
>>>  create mode 100644 tests/ref/fate/dca-core-14be
>>>  create mode 100644 tests/ref/fate/dca-core-14le
>>>  create mode 100644 tests/ref/fate/dca-core-16be
>>>  create mode 100644 tests/ref/fate/dca-core-16le
>>>  create mode 100644 tests/ref/fate/dca-core-dmix
>>>  create mode 100644 tests/ref/fate/dca-core-size
>>>  create mode 100644 tests/ref/fate/dca-core-x96
>>>  create mode 100644 tests/ref/fate/dca-core-xch
>>>  create mode 100644 tests/ref/fate/dca-core-xxch
>>>  create mode 100644 tests/ref/fate/dca-xll-ltrt
>>>  create mode 100644 tests/ref/fate/dca-xll-pbr
>>>  create mode 100644 tests/ref/fate/dca-xll-read
>>>  create mode 100644 tests/ref/fate/dca-xll-sync
>>>
>>> diff --git a/tests/fate/dca.mak b/tests/fate/dca.mak
>>> index b1681c6b59..563b83695f 100644
>>> --- a/tests/fate/dca.mak
>>> +++ b/tests/fate/dca.mak
>>> @@ -67,9 +67,45 @@ fate-dca-core: CMD = pcm -i $(TARGET_SAMPLES)/dts/dts.ts
>>>  fate-dca-core: CMP = oneoff
>>>  fate-dca-core: REF = $(SAMPLES)/dts/dts.pcm
>>>  
>>> +# lossy fixed point tests
>>> +DCA_FIXED_POINT = dca-core-14be dca-core-14le dca-core-16be dca-core-16le dca-core-size dca-core-x96 dca-core-xch dca-core-xxch
>>> +
>>> +define FATE_DCA_FIXED_POINT
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-$(1)
>>> +fate-$(1): CMD = md5 -flags +bitexact -i $(TARGET_SAMPLES)/dts/$(1) -f s24le
>>> +endef
>>> +
>>> +$(foreach N,$(DCA_FIXED_POINT),$(eval $(call FATE_DCA_FIXED_POINT,$(N))))
>>> +
>>> +# lossy fixed point downmix test
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-core-dmix
>>> +fate-dca-core-dmix: CMD = md5 -flags +bitexact -request_channel_layout 0x3 -i $(TARGET_SAMPLES)/dts/dca-core-xxch -f s24le
>>> +
>>>  FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-xll
>>>  fate-dca-xll: CMD = md5 -i $(TARGET_SAMPLES)/dts/master_audio_7.1_24bit.dts -f s24le
>>>  
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-xll-ltrt
>>> +fate-dca-xll-ltrt: CMD = md5 -err_detect crccheck -i $(TARGET_SAMPLES)/dts/dca-xll-ltrt -f s16le
>>
>> Any reason why these are using md5 instead of framemd5?
> 
> To save space, because I didn't expect individual frame checksums to be
> interesting in these tests.

It's not so much about each frame's checksum in most cases as it's about
the extra printed info. It makes it easier to detect if something
changed in how the stream is detected that might not affect checksums
(time base, pts/dts, etc). But i guess that's already covered in every
other xll test.

For that matter, you can do something like

fate-dca-core-dmix: REF = 8e8f2e223787d2436853b5310dc6ec04

To avoid creating external ref files for md5 tests.

> 
>>> +
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-xll-pbr
>>> +fate-dca-xll-pbr: CMD = framemd5 -err_detect crccheck -i $(TARGET_SAMPLES)/dts/dca-xll-pbr -c:a pcm_s16le
>>> +
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-xll-read
>>> +fate-dca-xll-read: CMD = framemd5 -err_detect crccheck -i $(TARGET_SAMPLES)/dts/dca-xll-read -c:a pcm_s24le
>>> +
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-xll-sync
>>> +fate-dca-xll-sync: CMD = framemd5 -err_detect crccheck -i $(TARGET_SAMPLES)/dts/dca-xll-sync -c:a pcm_s24le
>>
>> Can you limit these to only a bunch of audio frames, the minimum amount
>> necessary to get the intended code coverage, in order to avoid big
>> samples and ref files with hundreds of lines? Also because calculating
>> md5 is expensive.
>> Notice how the first tests added using your samples (dcadec-suite
>> folder) are all like six frames each.
> 
> fate-dca-xll-read test can be possibly reduced to a few frames, but -pbr
> and -sync tests cannot (these need to encompass entire bitrate-managed
> section of audio track, which can be quite lengthy). I can change those
> to md5 if size of reference files is of concern.

Keep -pbr and -sync as they are and try to make the rest smaller, then.
Smallest size possible while making sure the test coverage is the
intended. It'll help save some bandwidth over time in our rsync server.

And you can keep them as framemd5. You chose it for those instead of md5
like with the above, so i assume frame checksum is relevant/interesting.

> 
>> Also, the naming scheme is different and you're placing them outside of
>> the dcadec-suite folder. But that's not as important as the above.
> 
> These files are not part of the original dcadec test suite, therefore
> different naming scheme. How do you prefer them to be named?

They are fine as is. Just figured you might want all the tests added
targeting your dcadec version in its own folder.

> 
>>> +
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-lbr
>>> +fate-dca-lbr: CMD = pcm -err_detect crccheck -i $(TARGET_SAMPLES)/dts/dca-lbr
>>> +fate-dca-lbr: CMP = oneoff
>>> +fate-dca-lbr: REF = $(SAMPLES)/dts/dca-lbr.pcm
>>> +
>>> +FATE_DCA-$(call DEMDEC, DTS, DCA) += fate-dca-lbr-256
>>> +fate-dca-lbr-256: CMD = pcm -err_detect crccheck -i $(TARGET_SAMPLES)/dts/dca-lbr-256
>>> +fate-dca-lbr-256: CMP = oneoff
>>> +fate-dca-lbr-256: REF = $(SAMPLES)/dts/dca-lbr-256.pcm
>>
>> Did you create the pcm files using your decoder, or created the lbr
>> samples from already existing pcm files? If the former, with what kind
>> of build and decoder settings?
> 
> PCM files where generated with FFmpeg git (x86_64 build) by running make
> fate-dca-lbr(-256).

Afaik, reference pcm files are usually created using x86_32 builds with
cpuflags set to 0. Don't think it's a requirement, though.

> 
>> I ask because, long ago when you first wrote the lbr decoder, you gave
>> me a bunch of samples i tried to use to write some tests. I used a
>> x86_32 build and forced no handwritten assembly (so it's all x87 floats)
>> to create pcm reference files, and the result was that x86_64 builds
>> (using sse floats, hand written assembly or otherwise) would
>> unexpectedly fail the tests by a huge error margin.
>>
>> I narrowed the problem down to dcadsp's lbr_bank_c. That function
>> compiled with x87 floats and sse floats would give different results,
>> and i couldn't figure out why. Maybe it was the compiler i used back
>> then (mingw-w64 gcc 5 i think).
> 
> That's unexpected, I'll check how this behaves on x86_32.
> 
> I would expect lfe_iir_c (which is probably broken) to give different
> results based on type of math used, but not lbr_bank_c. Samples I use
> for FATE here purposedly don't use LFE for this matter.

It might have been lfe_iir_c, now that you mention it. It was a while
ago, and i recall it was a function used by the lbr decoder that didn't
have a simd version.

> 
>> For that matter, the lbr samples you gave me back then were one mono,
>> one stereo and one 5.1, which was more varied than the two stereo ones
>> you're using here.
>> I can upload them somewhere if you don't have them anymore.
> 
> I think I should still have them, will look.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>