On Sat, 21 May 2022, Trystan Mata wrote:
Changes since the v4:
- Avoid having two mf_init function declared (missing #if !HAVE_UWP)
- Better commit message about linking with UWP
Changes since the v3:
- Library handle and function pointer are no longer in MFContext.
- If UWP is enabled, avcodec will be linked directly against MFPlat.DLL.
- MediaFoundation functions are now called like MFTEnumEx, like Martin
Storsjö suggested in his review of the v3.
I forgot to mention it on earlier versions, this patch addresses
https://trac.ffmpeg.org/ticket/9788.
diff --git a/libavcodec/mf_utils.c b/libavcodec/mf_utils.c
index eeabd0ce0b..b8756cccea 100644
--- a/libavcodec/mf_utils.c
+++ b/libavcodec/mf_utils.c
@@ -24,6 +24,7 @@
#include "mf_utils.h"
#include "libavutil/pixdesc.h"
+#include "compat/w32dlfcn.h"
HRESULT ff_MFGetAttributeSize(IMFAttributes *pattr, REFGUID guid,
UINT32 *pw, UINT32 *ph)
@@ -47,9 +48,83 @@ HRESULT ff_MFSetAttributeSize(IMFAttributes *pattr, REFGUID
guid,
#define ff_MFSetAttributeRatio ff_MFSetAttributeSize
#define ff_MFGetAttributeRatio ff_MFGetAttributeSize
-// MFTEnumEx was missing from mingw-w64's mfplat import library until
-// mingw-w64 v6.0.0, thus wrap it and load it using GetProcAddress.
-// It's also missing in Windows Vista's mfplat.dll.
+// Windows N editions does not provide MediaFoundation by default.
+// So to avoid DLL loading error, MediaFoundation is dynamically loaded except
+// on UWP build since LoadLibrary is not available on it.
+#if !HAVE_UWP
+static HMODULE mf_library = NULL;
+
+int ff_mf_load_library(void *log)
+{
+ mf_library = win32_dlopen("mfplat.dll");
+
+ if (!mf_library) {
+ av_log(log, AV_LOG_ERROR, "DLL mfplat.dll failed to open\n");
+ return AVERROR_UNKNOWN;
+ }
+
+ return 0;
+}
+
+void ff_mf_unload_library(void)
+{
+ FreeLibrary(mf_library);
+ mf_library = NULL;
+}
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".)
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.
+
+#define GET_MF_FUNCTION(ptr, func_name) \
+ ptr = (void *)GetProcAddress(mf_library, #func_name ""); \
Why the extra "" here?
+ if (!ptr) \
+ return E_FAIL;
+#else
+// In UWP (which lacks LoadLibrary), just link directly against
+// the functions - this requires building with new/complete enough
+// import libraries.
+#define GET_MF_FUNCTION(ptr, func_name) \
+ ptr = func_name; \
+ if (!ptr) \
+ return E_FAIL;
+#endif
+
+HRESULT ff_MFStartup(ULONG Version, DWORD dwFlags)
+{
+ HRESULT (WINAPI *MFStartup_ptr)(ULONG Version, DWORD dwFlags) = NULL;
+ GET_MF_FUNCTION(MFStartup_ptr, MFStartup);
For functions like this, fetching the function pointer every time it's
called probably is fine (as it's only called once on startup), but ...
+ return MFStartup_ptr(Version, dwFlags);
+}
+
+HRESULT ff_MFShutdown(void)
+{
+ HRESULT (WINAPI *MFShutdown_ptr)(void) = NULL;
+ GET_MF_FUNCTION(MFShutdown_ptr, MFShutdown);
+ return MFShutdown_ptr();
+}
+
+HRESULT ff_MFCreateSample(IMFSample **ppIMFSample)
+{
+ HRESULT (WINAPI *MFCreateSample_ptr)(IMFSample **ppIMFSample) = NULL;
+ GET_MF_FUNCTION(MFCreateSample_ptr, MFCreateSample);
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.
@@ -1034,7 +1034,11 @@ static int mf_create(void *log, IMFTransform **mft,
const AVCodec *codec, int us
return 0;
}
+#if !HAVE_UWP
+static int mf_init_encoder(AVCodecContext *avctx)
+#else
static int mf_init(AVCodecContext *avctx)
+#endif
{
MFContext *c = avctx->priv_data;
HRESULT hr;
@@ -1134,6 +1138,10 @@ static int mf_close(AVCodecContext *avctx)
ff_free_mf(&c->mft);
+#if !HAVE_UWP
+ ff_mf_unload_library();
+#endif
+
av_frame_free(&c->frame);
av_freep(&avctx->extradata);
@@ -1142,6 +1150,21 @@ static int mf_close(AVCodecContext *avctx)
return 0;
}
+#if !HAVE_UWP
+static int mf_init(AVCodecContext *avctx)
+{
+ int ret;
+
+ if ((ret = ff_mf_load_library(avctx)) == 0) {
+ if ((ret = mf_init_encoder(avctx)) == 0) {
+ return 0;
+ }
+ }
+ mf_close(avctx);
+ return ret;
+}
+#endif
+
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?
// Martin
_______________________________________________
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".
