Fix int normalisation when reading in floats with the replacement reader #702

bobsayshilol · 2021-02-12T23:33:36Z

This was initially spotted by inspection (I don't have hardware that falls back to using this path) but it was confirmed to be a bug after forcing psf->ieee_replace to be enabled in psf_open_file() and re-running the test suite.

Along with a patch to fix the normalisation, I've also extended floating_point_test.tpl to add tests that would have caught the incorrect normalisation.

…k floating point path. Without this the values were only being scaled to a short's max value.

…acement read functionality. These cover the issue fixed by the previous commit.

evpobr · 2021-02-13T06:35:57Z

Hi @bobsayshilol , thanks for pull request.

Do I understand correctly that it was most likely a typo?

@erikd , @arthurt , @SoapGentoo , any thoughts? Looks correct to me.

SoapGentoo · 2021-02-13T09:09:45Z

looks correct to me too. @evpobr could you test it locally, reverting the fix but keeping the test commit and seeing that the test fails without the fix?

evpobr · 2021-02-13T10:07:59Z

looks correct to me too. @evpobr could you test it locally, reverting the fix but keeping the test commit and seeing that the test fails without the fix?

Ok.

evpobr · 2021-02-13T10:22:59Z

PS D:\source\repos\libsndfile\build> ctest -R floating_point_test -V
UpdateCTestConfiguration  from :D:/source/repos/libsndfile/build/DartConfiguration.tcl
Parse Config file:D:/source/repos/libsndfile/build/DartConfiguration.tcl
UpdateCTestConfiguration  from :D:/source/repos/libsndfile/build/DartConfiguration.tcl
Parse Config file:D:/source/repos/libsndfile/build/DartConfiguration.tcl
Test project D:/source/repos/libsndfile/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 8
    Start 8: floating_point_test

8: Test command: D:\source\repos\libsndfile\build\floating_point_test.exe
8: Test timeout computed to be: 1500
8:     float_scaled_test              : float.raw ............... -500.0dB SNR ... ok
8:     float_scaled_test              : pcm_s8.raw ..............  -42.1dB SNR ... ok
8:     float_scaled_test              : pcm_u8.raw ..............  -42.1dB SNR ... ok
8:     float_scaled_test              : pcm_16.raw ..............  -90.2dB SNR ... ok
8:     float_scaled_test              : pcm_24.raw .............. -138.5dB SNR ... ok
8:     float_scaled_test              : pcm_32.raw .............. -226.0dB SNR ... ok
8:     float_scaled_test              : ulaw.raw ................  -61.7dB SNR ... ok
8:     float_scaled_test              : alaw.raw ................  -61.0dB SNR ... ok
8:     float_scaled_test              : ima_adpcm.wav ...........  -58.2dB SNR ... ok
8:     float_scaled_test              : ms_adpcm.wav ............  -49.2dB SNR ... ok
8:     float_scaled_test              : gsm610.raw ..............  -38.5dB SNR ... ok
8:     float_scaled_test              : g721_32.au ..............  -39.5dB SNR ... ok
8:     float_scaled_test              : g723_24.au ..............  -39.2dB SNR ... ok
8:     float_scaled_test              : g723_40.au ..............  -44.6dB SNR ... ok
8:     float_scaled_test              : le_paf_24.paf ........... -153.1dB SNR ... ok
8:     float_scaled_test              : be_paf_24.paf ........... -153.1dB SNR ... ok
8:     float_scaled_test              : dwvw_12.raw .............  -78.0dB SNR ... ok
8:     float_scaled_test              : dwvw_16.raw ............. -102.2dB SNR ... ok
8:     float_scaled_test              : dwvw_24.raw ............. -153.1dB SNR ... ok
8:     float_scaled_test              : adpcm.vox ...............  -56.4dB SNR ... ok
8:     float_scaled_test              : dpcm_16.xi ..............  -90.2dB SNR ... ok
8:     float_scaled_test              : dpcm_8.xi ...............  -42.1dB SNR ... ok
8:     float_scaled_test              : pcm_s8.sds ..............  -89.8dB SNR ... ok
8:     float_scaled_test              : pcm_16.sds .............. -132.2dB SNR ... ok
8:     float_scaled_test              : pcm_24.sds .............. -176.3dB SNR ... ok
8:     float_scaled_test              : alac_16.caf ............. -102.2dB SNR ... ok
8:     float_scaled_test              : alac_32.caf ............. -226.0dB SNR ... ok
8:     float_scaled_test              : alac_24.caf ............. -153.1dB SNR ... ok
8:     float_scaled_test              : alac_20.caf ............. -125.9dB SNR ... ok
8:     float_scaled_test              : flac_8.flac .............  -42.1dB SNR ... ok
8:     float_scaled_test              : flac_16.flac ............  -90.2dB SNR ... ok
8:     float_scaled_test              : flac_24.flac ............ -138.5dB SNR ... ok
8:     float_scaled_test              : vorbis.oga ..............  -41.3dB SNR ... ok
8:     float_scaled_test              : opus.opus ...............  -68.0dB SNR ... ok
8:     float_scaled_test              : replace_float.raw ....... -500.0dB SNR ... ok
8:     double_scaled_test             : double.raw .............. -500.0dB SNR ... ok
8:     double_scaled_test             : pcm_s8.raw ..............  -42.1dB SNR ... ok
8:     double_scaled_test             : pcm_u8.raw ..............  -42.1dB SNR ... ok
8:     double_scaled_test             : pcm_16.raw ..............  -90.2dB SNR ... ok
8:     double_scaled_test             : pcm_24.raw .............. -138.6dB SNR ... ok
8:     double_scaled_test             : pcm_32.raw .............. -186.6dB SNR ... ok
8:     double_scaled_test             : ulaw.raw ................  -61.7dB SNR ... ok
8:     double_scaled_test             : alaw.raw ................  -61.0dB SNR ... ok
8:     double_scaled_test             : ima_adpcm.wav ...........  -58.2dB SNR ... ok
8:     double_scaled_test             : ms_adpcm.wav ............  -49.2dB SNR ... ok
8:     double_scaled_test             : gsm610.raw ..............  -38.5dB SNR ... ok
8:     double_scaled_test             : g721_32.au ..............  -39.5dB SNR ... ok
8:     double_scaled_test             : g723_24.au ..............  -39.2dB SNR ... ok
8:     double_scaled_test             : g723_40.au ..............  -44.6dB SNR ... ok
8:     double_scaled_test             : be_paf_24.paf ........... -154.0dB SNR ... ok
8:     double_scaled_test             : le_paf_24.paf ........... -154.0dB SNR ... ok
8:     double_scaled_test             : dwvw_12.raw .............  -78.0dB SNR ... ok
8:     double_scaled_test             : dwvw_16.raw ............. -102.3dB SNR ... ok
8:     double_scaled_test             : dwvw_24.raw ............. -154.0dB SNR ... ok
8:     double_scaled_test             : adpcm.vox ...............  -56.4dB SNR ... ok
8:     double_scaled_test             : dpcm_16.xi ..............  -90.2dB SNR ... ok
8:     double_scaled_test             : dpcm_8.xi ...............  -42.1dB SNR ... ok
8:     double_scaled_test             : pcm_s8.sds ..............  -89.8dB SNR ... ok
8:     double_scaled_test             : pcm_16.sds .............. -132.3dB SNR ... ok
8:     double_scaled_test             : pcm_24.sds .............. -180.2dB SNR ... ok
8:     double_scaled_test             : alac_16.caf ............. -102.3dB SNR ... ok
8:     double_scaled_test             : alac_20.caf ............. -125.9dB SNR ... ok
8:     double_scaled_test             : alac_24.caf ............. -154.0dB SNR ... ok
8:     double_scaled_test             : alac_32.caf ............. -186.6dB SNR ... ok
8:     double_scaled_test             : flac_8.flac .............  -42.1dB SNR ... ok
8:     double_scaled_test             : flac_16.flac ............  -90.2dB SNR ... ok
8:     double_scaled_test             : flac_24.flac ............ -138.6dB SNR ... ok
8:     double_scaled_test             : vorbis.oga ..............  -41.3dB SNR ... ok
8:     double_scaled_test             : opus.opus ...............  -68.0dB SNR ... ok
8:     double_scaled_test             : replace_double.raw ...... -500.0dB SNR ... ok
8: 
8:     float_short_little_test        : float_short_little.au ... ok
8:     float_short_little_test        : float_short_little_replace.au  ok
8:     float_short_big_test           : float_short_big.au ...... ok
8:     float_short_big_test           : float_short_big_replace.au  ok
8:     float_int_little_test          : float_int_little.au ..... ok
8:     float_int_little_test          : float_int_little_replace.au  
8:
8: Line 491: Bad maximum (32766 should be 2147483647).
8:
1/1 Test #8: floating_point_test ..............***Failed   14.19 sec

0% tests passed, 1 tests failed out of 1

Total Test time (real) =  14.27 sec

The following tests FAILED:
          8 - floating_point_test (Failed)
Errors while running CTest

SoapGentoo · 2021-02-13T11:07:20Z

@bobsayshilol looks good, please add an entry to NEWS to capture this fix

bobsayshilol · 2021-02-13T15:21:21Z

Do I understand correctly that it was most likely a typo?

Yes, that looks like it was the case.

@bobsayshilol looks good, please add an entry to NEWS to capture this fix

89bed47 looks to have deprecated NEWS, so I've added it to CHANGELOG.md in 91cb012.

evpobr · 2021-02-13T15:30:36Z

Squash commits to one please before I merge it.

bobsayshilol · 2021-02-13T16:02:55Z

Squash commits to one please before I merge it.

Can that not be done as part of the merge? https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squash-and-merge-your-pull-request-commits

evpobr · 2021-02-14T04:45:51Z

Thanks @bobsayshilol !

bobsayshilol added 2 commits February 12, 2021 22:47

Use the correct scale when normalising an integer read on the fallbac…

664cbde

…k floating point path. Without this the values were only being scaled to a short's max value.

Add a second pass over the floating point tests that enables the repl…

b09320d

…acement read functionality. These cover the issue fixed by the previous commit.

evpobr added the Bug Something isn't working label Feb 13, 2021

SoapGentoo approved these changes Feb 13, 2021

View reviewed changes

Add an entry to CHANGELOG.md for libsndfile#702.

91cb012

evpobr assigned SoapGentoo Feb 14, 2021

evpobr added this to the v1.1.0 milestone Feb 14, 2021

evpobr merged commit ecb9672 into libsndfile:master Feb 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix int normalisation when reading in floats with the replacement reader #702

Fix int normalisation when reading in floats with the replacement reader #702

bobsayshilol commented Feb 12, 2021

evpobr commented Feb 13, 2021

SoapGentoo commented Feb 13, 2021

evpobr commented Feb 13, 2021

evpobr commented Feb 13, 2021

SoapGentoo commented Feb 13, 2021

bobsayshilol commented Feb 13, 2021

evpobr commented Feb 13, 2021

bobsayshilol commented Feb 13, 2021

evpobr commented Feb 14, 2021

Fix int normalisation when reading in floats with the replacement reader #702

Fix int normalisation when reading in floats with the replacement reader #702

Conversation

bobsayshilol commented Feb 12, 2021

evpobr commented Feb 13, 2021

SoapGentoo commented Feb 13, 2021

evpobr commented Feb 13, 2021

evpobr commented Feb 13, 2021

SoapGentoo commented Feb 13, 2021

bobsayshilol commented Feb 13, 2021

evpobr commented Feb 13, 2021

bobsayshilol commented Feb 13, 2021

evpobr commented Feb 14, 2021