mackron / dr_libs

Audio decoding libraries for C/C++, each in a single source file.
Other
1.24k stars 205 forks source link

Dev #249

Closed 6r0m closed 1 year ago

6r0m commented 1 year ago

implement: "extern inline" to the DRWAV_API for using dr_wav with different separated modules in one project.

mackron commented 1 year ago

Can't accept this one. This change doesn't sense.

6r0m commented 1 year ago

@mackron why do you say that?

for example, if we use two plugins in the Unreal Engine with your library (each of them has different purpose and from different authors) then we have errors when build project without inline keyword in each plugin.

with "extern inline" compiler will decide what to do with duplicate code (insert or generate with notifying other modules) https://www.drdobbs.com/the-new-c-inline-functions/184401540

thank you anyway for your great work.

6r0m commented 1 year ago

attached linker errors. RuntimeAudioImporter - it's the second module. SoundUtilityLibrary.cpp.obj - from first module

Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_alaw_to_f32 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_alaw_to_s16 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_alaw_to_s32 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_bytes_to_f32 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_bytes_to_s16 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_bytes_to_s32 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_bytes_to_s64 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_bytes_to_u16 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_bytes_to_u32 already defined in SoundUtilityLibrary.cpp.obj
Module.RuntimeAudioImporter.cpp.obj : error LNK2005: drwav_bytes_to_u64 already defined in SoundUtilityLibrary.cpp.obj

and so on

mackron commented 1 year ago

Those errors are because you've included the implementation section of dr_wav more than once. You can only do #define DRWAV_IMPLEMENTATION in one source file. It might be easier to have your own dr_wav.c that you put the implementation section in.

6r0m commented 1 year ago

@mackron imagine two plugins with dr_wav implementation. they both have own #define DRWAV_IMPLEMENTATION in their build.cs, because these plugins are autonomous in using your helpful tool.

RuntimeAudioImporter.Build.cs has:

PublicDefinitions.AddRange(
new string[]
 {
  "DR_WAV_IMPLEMENTATION=1",
  "DR_MP3_IMPLEMENTATION=1",
  "DR_FLAC_IMPLEMENTATION=1"
 }
);

and MetahumanSDK.Build.cs has:

PublicDefinitions.AddRange(
new string[]
 {
  "DR_WAV_IMPLEMENTATION=1"
 }
);

but we need all two of these plugins in our projects. when Unreal Engine builds modules, it looks at the build.cs of each plugin.

mackron commented 1 year ago

There's nothing special about the Unreal Engine build system when you break it all down - it's just compiling a bunch of .cpp files. If you were to include the same .cpp file multiple times you'd have the same issue. The DR_WAV_IMPLEMENTATION macro is designed to be defined within a source file, not as part of your build system options. You should create a separate .cpp file and do this:

#define DR_WAV_IMPLEMENTATION
#include "dr_wav.h"

Then there's no need to add anything to your build.cs file.

In any case, the DRWAV_API macro is designed to be customizable. Changing the default value as proposed in this pull request is not an appropriate way to do it because it'll change everything for everybody else who uses this library. Just define it yourself if you want to change it:

#define DRWAV_API extern inline
#define DR_WAV_IMPLEMENTATION
#include "dr_wav.h"

That said, defining everything as inline is just a messy hack to workaround the real issue, which is that you're defining the implementation multiple times which is ultimately due to using DR_WAV_IMPLEMENTATION in a way that it's not intended to be used.

6r0m commented 1 year ago

@mackron

thank you for detail explanation!

I've tried:


I have the same result (error LNK2005) when build project in the development mode.

as I understand, there is something special in the Unreal Build System. but don't understand which will happen, if you put "extern inline" for all users of your wonderful plugin)

I see a side effect for Unreal Plugins with your plugin out of the box. I realize that we can modify it, and I want to solve the problem properly in an universal way.

If you could collaborate, please help with setup non modified dr_wav.

mackron commented 1 year ago

Try creating a separate file, such as dr_wav.cpp, and then put the #define and #include lines in there with nothing else. Then add the dr_wav.cpp file to your project like you would with any other source file. Remove #define DR_WAV_IMPLEMENTATION from all other source files and build.cs scripts.

6r0m commented 1 year ago

@mackron thank you for your assistance! you are great without jokes!

I have created simple project with two plugins (consumers of the dr_wav) less than 1mb (without any code, only unreal base files with new project, plugins) https://drive.google.com/file/d/1LLW4CHnswdr1peTpyWpsZUjlPO-Xg2GR/view?usp=sharing

in the each plugin source folder I added: 1) ThirdParty/dr_wav.h 2) dr_wav.cpp with

#define DR_WAV_IMPLEMENTATION
#include "ThirdParty/dr_wav.h"

I didn't add any else, because in this case you are already could have LNK2005. just package the project through Editor or run UAT cmd.

all dr_wav.h and dr_wav.cpp are separated by the Plugin Source Folder. all dr_wav.cpp includes only own ThirdParty/dr_wav.h

still the same problem :(

mackron commented 1 year ago

I don't know how the Unreal build system handles shared source files between plugins, but clearly in this situation it's compiling in the same source file multiple times (once for each plugin). I don't know how to prevent Unreal from doing this. Maybe there's a way to compile the shared source files as a separate library and then pull it in as a dependency instead of just referencing the source files directly.