| Message ID | 20220915162145.2284957-4-mvanb1@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | 32bps FLAC patches | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Martijn van Beurden: > --- > tests/fate/flac.mak | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/tests/fate/flac.mak b/tests/fate/flac.mak > index 115cc965e1..4db28b1e1d 100644 > --- a/tests/fate/flac.mak > +++ b/tests/fate/flac.mak > @@ -6,6 +6,8 @@ FATE_FLAC += fate-flac-16-chmode-indep \ > fate-flac-16-lpc-cholesky \ > fate-flac-16-lpc-levinson \ > fate-flac-24-comp-8 \ > + fate-flac-32 \ > + fate-flac-32-wasted-bits \ > fate-flac-rice-params \ > > fate-flac-16-chmode-%: OPTS = -ch_mode $(@:fate-flac-16-chmode-%=%) > @@ -20,6 +22,13 @@ fate-flac-24-comp-%: OPTS = -compression_level $(@:fate-flac-24-comp-%=%) > fate-flac-24-%: REF = $(SAMPLES)/audio-reference/divertimenti_2ch_96kHz_s24.wav > fate-flac-24-%: CMD = enc_dec_pcm flac wav s24le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac $(OPTS) > > + > +fate-flac-32: REF = $(SAMPLES)/audio-reference/drums_2ch_44kHz_s32.wav > +fate-flac-32: CMD = enc_dec_pcm flac wav s32le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac -strict -2 We already have 32bit samples in the fate-suite, namely wavpack/lossless/32bit_int_partial.wv. Why don't you use these? (Anyway, don't hardcode the numerical value of 'experimental'.) > + > +fate-flac-32-wasted-bits: REF = $(SAMPLES)/audio-reference/drums_2ch_44kHz_s32_1wastedbit.wav > +fate-flac-32-wasted-bits: CMD = enc_dec_pcm flac wav s32le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac -strict -2 > + > fate-flac-rice-params: REF = $(SAMPLES)/audio-reference/chorusnoise_2ch_44kHz_s16.wav > fate-flac-rice-params: CMD = enc_dec_pcm flac wav s16le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac >
Op do 15 sep. 2022 om 18:58 schreef Andreas Rheinhardt < andreas.rheinhardt@outlook.com>: > Martijn van Beurden: > > --- > > tests/fate/flac.mak | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/tests/fate/flac.mak b/tests/fate/flac.mak > > index 115cc965e1..4db28b1e1d 100644 > > --- a/tests/fate/flac.mak > > +++ b/tests/fate/flac.mak > > @@ -6,6 +6,8 @@ FATE_FLAC += fate-flac-16-chmode-indep > \ > > fate-flac-16-lpc-cholesky > \ > > fate-flac-16-lpc-levinson > \ > > fate-flac-24-comp-8 > \ > > + fate-flac-32 > \ > > + fate-flac-32-wasted-bits > \ > > fate-flac-rice-params > \ > > > > fate-flac-16-chmode-%: OPTS = -ch_mode $(@:fate-flac-16-chmode-%=%) > > @@ -20,6 +22,13 @@ fate-flac-24-comp-%: OPTS = -compression_level > $(@:fate-flac-24-comp-%=%) > > fate-flac-24-%: REF = > $(SAMPLES)/audio-reference/divertimenti_2ch_96kHz_s24.wav > > fate-flac-24-%: CMD = enc_dec_pcm flac wav s24le $(subst > $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac $(OPTS) > > > > + > > +fate-flac-32: REF = $(SAMPLES)/audio-reference/drums_2ch_44kHz_s32.wav > > +fate-flac-32: CMD = enc_dec_pcm flac wav s32le $(subst > $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac -strict -2 > > We already have 32bit samples in the fate-suite, namely > wavpack/lossless/32bit_int_partial.wv. Why don't you use these? > (Anyway, don't hardcode the numerical value of 'experimental'.) > I wasn't sure that was possible. If for example the build is configured to leave wavpack out that wouldn't work. Or is it assumed that fate is only run with a ffmpeg build configured with default and some additions but nothing default left out? Considering the strict value, I guess I should also change the help text then? Now it reads: encoding as 24 bits-per-sample, more is considered experimental. Add -strict -2 if you want to encode more than 24 bits-per-sample
Martijn van Beurden: > Op do 15 sep. 2022 om 18:58 schreef Andreas Rheinhardt < > andreas.rheinhardt@outlook.com>: > >> Martijn van Beurden: >>> --- >>> tests/fate/flac.mak | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/tests/fate/flac.mak b/tests/fate/flac.mak >>> index 115cc965e1..4db28b1e1d 100644 >>> --- a/tests/fate/flac.mak >>> +++ b/tests/fate/flac.mak >>> @@ -6,6 +6,8 @@ FATE_FLAC += fate-flac-16-chmode-indep >> \ >>> fate-flac-16-lpc-cholesky >> \ >>> fate-flac-16-lpc-levinson >> \ >>> fate-flac-24-comp-8 >> \ >>> + fate-flac-32 >> \ >>> + fate-flac-32-wasted-bits >> \ >>> fate-flac-rice-params >> \ >>> >>> fate-flac-16-chmode-%: OPTS = -ch_mode $(@:fate-flac-16-chmode-%=%) >>> @@ -20,6 +22,13 @@ fate-flac-24-comp-%: OPTS = -compression_level >> $(@:fate-flac-24-comp-%=%) >>> fate-flac-24-%: REF = >> $(SAMPLES)/audio-reference/divertimenti_2ch_96kHz_s24.wav >>> fate-flac-24-%: CMD = enc_dec_pcm flac wav s24le $(subst >> $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac $(OPTS) >>> >>> + >>> +fate-flac-32: REF = $(SAMPLES)/audio-reference/drums_2ch_44kHz_s32.wav >>> +fate-flac-32: CMD = enc_dec_pcm flac wav s32le $(subst >> $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac -strict -2 >> >> We already have 32bit samples in the fate-suite, namely >> wavpack/lossless/32bit_int_partial.wv. Why don't you use these? >> (Anyway, don't hardcode the numerical value of 'experimental'.) >> > > I wasn't sure that was possible. If for example the build is configured to > leave wavpack out that wouldn't work. Or is it assumed that fate is only > run with a ffmpeg build configured with default and some additions but > nothing default left out? > The wavpack decoder would have to be a requirement of this test. (The wav demuxer is btw needed as a requirement for the current tests, but has been left out; the pcm_s16le decoder is also needed for the tests with s16 input samples and the pcm_s24le decoder for the with s24 input samples, but checking for this has been forgotten.) Notice that the test functions would have to be tweaked (the flac tests use CMP = oneoff, i.e. they do not check checksums of the encoded file, but instead test that the decoded audio data coincides with what has been sent to the decoder). (In any case, I don't get why there are two samples: The flac decoder is intra-only, so if you made the samples so that one of the frames had a wasted bit, you could test all the codepaths that two samples test. And it would even have the advantage that it tests what happens when it swiches from >0 wasted bits to 0 wasted bits to >0 wasted bits and back.) > Considering the strict value, I guess I should also change the help text > then? Now it reads: encoding as 24 bits-per-sample, more is considered > experimental. Add -strict -2 if you want to encode more than 24 > bits-per-sample Yes, that should also be changed. - Andreas
Op do 15 sep. 2022 om 20:20 schreef Andreas Rheinhardt < andreas.rheinhardt@outlook.com>: > (In any case, I don't get why there are two samples: The flac decoder is > intra-only, so if you made the samples so that one of the frames had a > wasted bit, you could test all the codepaths that two samples test. And > it would even have the advantage that it tests what happens when it > swiches from >0 wasted bits to 0 wasted bits to >0 wasted bits and back.) > It seemed easier for anyone triggering a problem with wasted bits to figure where the problem came from. A problem in the second file but not in the first is quite certain to be related to the wasted bits handling. But perhaps the probability of this happening is so low that it is not worth increasing the size of the fate sample collection. I'll create a new file and ask for a replacement.
Op do 15 sep. 2022 om 21:52 schreef Martijn van Beurden <mvanb1@gmail.com>: > Op do 15 sep. 2022 om 20:20 schreef Andreas Rheinhardt < > andreas.rheinhardt@outlook.com>: > >> (In any case, I don't get why there are two samples: The flac decoder is >> intra-only, so if you made the samples so that one of the frames had a >> wasted bit, you could test all the codepaths that two samples test. And >> it would even have the advantage that it tests what happens when it >> swiches from >0 wasted bits to 0 wasted bits to >0 wasted bits and back.) >> > > It seemed easier for anyone triggering a problem with wasted bits to > figure where the problem came from. A problem in the second file but not in > the first is quite certain to be related to the wasted bits handling. But > perhaps the probability of this happening is so low that it is not worth > increasing the size of the fate sample collection. I'll create a new file > and ask for a replacement. > Files have been replaced, there is now one file for this test with varying levels of wasted bits. At the end it also has a differing number of wasted bits between the left and right channel. In other decoders this has been a problem in the past. It now changes 0-0, 1-1, 0-0, 1-1, 3-2, 2-3, where each pair is the number of wasted bits for the left and right channel. I'll send new patches in a moment.
diff --git a/tests/fate/flac.mak b/tests/fate/flac.mak index 115cc965e1..4db28b1e1d 100644 --- a/tests/fate/flac.mak +++ b/tests/fate/flac.mak @@ -6,6 +6,8 @@ FATE_FLAC += fate-flac-16-chmode-indep \ fate-flac-16-lpc-cholesky \ fate-flac-16-lpc-levinson \ fate-flac-24-comp-8 \ + fate-flac-32 \ + fate-flac-32-wasted-bits \ fate-flac-rice-params \ fate-flac-16-chmode-%: OPTS = -ch_mode $(@:fate-flac-16-chmode-%=%) @@ -20,6 +22,13 @@ fate-flac-24-comp-%: OPTS = -compression_level $(@:fate-flac-24-comp-%=%) fate-flac-24-%: REF = $(SAMPLES)/audio-reference/divertimenti_2ch_96kHz_s24.wav fate-flac-24-%: CMD = enc_dec_pcm flac wav s24le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac $(OPTS) + +fate-flac-32: REF = $(SAMPLES)/audio-reference/drums_2ch_44kHz_s32.wav +fate-flac-32: CMD = enc_dec_pcm flac wav s32le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac -strict -2 + +fate-flac-32-wasted-bits: REF = $(SAMPLES)/audio-reference/drums_2ch_44kHz_s32_1wastedbit.wav +fate-flac-32-wasted-bits: CMD = enc_dec_pcm flac wav s32le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac -strict -2 + fate-flac-rice-params: REF = $(SAMPLES)/audio-reference/chorusnoise_2ch_44kHz_s16.wav fate-flac-rice-params: CMD = enc_dec_pcm flac wav s16le $(subst $(SAMPLES),$(TARGET_SAMPLES),$(REF)) -c flac