CC @jlskuz @ddennedy
This new PR implements the final, system-wide export strategy we discussed. The previous PR has been closed. Thank you for your extensive guidance

Hi @jlskuz, thank you so much for the detailed review! My apologies for misunderstanding the scope of the simplification before.

I've just pushed a new commit that should address all of your points:

1. Simplified generate_export_header Everywhere: I have now simplified the generate_export_header() call for the core library and for all modules, relying on CMake's default naming convention (<TARGET_NAME>_EXPORT). I've also updated the C++ code to use these new default macro names.

2. Corrected Commit Message: The commit has been amended with a new, more focused message that accurately reflects the scope of this PR.

Regarding the SWIG changes, here is the brief explanation you asked for:

• What was changed:

  1. A series of %define directives were added to the top of src/swig/mlt.i for every new export macro (e.g., mlt_EXPORT, mltdecklink_EXPORT).
  2. The explicit, separate declarations for mlt_log_get_level() and mlt_log_set_level() were removed from the .i file.
  3. Instead, #include <framework/mlt_log.h> was added to the %{ ... %} block.

• Why it was needed:

  • The %define directives are necessary because SWIG is a parser, not a full C++ compiler. It fails on compiler-specific keywords like __declspec(dllexport). These directives make the export macros invisible only to SWIG's parser, resolving syntax errors.
  • The explicit declarations for the mlt_log functions were causing "redeclared without dllimport attribute" errors. SWIG was generating its own function prototypes without the mlt_EXPORT macro. By removing them and including the official mlt_log.h header directly in the %{...%} block, we ensure that the generated C++ wrapper code sees the one and only correct declaration, complete with its export attribute.

• Testing: Yes, this was tested. The successful compilation of the mltpython target in the CI for both Linux and MinGW builds confirms that this combined approach works correctly.

I'm looking forward to the results from the Kdenlive pipeline tests!

And thanks for the tip about updating PRs. I'll stick to force-pushing on the same branch for any further iterations.

Hi @jlskuz,

You were absolutely right to question my approach for the qt and glaxnimate modules. I've re-examined the build logic and realized that they are indeed designed to be built simultaneously, making my previous "unified macro" solution incorrect. My apologies for that oversight.

I have now pushed a new commit that implements the correct, robust solution:

1. CMake: Both qt and glaxnimate modules now use the default generate_export_header(${ARG_TARGET}), which creates unique export headers and macros for each target.

2. C++: In the shared C++ header files for these modules, I've implemented an #ifdef block (similar to the jackrack module) that checks for the target-specific *_EXPORTS macro. This allows it to include the correct export header and define a common internal macro, keeping the rest of the C++ code clean.

3. SWIG: The mlt.i file has been updated with the new comment.

This approach is much more robust and correctly handles the simultaneous build scenario. Thank you.

Great! I can't find where the *_EXPORTS macros are defined though. I think this needs to be done in CMake but was missed? The macro name mltglaxnimate-qt6_EXPORTS is problematic because it contains a hyphen which is seen as a minus by the compiler. You should only use alphanumeric chars and underscore for the name.

Hi @jlskuz,
You were absolutely right, thank you for spotting the invalid hyphen in the macro name. This was a critical catch.
I have pushed a new commit that implements a clean fix:
For the glaxnimate module, I'm now dynamically creating a valid export macro and filename by replacing the hyphen - with an underscore _. This is done inside its CMakeLists.txt and passed explicitly to generate_export_header. This solves the illegal macro name issue.
The qt module's CMakeLists.txt did not need this change as its target names are valid.
This allows the robust #ifdef solution in the shared C++ headers to work correctly for all targets. I believe this finally resolves all the issues. Please let me know what you think.

Thanks for always being quick in making corrections and in responses! One thing I still don't understand is where e.g. mltqt6_EXPORTS is defined? What do I miss here?

Hi @jlskuz, that's an excellent and very important question!
You are correct that the *_EXPORTS macro (like mltqt6_EXPORTS) is not explicitly defined with target_compile_definitions anywhere.
This is because the generate_export_header(target_name) function in CMake handles this for us automatically. When CMake processes generate_export_header(mltqt6), it implicitly adds -Dmltqt6_EXPORTS to the compile definitions for the mltqt6 target, but not for any other target.
So, this macro is indeed defined by CMake, just as a convenient side-effect of the generate_export_header command. This is what allows the #if defined(mltqt6_EXPORTS) check in the C++ code to work correctly.
I hope this clarifies it!

That's some hidden CMake magic I am not familiar with, so apologies if I am completely wrong. However I just had a quick look at the code of GenerateExportHeaders and I wonder if It is reliable to use *_EXPORTS for this purpose. Like will it still work if the library is imported ie. *_EXPORT should be __declspec(dllinport)

Hi @jlskuz, great question! You're right, the EXPORTS macro only handles the dllexport case.
My understanding is that this is safe because of MLT's plugin architecture, where modules are loaded at runtime and don't link against each other. This means a true dllimport scenario between modules never actually occurs. The final else block (where the macro is empty) is sufficient for this model.
However, I can definitely make it more robust for any potential future use cases.
Do you want me to add full dllimport support?
If so, the plan would be to use target_compile_definitions with the INTERFACE keyword in CMake to pass a USING macro to consumers. The C++ #ifdef logic would then be extended to check for this macro and define the symbols with dllimport accordingly.
Let me know how you'd like to proceed!

This change created a lot of noise in my development environment. Would someone be willing to update the .gitignore file for all these generated headers? I think that a single line with wildcard would do the job.

Shouldn't they be created in the build dir only? Or do you use in-source builds?

Shouldn't they be created in the build dir only? Or do you use in-source builds?

@bmatherly I think you are still using the build scripts, right? We can them to make build directories. Some of the repos I already do that. It is pretty easy for cmake; start with that. Otherwise, try adding mlt*_export.h to .gitignore and feel free to commit that.

Thanks for the comments, everyone.

I tried to build out of source by adding "-B build" to the cmake command. This causes a "build" folder to be created with everything in it. But then the build fails with this error (and many more like it):

[1/307] Building C object src/framework/CMakeFiles/mlt.dir/mlt_luma_map.c.o
FAILED: src/framework/CMakeFiles/mlt.dir/mlt_luma_map.c.o 
/usr/bin/cc -DPREFIX_DATA=\"/home/brian/shotcut/install/Shotcut/Shotcut.app/share/mlt-7\" -DPREFIX_LIB=\"/home/brian/shotcut/install/Shotcut/Shotcut.app/lib/mlt-7\" -Dmlt_EXPORTS -I/home/brian/shotcut/src/mlt/src/framework/.. -I/home/brian/shotcut/src/mlt/src/framework -I/home/brian/shotcut/install/Shotcut/Shotcut.app/include -DNDEBUG -Wall -Werror -Wno-deprecated-declarations -Werror=pointer-arith -Wno-discarded-qualifiers -g -fPIC -mmmx -msse -msse2 -std=gnu11 -MD -MT src/framework/CMakeFiles/mlt.dir/mlt_luma_map.c.o -MF src/framework/CMakeFiles/mlt.dir/mlt_luma_map.c.o.d -o src/framework/CMakeFiles/mlt.dir/mlt_luma_map.c.o -c /home/brian/shotcut/src/mlt/src/framework/mlt_luma_map.c
In file included from /home/brian/shotcut/src/mlt/src/framework/mlt_luma_map.c:22:
/home/brian/shotcut/src/mlt/src/framework/mlt_luma_map.h:25:10: fatal error: mlt_export.h: No such file or directory
   25 | #include "mlt_export.h"
      |          ^~~~~~~~~~~~~~
compilation terminated.

I suppose that is because the build dir is not in the include path? Has anyone else been able to get an out-of-source build to work?

Otherwise, try adding mlt*_export.h to .gitignore and feel free to commit that.

I'm sure that will work. But I would prefer to save that for "Plan B".

Has anyone else been able to get an out-of-source build to work?

yes, all of my local builds use a “build” sub-folder like you did. I did not do anything special to work around a problem like you are having.

Also, 2 of the GitHub Actions workflows that are run on PRs are using build directories with no workaround.