drowe67 / LPCNet

Experimental Neural Net speech coding for FreeDV
BSD 3-Clause "New" or "Revised" License
68 stars 24 forks source link

Additional Windows related updates after Codec2 decoupling #45

Closed tmiw closed 1 year ago

tmiw commented 1 year ago

While making sure freedv-gui still compiles, I noticed I was having build issues with MinGW. Further investigation revealed. that -fvisibility=hidden doesn't actually work for DLLs, so this PR renames each of the Codec2 functions used to avoid duplicate symbol errors.

See #43 for more details of the original change.

drowe67 commented 1 year ago

While making sure freedv-gui still compiles, I noticed I was having build issues with MinGW. Further investigation revealed. that -fvisibility=hidden doesn't actually work for DLLs,

Oh dear, that's unfortunate :disappointed:

Should we remove -fvisibility=hidden or revert the CMakefile.txt given it's not a global solution?

tmiw commented 1 year ago

While making sure freedv-gui still compiles, I noticed I was having build issues with MinGW. Further investigation revealed. that -fvisibility=hidden doesn't actually work for DLLs,

Oh dear, that's unfortunate 😞

Should we remove -fvisibility=hidden or revert the CMakefile.txt given it's not a global solution?

I think it's still good practice if only so that someone doesn't decide to try to call __codec2__codec2_fftr() for example. It's also possible that a different version of Clang or GCC will eventually make that command line option work for Windows too.

(Speaking of which, I've been meaning to build a new Debian based Docker container that uses the LLVM version of MinGW. On top of a newer compiler, we'd also get the ability to easily build ARM builds for Windows.)

drowe67 commented 1 year ago

@tmiw OK if I merge this?

tmiw commented 1 year ago

@tmiw OK if I merge this?

Go for it. 👍