[FFmpeg-devel] [PATCH v5] avcodec/mfenc: Dynamically load MFPlat.DLL

[Trystan Mata]

Firstly, thank you for your review.

 > You shouldn't use win32_dlopen directly (there's no preexisting code 
that
 > does that). If you want to use the helpers from w32dlfcn.h, call dlopen
 > and dlclose, which then redirect to those functions. But here I don't 
see
 > any need to use that wrapper when you probably could just call 
LoadLibrary
 > directly. (There's no need for using wchar APIs and long path 
handling and
 > all that, when it's just given the hardcoded path "mfplat.dll".)

Ok, I will switch totally only LoadLibrary calls.

 > Secondly, this won't probably work as intended if you have two mfenc
 > instances running at the same time - you'd load the library twice but 
only
 > unload it once. To work around that, you can either add some sort of
 > reference counting around the library, or keep the library reference 
in a
 > per-encoder struct.
 >
 > In this case I think I would prefer to keep it in a struct.

 > This is called once per frame at least, right? For such cases I think it
 > would be much better if we'd keep the function pointer in a struct 
(which
 > then would be owned/contained in MFContext) so we don't need to look 
it up
 > every time.

I completely missed this. I'm not used to dynamic loading.
I will fix this by putting back the library handle and function ptrs in the
MFContext struct.

I think I will put each looked up symbol in the struct. So every MF 
function is loaded the same way.

 > This feels a bit messy with the same function defined differently 
between
 > the desktop/UWP cases. Wouldn't it be more straightforward to just keep
 > all the code for the desktop case, and add ifdefs within e.g.
 > ff_mf_load_library and ff_mf_unload_library, so that those functions are
 > simple no-ops if building for UWP?

I will fix that, thank you again for review.

// Trystan