search

PortAudio / portaudio

PortAudio is a cross-platform, open-source C language library for real-time audio input and output.
Other
1.37k stars 286 forks source link

Fix issues with DirectSound thread cleanup code #922

Open dechamps opened 1 month ago

dechamps commented 1 month ago

This PR fixes a couple of issues with the way the DirectSound Host API cleans up its streaming thread, and also simplifies the code as a nice side effect.

Details can be found in the commit messages.

philburk commented 1 month ago

This PR fixes a couple of issues

Thanks for offering the fix. Could you create an Issue, or two, so that we can discuss the bug?

RossBencina commented 1 month ago

I agree that the portaudio code is not consistent with the microsoft API docs at https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/endthread-endthreadex?view=msvc-170 I can see where the confusion comes from:

It also looks like the old code was not consistently using the macros supplied for creating and closing the thread (the macros exist for the purpose of cygwin compatibility when beginthreadex/endthreadex is not used). Ideally I think the macros should be retained and fixed rather than eliminating them, since they clearly abstract the differences between the two runtimes.

dechamps commented 4 weeks ago

Could you create an Issue, or two, so that we can discuss the bug?

Done in #925 and #926.

Ideally I think the macros should be retained and fixed rather than eliminating them, since they clearly abstract the differences between the two runtimes.

Done by preserving the CLOSE_THREAD_HANDLE macro.

(I would argue that these macros don't really make sense in practice and we should get rid of them, because they are abstracting away differences that don't really exist - the official docs of _beginthreadex() do not even try to hide the fact that it's really just a thin wrapper around CreateThread(), and that the return value is really just a Win32 thread HANDLE in disguise, as evidenced by the fact that we're being told to pass it to CloseHandle(). Really the only difference is which function is used to create the thread - CreateThread() vs. _beginthreadex() and everything else is the same. Given this, the macros just seem to obscure the code for no apparent benefit. But I guess I don't have strong feelings about this.)