videolabs / libspatialaudio

Ambisonic encoding / decoding and binauralization library in C++
Other
198 stars 37 forks source link

Replace variable length array with vector to fix MSVC build #17

Closed dgrahelj closed 4 years ago

dgrahelj commented 4 years ago

When building libspatialaudio with MIT_HRTF using MSVC (Visual Studio 15 2017 Win64), I get the following error, because of the variable length array in mit_hrtf.cpp:

libspatialaudio\source\hrtf\mit_hrtf.cpp(25): error C2131: expression did not evaluate to a constant

Replacing the C-style array with a std::vector fixes the problem. The function MIT_HRTF::get is only ever called in the Configure step of the the binauralizer classes, so it's not real-time critical and vector allocations are not a problem.

chouquette commented 4 years ago

Hi,

Thanks for your patch! I agree with the idea, but do you need the 2 vectors? It seems std::vector<short> psHRTF[2] would be enough, and would be a bit simpler to read IMO

dgrahelj commented 4 years ago

I agree with it being simpler to read, but it would create a few more additions since I guess you would then have to add these lines to reserve space for the vectors (otherwise data() might return nullptr):

std::vector<short> psHRTF[2];
psHRTF[0].reserve(i_len);
psHRTF[1].reserve(i_len);

A third option would be to have a std::array of vectors, since one of the dimensions is a compile time constant:

std::array<std::vector<short>, 2> psHRTF{std::vector<short>(i_len), std::vector<short>(i_len)};
chouquette commented 4 years ago

I suppose you could also to the same initialization with a regular array no?

dgrahelj commented 4 years ago

You're right. Just updated

chouquette commented 4 years ago

LGTM :) Thanks!

jbkempf commented 4 years ago

Thanks.