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

[foo86]

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.

> > +
> > +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.

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

> > +
> > +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).

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

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