-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
win-mf: Add Windows Media Foundation Capture Device and Intel NPU AI features #10471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
fc46b24
to
0584cda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a more descriptive commit message.
In C++ for variables/functions/identifiers that remain in C++, we prefer camelCase over snake_case.
Is it possible to, at minimum, split this into two commits, or perhaps two PRs:
- Add Media Foundation
- Add the NPU features
c8280fd
to
dcea1bb
Compare
|
The deps updated with obsproject/obs-deps#242 was merged, so this is no longer a draft, but it will need to be rebased. |
So I was able to get this building with current deps, however it does not work on Windows 10. This is due to usage of IMFCameraControlMonitor. I assume this is expected, but I'm noting it for posterity.
|
I launched this PR on a Windows 11 Insider build, and got a crash when adding the source. Snippets:
Full logs (session and crash): |
It looks like the Crash logs are from the Windows 10 build, are there any from your attempt at adding the source on Windows 11? |
This builds and runs on Windows 11 23H2 with the Windows 11 SDK (10.0.22621.0). The "Video Capture Device 2" is available and correctly loads my webcam. However, in Debug configuration, it spams the log with the following: log spam
Given the log spam, I wonder if something is not appropriately gated. Appropriately, since the hardware I'm testing on at the moment does not have an NPU, I do not see the NPU features (background removal, eye gaze correction, etc.). I will re-check on capable hardware at a later time. |
The current implementation of this PR will also need to bump the minimum Windows SDK version to 22621 due to the usage of IMFCameraControlNotify. obs-studio/cmake/windows/compilerconfig.cmake Lines 45 to 47 in 8b975c4
|
677be88
to
2b117ff
Compare
6598aae
to
eca2e43
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this PR has some files generated using an older version of obs-plugintemplate
. Those files should be replaced using a newer version of the obs-plugintemplate
.
New code files should have appropriate license headers (GPLv2+).
2c6ef1a
to
bbe146a
Compare
plugins/win-mf/libmfcapture/source/DeviceControlChangeListener.cpp
Outdated
Show resolved
Hide resolved
plugins/win-mf/libmfcapture/source/DeviceControlChangeListener.hpp
Outdated
Show resolved
Hide resolved
@@ -87,4 +87,5 @@ add_obs_plugin(text-freetype2) | |||
add_obs_plugin(vlc-video WITH_MESSAGE) | |||
add_obs_plugin(win-capture PLATFORMS WINDOWS) | |||
add_obs_plugin(win-dshow PLATFORMS WINDOWS) | |||
add_obs_plugin(win-mf PLATFORMS WINDOWS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo for OBS: are we satisfied using win-mf
for the plugin name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if the name were spelt-out - the abbreviation is ambiguous from the outside and doesn't provide any good hint at what's inside the can.
win-mediafoundation
would be acceptable, personally I don't like the platform prefixes and rather let the technology speak for itself (i.e., a plugin called "obs-mediafoundation" contains support for "Media Foundation", which just happens to be only available on Windows).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incidentally, we used to have a module named win-mf
that provided Media Foundation encoders on Windows, so it might not be a good idea to use the same name for a capture plugin.
VS_VERSION_INFO VERSIONINFO | ||
FILEVERSION 30,0,1,0 | ||
BEGIN | ||
BLOCK "StringFileInfo" | ||
BEGIN | ||
BLOCK "040904B0" | ||
BEGIN | ||
VALUE "CompanyName", "Intel Corporation" | ||
VALUE "FileDescription", "OBS Video Capture module" | ||
VALUE "FileVersion", "30.0.1" | ||
VALUE "ProductName", "OBS Studio" | ||
VALUE "ProductVersion", "30.0.1" | ||
VALUE "LegalCopyright", "Copyright (C) 2023, Intel Corporation" | ||
VALUE "InternalName", "win-mf" | ||
VALUE "OriginalFilename", "win-mf" | ||
END | ||
END | ||
BLOCK "VarFileInfo" | ||
BEGIN | ||
VALUE "Translation", 0x409,1200 | ||
END | ||
END |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than hard-coding this, we tend to use an .rc.in
file and have CMake process that into an .rc
file, with few exceptions. @PatTheMav ?
I'm not sure if the rest of the file contents here are used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, almost all values are defined by CMake project definition, so the file should be generated by CMake as well. Particularly version numbers will be fully dynamic.
@@ -0,0 +1,23 @@ | |||
// {{NO_DEPENDENCIES}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the .rc
file, I'm not sure what these are used for.
@@ -0,0 +1,20 @@ | |||
/* mf-plugin.cpp */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an appropriate license header with copyright info.
class VideoBufferLock { | ||
|
||
public: | ||
VideoBufferLock(IMFMediaBuffer *pBuffer) | ||
{ | ||
m_spBuffer = pBuffer; | ||
m_spBuffer->QueryInterface(IID_PPV_ARGS(&m_sp2DBuffer)); | ||
} | ||
|
||
~VideoBufferLock() | ||
{ | ||
UnlockBuffer(); | ||
m_spBuffer = nullptr; | ||
m_sp2DBuffer = nullptr; | ||
} | ||
|
||
HRESULT LockBuffer(LONG lDefaultStride, DWORD dwHeightInPixels, BYTE **ppbScanLine0, LONG *plStride) | ||
{ | ||
HRESULT hr = S_OK; | ||
if (m_sp2DBuffer) { | ||
hr = m_sp2DBuffer->Lock2D(ppbScanLine0, plStride); | ||
} else { | ||
BYTE *pData = NULL; | ||
hr = m_spBuffer->Lock(&pData, NULL, NULL); | ||
if (SUCCEEDED(hr)) { | ||
*plStride = lDefaultStride; | ||
if (lDefaultStride < 0) { | ||
*ppbScanLine0 = pData + abs(lDefaultStride) * (dwHeightInPixels - 1); | ||
} else { | ||
*ppbScanLine0 = pData; | ||
} | ||
} | ||
} | ||
|
||
m_bLocked = (SUCCEEDED(hr)); | ||
return hr; | ||
} | ||
|
||
void UnlockBuffer() | ||
{ | ||
if (m_bLocked) { | ||
if (m_sp2DBuffer) { | ||
m_sp2DBuffer->Unlock2D(); | ||
} else { | ||
m_spBuffer->Unlock(); | ||
} | ||
m_bLocked = FALSE; | ||
} | ||
} | ||
|
||
private: | ||
wil::com_ptr_nothrow<IMFMediaBuffer> m_spBuffer; | ||
wil::com_ptr_nothrow<IMF2DBuffer> m_sp2DBuffer; | ||
|
||
BOOL m_bLocked = FALSE; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if we should define this in a separate file. @PatTheMav ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, any class definition should get its own separate source file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved the code into a separate file, but should it also have a header file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For new files, use appropriate license headers (GPLv2+).
d1d4308
to
c014990
Compare
9ff3014
to
d9c667d
Compare
3b22ef5
to
e925aa4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newly added files should have appropriate and compatible license headers (GPLv2+).
@@ -0,0 +1,21 @@ | |||
class CriticalSection { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add appropriate license headers (GPLv2+).
@@ -0,0 +1,58 @@ | |||
#include <mfidl.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add appropriate license headers (GPLv2+).
libmfcapture/source/DeviceControlChangeListener.cpp | ||
libmfcapture/source/mfcapture.cpp | ||
libmfcapture/source/PhysicalCamera.cpp | ||
libmfcapture/source/VideoBufferLock.cpp | ||
libmfcapture/source/CriticalSection.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alphabetize these.
bool _deactivateWhenNotShowing = false; | ||
bool _flip = false; | ||
bool _active = false; | ||
bool _autorotation = true; | ||
bool _firstframe = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid prefixing variables with _
. While technically permitted by the spec in these cases, it's better to avoid altogether.
|
||
void MediaFoundationSourceLoop(); | ||
|
||
void static __stdcall _OnVideoData(void *pData, int Size, long long llTimestamp, void *pUserData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The C++ standard reserves identifiers that begin with an underscore and a capital letter.
static DWORD CALLBACK MediaFoundationSourceThread(LPVOID ptr) | ||
{ | ||
MediaFoundationSourceInput *input = (MediaFoundationSourceInput *)ptr; | ||
os_set_thread_name("win-MediaFoundation: MediaFoundationSourceThread"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os_set_thread_name("win-MediaFoundation: MediaFoundationSourceThread"); | |
os_set_thread_name("win-mediafoundation: MediaFoundationSourceThread"); |
if (!properties.empty()) { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the intent of this block?
|
||
inline bool MediaFoundationSourceInput::Activate(obs_data_t *settings) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary blank line.
m_spBuffer = nullptr; | ||
m_sp2DBuffer = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these need to be manually set to nullptr
in the destructor? I thought wil::com_ptr_nothrow
and wil::com_ptr
were smart pointers that should clean up after themselves.
VALUE "ProductName", "${OBS_PRODUCT_NAME}" | ||
VALUE "ProductVersion", "${OBS_VERSION_CANONICAL}" | ||
VALUE "Comments", "${OBS_COMMENTS}" | ||
VALUE "LegalCopyright", "${OBS_LEGAL_COPYRIGHT}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may need to hardcode this line to use a specific copyright attribution.
Description
PR is linked with: obsproject/obs-deps#242 (Requires wil headers to build)
Windows 11 is also required for this PR to build and Windows 11 SDK (10.0.22000.0).
This is a draft implementation of Intel AI features using the NPU(=”Intel AI Boost”) on Intel Core Ultra(a.k.a Meteor Lake).
It enables Background Blur(Standard and Portrait), Background Removal, Auto Framing and Eye Contact correction over new Microsoft Effect Package and Media Foundation.
Because this capture device is based on Media Foundation, we created separate “Video Capture Device 2”. We are thinking that it’s better to integrate it into the existing DShow based Video Capture Device in order to not to make end-users confused.
Thus, we need help for (1) overall design guidance how we can integrate - how we can kick off MF based device instead of DShow base, (2) Any missing features to get merged? Because we focus on prototyping new AI features, wondering if we’re missing something on the MF device.
BTW, this feature only works on OEM integrated web-cam, external USB camera is planned to be supported for the future, but not yet supported as of March 2024, no ETA announced.
Screen shots:

GPU 99%, NPU 30% over 3D Mark, w/ AI features are enabled
New UI for AI – Background Blur, Removal, Auto Framing, Eye Contact by NPU(=Intel AI Boost)

they only appear on MTL, otherwise this section would be skipped.
Motivation and Context
Utilize Intel NPU for OBS and be able to add AI capabilities in OBS for Intel devices.
How Has This Been Tested?
This has been tested on:
Types of changes
Checklist: