search

leomccormack / Spatial_Audio_Framework

A cross-platform framework for developing spatial audio algorithms and software in C/C++
https://leomccormack.github.io/Spatial_Audio_Framework/
Other
569 stars 91 forks source link

Access violation in `saf_example_ambi_bin` while loading HRTFs from SONICOM dataset #55

Open 1enn0 opened 1 year ago

1enn0 commented 1 year ago

Description

Using a HRTF from the SONICOM HRTF dataset will throw an exception (access violation). This has been tested with multiple HRTFs from the mentioned dataset.

Environment

  1. OS: Windows 11 22H2 (22621.1778)
  2. Compiler: MSVC 14.36.32532
  3. SAF performance library being used: SAF_USE_INTEL_MKL
  4. Any optional SAF flags enabled: SAF_ENABLE_SOFA_READER_MODULE, SAF_ENABLE_EXAMPLES_TESTS
  5. Visual Studio Professional 2022 17.6.2, CMake 3.26.2

Reproducible Steps

Steps to create the smallest reproducible scenario:

  1. Modify test__saf_example_ambi_bin() in file src/test/test__examples.c to load a SOFA file from the SONICOM dataset: 
    // ...
ambi_bin_create(&hAmbi);
ambi_bin_setSofaFilePath(hAmbi, "path/to/eg/P0010_Windowed_48kHz.sofa");
// ...
  2. Build and run saf_test with above environment.

Expected Output

Test should pass.

Actual Output

Access violation in framework/modules/saf_utilities/saf_utility_geometry.c:782 while trying to read faceIdx[k]. faceIdx does not exist because nFaceIdx is 0 here.

Additional information

Call Stack:

[0] saf_test.exe!sphVoronoi
    at ...\framework\modules\saf_utilities\saf_utility_geometry.c(782)
[1] saf_test.exe!getVoronoiWeights
    at ...\framework\modules\saf_utilities\saf_utility_geometry.c(959)
[2] saf_test.exe!ambi_bin_initCodec
    at ...\examples\src\ambi_bin\ambi_bin.c(262)
[3] saf_test.exe!test__saf_example_ambi_bin
    at ...\test\src\test__examples.c(56)
[4] saf_test.exe!main_test
    at ...\test\src\saf_test.c(173)
[5] saf_test.exe!main
    at ...\test\src\saf_test_wrapper.cpp(30)
rapolasd commented 1 year ago

Hi!

This is due to the fact that in the current release of the SONICOM HRTF database we kept redundant measurements of the top HRTF (90 degree elevation) in the sofa file (during the measurement, the top direction was measured each time the chair was rotated). Having multiple measurements at the same position in the sofa file caused the HRTF interpolation to fail. Since finding this bug ourselves, we decided to remove the redundant positions from the database. We will upload the new versions to the website soon but in the meantime, let me know if you want me to send you some specific corrected HRTF files directly.

This could also be an improvement to the SAF HRTF loader to check for overlapping positions, warn the user, and only take one of the measurements at that location.

1enn0 commented 1 year ago

Hi @rapolasd, thank you for the info! I suspected something like this might be happening. I'm happy to wait until you update the database :)

Meanwhile, I did a quick check some time ago and built the test above from the develop branch. IIRC, the issue did not appear (will have to verify). There was some work on the ambi_bin example app, for example this commit. Maybe @leomccormack can help whether the issue with the overlapping positions has already been addressed in some way?