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
_______________________________________________
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".

Reply via email to