Handling KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE on avscamera DMFT by welljeng · Pull Request #1325 · microsoft/Windows-driver-samples

welljeng · 2025-09-24T06:18:52Z

In avscamera DMFT, its KsEvent and KsProperty functions handle KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE property to allow DMFT to behave appropriately in response of various camera streaming profile selected.

…ng behavior on camera DMFT.

chrilaMSFT · 2025-09-25T20:47:24Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+    }
 done:
-    return hr;
+    return S_OK;


S_OK

return hr

If source device return failed on the following, hr won't hold S_OK here. I would assume if source device returned failed, but the DMFT itself supports the KsEvent, it should reply OK.

https://github.com/welljeng/Windows-driver-samples/blob/4a380c3ff93f2227e40865cb472638954b3c31a1/avstream/avscamera/DMFT/AvsCameraDMFT.cpp#L1107

The issue is with style there is multiple exit points in this new function while the rest of the code has single exit point I would keep the styling the same as the rest of the code

chrilaMSFT · 2025-09-25T20:50:09Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+        && (pEvent->Id == KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE))
+    {
+
+        m_hSelectedProfileKSEvent = nullptr;


m_hSelectedProfileKSEvent

use WIL handle object looks like this code leaks handles

chrilaMSFT · 2025-09-25T20:50:29Z

IFACEMETHODIMP CMultipinMft::KsEvent(

no lock?

Refers to: avstream/avscamera/DMFT/AvsCameraDMFT.cpp:1055 in 4a380c3. [](commit_id = 4a380c3, deletion_comment = False)

chrilaMSFT · 2025-09-25T20:51:10Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+            FALSE,
+            DUPLICATE_SAME_ACCESS) == false)
+        {
+            return E_INVALIDARG;


return E_INVALIDARG;

match coding style of the rest of the code with macro checks

chrilaMSFT · 2025-09-25T21:12:09Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+    if (pProperty->Id == KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE)
+    {
+        DMFTCHECKHR_GOTO(ProfilePropertyHandler(pProperty, ulPropertyLength, pvPropertyData, ulDataLength, pulBytesReturned), done);
+        goto done;


NIT: only use goto for error case

chrilaMSFT · 2025-09-25T21:12:45Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+        }
+        if (m_isProfileDDISupportedInBaseDriver.value_or(true))
+        {
+            m_hSelectedProfileKSEventSentToDriver = CreateEventExW(


m_hSelectedProfileKSEventSentToDriver

use WIL event

chrilaMSFT · 2025-09-25T21:13:31Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+    UNREFERENCED_PARAMETER(ulPropertyLength);
+    HRESULT hr = S_OK;
+
+    if (pProperty->Flags & KSPROPERTY_TYPE_SET)


chrilaMSFT · 2025-09-25T21:15:37Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

@@ -1293,6 +1371,102 @@ IFACEMETHODIMP CMultipinMft::Shutdown(
    return ShutdownEventGenerator();


Need to handle canceling event and outstanding controls in shutdown

https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/ksproperty-cameracontrol-extended-profile

Referring to the doc. above, extended profile is not cancelable.

chrilaMSFT · 2025-09-25T21:17:17Z

avstream/avscamera/DMFT/AvsCameraDMFT.vcxproj

    <ClCompile>
      <PreprocessorDefinitions>%(PreprocessorDefinitions);UNICODE;MF_WPP;SECURITY_WIN32;MFT_UNIQUE_METHOD_NAMES;MF_DEVICEMFT_ALLOW_MFT0_LOAD</PreprocessorDefinitions>
      <AdditionalIncludeDirectories>$(IntDir);%(AdditionalIncludeDirectories);..\..\common;..\common</AdditionalIncludeDirectories>
+      <LanguageStandard>stdcpp17</LanguageStandard>


stdcpp17

I would recommend minimizing the $(Configuration)|$(Platform) unique configs move the common ones to a global setting

chrilaMSFT · 2025-09-25T21:18:43Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+                }
+                SetEvent(m_hSelectedProfileKSEvent);
+                m_hSelectedProfileKSEvent = nullptr;
+            }


Wait should not block

Consider using MFPutWaitingWorkItem to signal the m_hSelectedProfileKSEvent (using m_hSelectedProfileKSEventSentToDriver as the event handle to the API in question). This doesn't afford you the ability to use a timeout, but the spec requires drivers to signal the event both in the case of success or if the operation can't complete.

chrilaMSFT · 2025-09-25T21:19:24Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+
+    /// PDMFT only cares about ExtendedCameraControls, all others 
+    /// are just blindly forwarded to the upstream DMFTs. 
+    if (!IsEqualCLSID(pProperty->Set, KSPROPERTYSETID_ExtendedCameraControl))


IsEqualCLSID

requires a lock

chrilaMSFT · 2025-09-25T21:21:34Z

avstream/avscamera/DMFT/AvsCameraDMFT.h

    PWCHAR                       m_SymbolicLink;
+    HANDLE                       m_hSelectedProfileKSEvent;
+    HANDLE                       m_hSelectedProfileKSEventSentToDriver;
+    std::optional<bool>          m_isProfileDDISupportedInBaseDriver;


m_isProfileDDISupportedInBaseDriver

this should be a static property of the DMFT since it ideally is developed with the camera/driver it is being used with

I am hesitate of make it static since if the same DMFT is being applied on multiple camera DMFT chain, all the DMFT instances will share the same m_isProfileDDISupportedInBaseDriver value, which might not be the ideal case.
Correct me if I were wong.

We don't want drivers querying static info if they are shipped with specific HW if this changes dynamically it is probably a bug

chrilaMSFT · 2025-09-25T21:22:23Z

avstream/avscamera/DMFT/AvsCameraDMFT.h


 interface IDirect3DDeviceManager9;

+constexpr int kMAX_WAIT_TIME_DRIVER_PROFILE_KSEVENT = 3000;// ms, amount of time to wait for the profile DDI KsEvent sent to the driver


kMAX_WAIT_TIME_DRIVER_PROFILE_KSEVENT

I think pipeline should handle max wait

The constant is removed due to migrate to MFPutWaitingWorkItem instead of blocking wait.

chrilaMSFT · 2025-09-25T21:23:14Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+// TODO: required to avoid bug OS bug 36971659 in extended property handling that introduces a 16 bytes cookie
+typedef struct
+{
+    //byte cookieBuffer[16];


remove commented out code

chrilaMSFT · 2025-09-25T21:23:31Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

    DMFTCHECKHR_GOTO(pAttributes->SetUINT32( MF_SA_D3D_AWARE, TRUE ), done);
    DMFTCHECKHR_GOTO(pAttributes->SetString( MFT_ENUM_HARDWARE_URL_Attribute, L"SampleMultiPinMft" ),done);
    m_spAttributes = pAttributes;
+


NIT: remove

SangChoe · 2025-09-25T22:21:18Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

-        pvPropertyData,
-        ulDataLength,
-        pulBytesReturned),done);
+    if (pProperty->Id == KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE)


if (pProperty->Id == KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE)

This is incorrect. You're only checking against the Id. The combination should always be Set + Id to determine the control.

In this particular case, you're going to intercept any control whose Id is 34 regardless of whether that Id belongs to the KSPROPERTYSETID_ExtendedCameraControl or not. I think what you meant to do here is to intercept and call ProfilePropertyHandler only if the Set == KSPROPERTYSETID_ExtendedCameraControl && Id == KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE.

Excellent poinnt!!

SangChoe · 2025-09-25T22:23:10Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+
+            m_selectedProfileId.Type = pProfile->ProfileId;
+            m_selectedProfileId.Index = pProfile->Index;
+            m_selectedProfileId.Unused = pProfile->Reserved;


This information is never forwarded to the driver so the SentToDriver event will never signal.

2. On event handling, removed blocking wait, use MFPutWaitingWorkItem instead.

…ljeng/Windows-driver-samples into user/chengfrank/DMFTFaceAuth

chrilaMSFT · 2025-10-15T20:50:47Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+    KSCAMERA_EXTENDEDPROP_HEADER header;
+} KSCAMERA_EXTENDEDPROP_HEADER_BUFFERED, * PKSCAMERA_EXTENDEDPROP_HEADER_BUFFERED;
+
 //


Remove it is defined in ksmedia.h which should be already included

chrilaMSFT · 2025-10-15T20:55:08Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

 }
+
+HRESULT CMultipinMft::ProfileAsyncResultCallback(_In_ IMFAsyncResult* pResult)
+{


chrilaMSFT · 2025-10-15T21:00:19Z

avstream/avscamera/DMFT/AvsCameraDMFT.h

+    wil::unique_event_nothrow    m_hSelectedProfileKSEventSentToDriver;
+    std::optional<bool>          m_isProfileDDISupportedInBaseDriver;
+    SENSORPROFILEID              m_selectedProfileId;
+    MFAsyncCallback<CMultipinMft>m_profileCallback;


This code is going to break if parent class goes away

chrilaMSFT · 2025-10-15T21:01:23Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

-        ulDataLength,
-        pBytesReturned), done);
+    // handle the event if it is to set profile
+    if (pEvent != nullptr


what happens if this gets called twice before the first completes?

chrilaMSFT · 2025-10-15T21:11:57Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+        hr = m_spIkscontrol->KsEvent(pEvent,
+            ulEventLength,
+            pEventData,
+            ulDataLength,
+            pBytesReturned);
+		DMFTCHECKHR_GOTO(hr, done);


Suggested change

hr = m_spIkscontrol->KsEvent(pEvent,

ulEventLength,

pEventData,

ulDataLength,

pBytesReturned);

DMFTCHECKHR_GOTO(hr, done);

DMFTCHECKHR_GOTO(m_spIkscontrol->KsEvent(pEvent, ulEventLength, pEventData, ulDataLength, pBytesReturned), done);

chrilaMSFT · 2025-10-15T21:15:57Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+            hr = m_hSelectedProfileKSEventSentToDriver.create();
+            DMFTCHECKHR_GOTO(hr, done);


DMFTCHECKHR_GOTO(m_hSelectedProfileKSEventSentToDriver.create(), done);

chrilaMSFT · 2025-10-15T21:18:07Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+            driverEventData.EventHandle.Event = m_hSelectedProfileKSEventSentToDriver.get();
+            DMFTRACE(DMFT_GENERAL, TRACE_LEVEL_INFORMATION, "Handling profile set KsEvent, created profile KsEvent handle for driver: %p", m_hSelectedProfileKSEventSentToDriver.get());
+            // defer to source device
+            HRESULT hr2 = m_spIkscontrol->KsEvent(pEvent, ulEventLength, (void*)(&driverEventData), ulDataLength, pBytesReturned);


this code will show success while actually failing

chrilaMSFT · 2025-10-15T21:22:03Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+        {
+            hr = m_hSelectedProfileKSEventSentToDriver.create();
+            DMFTCHECKHR_GOTO(hr, done);
+            KSEVENTDATA driverEventData = {};


Missing size and data from original call

chrilaMSFT · 2025-10-15T21:23:55Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

@@ -1039,14 +1058,57 @@ IFACEMETHODIMP CMultipinMft::KsEvent(
    Implements IKSProperty::KsEvent function.


Why do you need to handle the event? It complicates the sample, I would just grab the control set it would make the change a lot smaller.

chrilaMSFT · 2025-10-15T21:26:15Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+done:
+    DMFTRACE(DMFT_GENERAL, TRACE_LEVEL_INFORMATION, "%!FUNC! exiting %x = %!HRESULT!", hr, hr);
+    return hr;
+} CATCH_RETURN()


Catch returns are only needed on the external com interfaces since the caller cannot not handle exceptions from external module.

chrilaMSFT · 2025-10-15T21:27:17Z

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

+            pEventData,
+            ulDataLength,
+            pBytesReturned);
+		DMFTCHECKHR_GOTO(hr, done);


[](http://example.com/codeflow?start=0&length=2)

remove tabs

Frank Cheng and others added 4 commits September 24, 2025 14:18

Handling KsEvent, KSCAMERAPROFILE_FaceAuth_Mode for different streami…

68cc5fa

…ng behavior on camera DMFT.

Remove Wil dependancy from AvsCameraDMFT.vcxproj to fix CI/CD issue.

bc6eda8

Eliminate WIL dependancy on AvsCameraDMFT.h/cpp.

93a1488

Merge branch 'main' into user/chengfrank/DMFTFaceAuth

4a380c3

welljeng changed the title ~~Handling KsEvent, KSCAMERAPROFILE_FaceAuth_Mode for different streami…~~ Handling KSCAMERAPROFILE_FaceAuth_Mode on avscamera DMFT Sep 25, 2025

welljeng changed the title ~~Handling KSCAMERAPROFILE_FaceAuth_Mode on avscamera DMFT~~ Handling KSPROPERTY_CAMERACONTROL_EXTENDED_PROFILE on avscamera DMFT Sep 25, 2025

welljeng marked this pull request as ready for review September 25, 2025 18:02

welljeng requested a review from a team as a code owner September 25, 2025 18:02

chrilaMSFT reviewed Sep 25, 2025

View reviewed changes

chrilaMSFT requested review from a team and jiteshkris September 25, 2025 21:26

SangChoe reviewed Sep 25, 2025

View reviewed changes

Frank Cheng and others added 3 commits October 1, 2025 14:29

Rollback to use WIL.

0960a1e

1. PR review results enhancements.

4ff5b33

2. On event handling, removed blocking wait, use MFPutWaitingWorkItem instead.

Merge branch 'main' into user/chengfrank/DMFTFaceAuth

efb9311

Frank Cheng added 3 commits October 9, 2025 10:51

Fix CI/CD nuget built failure.

9ce1c2f

Merge branch 'user/chengfrank/DMFTFaceAuth' of https://github.com/wel…

984a42b

…ljeng/Windows-driver-samples into user/chengfrank/DMFTFaceAuth

Remove nuget package settings from AvsCameraDMFT.vcxproj

0f0734b

welljeng requested review from SangChoe and chrilaMSFT October 9, 2025 06:42

chrilaMSFT reviewed Oct 15, 2025

View reviewed changes

avstream/avscamera/DMFT/AvsCameraDMFT.cpp

}

HRESULT CMultipinMft::ProfileAsyncResultCallback(_In_ IMFAsyncResult* pResult)

{

Copy link

chrilaMSFT Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No lock?

chrilaMSFT reviewed Oct 15, 2025

View reviewed changes

chrilaMSFT requested changes Oct 15, 2025

View reviewed changes

chrilaMSFT reviewed Oct 15, 2025

View reviewed changes

chrilaMSFT requested review from LPBourret and removed request for SangChoe October 17, 2025 17:41

		@@ -1293,6 +1371,102 @@ IFACEMETHODIMP CMultipinMft::Shutdown(
		return ShutdownEventGenerator();


		interface IDirect3DDeviceManager9;

		constexpr int kMAX_WAIT_TIME_DRIVER_PROFILE_KSEVENT = 3000;// ms, amount of time to wait for the profile DDI KsEvent sent to the driver

		hr = m_hSelectedProfileKSEventSentToDriver.create();
		DMFTCHECKHR_GOTO(hr, done);

		@@ -1039,14 +1058,57 @@ IFACEMETHODIMP CMultipinMft::KsEvent(
		Implements IKSProperty::KsEvent function.

Conversation

welljeng commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrilaMSFT commented Sep 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

welljeng Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

welljeng commented Sep 24, 2025 •

edited

Loading

welljeng Sep 26, 2025 •

edited

Loading