rnnoise icon indicating copy to clipboard operation
rnnoise copied to clipboard

Windows and CMake support

Open almoghamdani opened this issue 4 years ago • 9 comments

This PR includes the following changes:

  • Removed the use of VLA in order to support MSVC that doesn't have implementation for VLA and fixed more issues that occurred during the compile on Windows.
  • Add CMakeLists.txt to support compile using CMake.
  • Add an option to compile without opus functions if linking against Opus

almoghamdani avatar Jul 27 '19 11:07 almoghamdani

Regarding eb45832a2a39df6955a5da0c4aebf5586cd9c8e4: I would do something like mumble-voip@b30f2bb8049601c3b7253d47f75dc6a68c5f32fd, instead.

davidebeatrici avatar Sep 20 '19 02:09 davidebeatrici

Very welcome change! Autotools, VLAs and overlap with Opus source code are really annoying. The code does high quality denoising, but once you start deploying it on different platforms you realize it needs a lot of TLC around compilation/build process

witaly-iwanow avatar Mar 12 '20 07:03 witaly-iwanow

Is there a reason why this was never merged?

tahnik avatar Nov 08 '20 18:11 tahnik

I'm not heavily involved in the development, but took the chance to merge the C++ header fix in #131, causing this change to no longer merge cleanly. Perhaps you can rebase it without the header change?

petterreinholdtsen avatar Jan 18 '21 09:01 petterreinholdtsen

@petterreinholdtsen I rebased and removed the header changes.

almoghamdani avatar Jan 18 '21 20:01 almoghamdani

@almogh52 maybe you also want to have a look at my changes based on your branch which can be found here: https://github.com/alfatraining/rnnoise/commit/7d6f71f1a9f7153dc394258dd6ef7109ae79795b

These changes are mostly carried over from community-contributed CMake code for libspeex.

j-schultz avatar Jan 20 '21 13:01 j-schultz

This pull request also remove two symbols from the list of exported symbols in the library.

Anyway, I suspect the different parts of this pull request should be handled and evaluated separately. At least I believe the introduction of a parallel build system should be decided independently of moving several local variables to the heap. Perhaps better to make separate pull reqeusts for each set of changes?

petterreinholdtsen avatar Jan 22 '21 21:01 petterreinholdtsen

Once #164 is merged you can drop the VLA-related changes and just add to the CMake project the following:

if(MSVC)
	target_compile_definitions(rnnoise PRIVATE "USE_MALLOC")
endif()

davidebeatrici avatar Feb 16 '21 13:02 davidebeatrici

@almoghamdani static win32 lib built. But I failed to integrated the lib into a win32 console project.

LNK2019 unreferenced symbol _opus_fft_c, _forward_transform refered from testRNNoise xxxxxxxx\testRNNoise\rnnoise.lib(denoise.obj)

wnpllrzodiac avatar May 12 '21 09:05 wnpllrzodiac