search

xiph / rnnoise

Recurrent neural network for audio noise reduction
BSD 3-Clause "New" or "Revised" License
3.89k stars 882 forks source link

Use std::vector #193

Closed WindowsNT closed 2 years ago

WindowsNT commented 2 years ago

I'ts better to ensure portability by converting all files to C++ and avoid variable-size arrays. That way it will also compile with Visual Studio.

petterreinholdtsen commented 2 years ago

[Michael Chourdakis]

I'ts better to ensure portability by converting all files to C++ and avoid variable-size arrays. That way it will also compile with Visual Studio.

How will converting to C++ ensure portability? Did C++ compilers reach binary compatibility and stability recently?

-- Happy hacking Petter Reinholdtsen

rillian commented 2 years ago

I'm sure they mean portability to MSVC.

Replacing variable-length array declarations with dynamic allocations would be a simpler resolution. That still adds portability concerns for some very constrained targets, but it looks like we already have some allocator calls.

@WindowsNT Can you offer a list of the source locations which are failing to compile for you?

WindowsNT commented 2 years ago

Variable size arrays are not standard. std::vector is. I have already converted everything to std::vector, just giving a hint to make this work for windows as well.

rillian commented 2 years ago

I'm not the maintainer, but we're unlikely to convert the project to C++ to work around Visual Studio's incomplete C99 support.

jmvalin commented 2 years ago

RNNoise is C99. std::vector is not standard C. Also, it performs dynamic allocation, which is bad for real-time processing. The correct solution is to do what Opus does, which is have other fallbacks: either alloca() or offsets into a (non-threadsafe) large global array.