[dimula73]

Instead of relying of the includes order we should properly
check if the function is actually provided by our version
of pthread library.

The patch basically reverts an original hack to resolve the
issue in msys2 and implements a proper detection mechanism
for the presence of nanosleep() in the pthread library

[dimula73]

There is still an unorthodox usage of WIN_PTHREADS_TIME_H in mlt_types.h, which dates back from 2015. Theoretically, it could also be replaced with HAVE_PTHREAD_NANOSLEEP, though we would then have to add a public-interface definition for libmlt, like that:

target_compile_definitions(mlt PUBLIC "HAVE_PTHREAD_NANOSLEEP=${HAVE_PTHREAD_NANOSLEEP}")

Though I'm not sure if such a change is acceptable or not

[ddennedy]

unorthodox usage of WIN_PTHREADS_TIME_H in mlt_types.h

It used to exist even if not in a standard. Google search on it first hit leads me to;
https://github.com/tonytheodore/winpthreads/blob/master/include/pthread_time.h

Since it is unorthodox it is OK to change it IMO.

[ddennedy]

I approve the change but wait to see what you do about mlt_types.h

[dimula73]

Hi, @ddennedy!

I did a small test here and it seems like it is not easy to add HAVE_PTHREAD_NANOSLEEP into a global definitions interface of the entire library (.pc and Mlt7Config.cmake files will have to be adjusted for that manually).

So, we have three options here:

1. Merge this patch as it is and ignore the fact that the implementation and prototype for MLT's nanosleep() are guarded by different definitions.

2. Revert nanosleep()'s implementation to be guarded by the undocumented WIN_PTHREADS_TIME_H and just include pthread_time.h into the translation unit as well (this version will be the nearest to the original implementation)

3. Add a global mlt_config.h file, which would be generated by CMake and provide this macro for mlt_types.h.

[ddennedy]

nanosleep() for win32 is only supposed to be used internally but was added to mlt_types.h for convenience. We can add a src/win32/mlt_win32.h, but then it needs to be conditionally included wherever nanosleep is called. Or, your option 2 if __MINGW32__

[ddennedy]

This was added for a reason not too long ago. See git blame. Explain why it is no longer needed.

[dimula73]

Github lost my long comment that I was writing, so I will be short :(

The commit introducing that is: 64aa543

It has no explanation why it is needed, and what issue it is supposed to fix. Every target having this variable does already link to Threads::Threads so the hack looks unnecessary.

[ddennedy]

Please address the comments.

[dimula73]

Hi, @ddennedy!

I plan to return to this patch some time soon, I was just busy with other things (and then the vacation came :) )

[Copilot]

🟡 Changes recommended

Two public-header regressions break installed Windows header lookup and the fallback nanosleep() type declaration for C consumers.

2 open issues

• Fix installed mlt_config.h include path · New
• Declare timespec before guarded pthread include · New

What changed in this PR

This PR replaces MLT’s include-order-dependent Windows nanosleep() workaround with a CMake capability check against the pthread library.

Changes:

• Detects pthread-provided nanosleep() and generates an installed configuration header.
• Uses the detected capability to guard compatibility declarations and implementation.
• Removes inferred pthread library paths and updates thread and SDL2 target linking.

File	Description
src/​win32/​win32.c	Guards fallback nanosleep() using the detected capability.
src/​modules/​sdl2/​CMakeLists.txt	Removes explicit inferred pthread linking.
src/​modules/​avformat/​CMakeLists.txt	Removes explicit inferred pthread linking.
src/​melt/​CMakeLists.txt	Updates MinGW thread and SDL2 target linking.
src/​framework/​mlt_types.h	Uses configuration-based guards for Windows declarations.
src/​framework/​mlt_config.h.in	Adds the pthread nanosleep() capability macro template.
src/​framework/​CMakeLists.txt	Checks availability and generates and installs the configuration header.
CMakeLists.txt	Removes pthread library path inference from the iconv path.

_{Review effort: Balanced}

[dimula73]

Hi, @ddennedy!

I've scraped the previous approach and implemented a full version with mlt_config.h which stores the status of the pthread library available on the host system.

[dimula73]

I will also check consumption of this package on Windows now... I've check the building only

[dimula73]

I will also check consumption of this package on Windows now... I've check the building only

Yes, consumption in Krita with on Windows works fine (llvm-mingw toolchain).