On Mon, 31 Mar 2025 at 02:50, softworkz .
<softworkz-at-hotmail....@ffmpeg.org> wrote:
>
>
>
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Jan
> > Ekström
> > Sent: Montag, 31. März 2025 00:05
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v2] w32pthreads: add support for
> > setting thread name
> >
> > On Tue, Mar 4, 2025 at 5:14 PM Kacper Michajłow <kaspe...@gmail.com>
> > wrote:
> > >
> > > Signed-off-by: Kacper Michajłow <kaspe...@gmail.com>
> > > ---
> > > compat/w32pthreads.h | 30 ++++++++++++++++++++++++++++++
> > > libavutil/thread.h | 2 ++
> > > 2 files changed, 32 insertions(+)
> > >
> > > diff --git a/compat/w32pthreads.h b/compat/w32pthreads.h
> > > index fd6428e29f..8d5b4729fa 100644
> > > --- a/compat/w32pthreads.h
> > > +++ b/compat/w32pthreads.h
> > > @@ -44,6 +44,7 @@
> > > #include "libavutil/internal.h"
> > > #include "libavutil/mem.h"
> > > #include "libavutil/time.h"
> > > +#include "libavutil/wchar_filename.h"
> > >
> > > typedef struct pthread_t {
> > > void *handle;
> > > @@ -209,4 +210,33 @@ static inline int pthread_setcancelstate(int
> > state, int *oldstate)
> > > return 0;
> > > }
> > >
> > > +static inline int win32_thread_setname(const char *name)
> > > +{
> > > + typedef HRESULT (WINAPI *SetThreadDescriptionFn)(HANDLE, PCWSTR);
> > > + SetThreadDescriptionFn pSetThreadDescription;
> > > + HRESULT hr;
> > > + wchar_t *wname;
> > > +
> > > +#if !HAVE_UWP
> > > + HMODULE kernel32 = GetModuleHandleW(L"kernel32.dll");
> > > + if (!kernel32)
> > > + return AVERROR(ENOSYS);
> > > + pSetThreadDescription = (SetThreadDescriptionFn)
> > > + GetProcAddress(kernel32, "SetThreadDescription");
> > > + if (!pSetThreadDescription)
> > > + return AVERROR(ENOSYS);
> > > +#else
> > > + WINBASEAPI HRESULT WINAPI
> > > + SetThreadDescription(HANDLE hThread, PCWSTR lpThreadDescription);
> > > + pSetThreadDescription = &SetThreadDescription;
> > > +#endif
> > > +
> > > + if (utf8towchar(name, &wname) < 0)
> > > + return AVERROR(ENOMEM);
> > > +
> > > + hr = pSetThreadDescription(GetCurrentThread(), wname);
> > > + av_free(wname);
> > > + return SUCCEEDED(hr) ? 0 : AVERROR(EINVAL);
> > > +}
> > > +
> >
> > I can only comment on the non-UWP side of things, but in general the
> > code seems fine. I guess this function definition has not been in
> > mingw-w64 etc for long enough to hope it would always be there and
> > thus we need to define it (similar to pf_DXGIGetDebugInterface)?
> >
> > The only question mark that is left is whether this functionality is
> > actually in kernel32 or kernelbase. When I first saw
> > https://stackoverflow.com/questions/62243162/how-to-access-
> > setthreaddescription-in-windows-2016-server-version-1607
> > I more or less thought of it as a possibly fluke or so, but then
> > apparently mingw-w64 went for kernelbase as well in winpthreads?
> > https://sourceforge.net/p/mingw-w64/mailman/message/58829419/
> >
> > What it seems like is that kernel32 works for (at the very least) 20+
> > versions of win10+, and older stuff such as the still-supported server
> > 2016 requires direct usage of kernelbase?
> >
> > Jan
> > _______________________________________________
>
> Hi Jan,
>
> the actual implementation is (and has always been) in kernelbase.dll.
>
> It was introduced there with Windows SDK 10.0.14393 (Windows OS 1607).
> At some point between SDKs 14393 and 18362 (don't have the in-betweens
> installed), a forwarding export had been added to kernel32.dll (try: dumpbin
> /exports c:\windows\system32\kernel32.dll | findstr SetThreadDescription).
>
> So, using kernelbase.dll will cover more Windows versions than with
> kernel32.dll.
I don't think the few unsupported versions that would be covered by
this are worth the added complexity in the code. Maybe it will never
be moved out of kernelbase.dll, but our entry point is kernel32.dll as
documentation states. I don't see the need to check both and actually
we would need to check the api set too. Which is actually the first
forwarded target from kernel32.dll
> dumpbin /exports c:\windows\system32\kernel32.dll | findstr
> SetThreadDescription
1431 596 SetThreadDescription (forwarded to
api-ms-win-core-processthreads-l1-1-3.SetThreadDescription)
Chromium uses kernel32.dll
https://chromium.googlesource.com/chromium/src/+/1a4233541ca76ad2cfda2630d44744b5d29dd73a/base/threading/platform_thread_win.cc#222
mpv uses kernel32.dll
https://github.com/mpv-player/mpv/blob/52a95d91bd46ffffbded9c5440db6bdd4dcada65/osdep/threads-win32.h#L166
winpthreads uses kernelbase.dll
https://github.com/mingw-w64/mingw-w64/blob/90da6c6535c503159e952dc8b44f8778bed6f622/mingw-w64-libraries/winpthreads/src/misc.c#L51
vlc uses both api-ms-win-core-processthreads-l1-1-3.dll and
kernelbase.dll
https://code.videolan.org/videolan/vlc/-/blob/7616581625670b94381f78e6cf3021b5625551df/src/win32/thread.c#L726-728
I think using kernel32.dll on win32 target is fine. Though the
annoying part is the UWP target and I probably will remove support for
UWP here. There is small window 1507-1607 when this API was not
available and since UWP doesn't allow LoadLibrary or GetModuleHandle
the only way to detect if this API is available is to use
QueryOptionalDelayLoadedAPI(). BUT it works only if the library is
delayed loaded in the first place, so it needs /DELAYLOAD:kernel32.dll
when linking, but it's not something we want to force UWP as a
library...
- Kacper
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
