[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