Address review fixes for Windows MSMF backend

wysaid · wysaid · commit 7cde5e76ae37 · 2026-03-07T18:58:57.000+08:00
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -253,11 +253,19 @@ elseif (WIN32)
 
     # MSVC: Delay load Media Foundation DLLs to preserve legacy DirectShow-only startup paths.
     if (MSVC)
-        target_link_options(ccap PRIVATE
-            /DELAYLOAD:mf.dll
-            /DELAYLOAD:mfplat.dll
-            /DELAYLOAD:mfreadwrite.dll
-        )
+        if (CCAP_BUILD_SHARED)
+            target_link_options(ccap PRIVATE
+                /DELAYLOAD:mf.dll
+                /DELAYLOAD:mfplat.dll
+                /DELAYLOAD:mfreadwrite.dll
+            )
+        else ()
+            target_link_options(ccap INTERFACE
+                /DELAYLOAD:mf.dll
+                /DELAYLOAD:mfplat.dll
+                /DELAYLOAD:mfreadwrite.dll
+            )
+        endif ()
         target_link_libraries(ccap PUBLIC delayimp.lib)
     endif ()
 else ()
diff --git a/README.zh-CN.md b/README.zh-CN.md
@@ -182,8 +182,17 @@ Windows 上默认优先使用 Media Foundation，并在需要时自动回退到
 - 设置环境变量 `CCAP_WINDOWS_BACKEND=auto|msmf|dshow`，对整个进程生效，包括 CLI 和 Rust 绑定。
 
 ```cpp
-ccap::Provider msmfProvider("", "msmf");
-ccap::Provider dshowProvider("", "dshow");
+// 下面两段是互斥示例；同一时刻不要对同一设备同时创建两个 Provider。
+
+// Force MSMF
+{
+    ccap::Provider provider("", "msmf");
+}
+
+// Force DirectShow
+{
+    ccap::Provider provider("", "dshow");
+}
 ```
 
 ### Rust 绑定
diff --git a/bindings/rust/build.rs b/bindings/rust/build.rs
@@ -196,6 +196,7 @@ Please vendor the sources into bindings/rust/native/, or set CCAP_SOURCE_DIR to
         {
             build
                 .file(ccap_root.join("src/ccap_imp_windows.cpp"))
+                .file(ccap_root.join("src/ccap_imp_windows_msmf.cpp"))
                 .file(ccap_root.join("src/ccap_file_reader_windows.cpp"));
         }
 
@@ -368,10 +369,11 @@ Please vendor the sources into bindings/rust/native/, or set CCAP_SOURCE_DIR to
 
     #[cfg(target_os = "windows")]
     {
+        println!("cargo:rustc-link-lib=mf");
         println!("cargo:rustc-link-lib=strmiids");
         println!("cargo:rustc-link-lib=ole32");
         println!("cargo:rustc-link-lib=oleaut32");
-        // Media Foundation libraries for video file playback
+        // Media Foundation libraries for the MSMF camera backend and video file playback
         println!("cargo:rustc-link-lib=mfplat");
         println!("cargo:rustc-link-lib=mfreadwrite");
         println!("cargo:rustc-link-lib=mfuuid");
diff --git a/bindings/rust/src/provider.rs b/bindings/rust/src/provider.rs
@@ -96,15 +96,14 @@ impl Provider {
     ///
     /// On Windows, `extra_info` can be used to force backend selection with values like
     /// `"auto"`, `"msmf"`, `"dshow"`, or `"backend=<value>"`.
-    pub fn with_device_and_extra_info(
-        device_index: i32,
-        extra_info: Option<&str>,
-    ) -> Result<Self> {
+    pub fn with_device_and_extra_info(device_index: i32, extra_info: Option<&str>) -> Result<Self> {
         let extra_info = optional_c_string(extra_info, "extra info")?;
         let handle = unsafe {
             sys::ccap_provider_create_with_index(
                 device_index,
-                extra_info.as_ref().map_or(ptr::null(), |value| value.as_ptr()),
+                extra_info
+                    .as_ref()
+                    .map_or(ptr::null(), |value| value.as_ptr()),
             )
         };
         if handle.is_null() {
@@ -144,7 +143,9 @@ impl Provider {
         let handle = unsafe {
             sys::ccap_provider_create_with_device(
                 c_name.as_ptr(),
-                extra_info.as_ref().map_or(ptr::null(), |value| value.as_ptr()),
+                extra_info
+                    .as_ref()
+                    .map_or(ptr::null(), |value| value.as_ptr()),
             )
         };
         if handle.is_null() {
@@ -271,6 +272,11 @@ impl Provider {
         auto_start: bool,
     ) -> Result<()> {
         if let Some(name) = device_name {
+            let c_name = CString::new(name).map_err(|_| {
+                CcapError::InvalidParameter("device name contains null byte".to_string())
+            })?;
+            let extra_info = optional_c_string(extra_info, "extra info")?;
+
             // Recreate provider with specific device
             if !self.handle.is_null() {
                 // If the previous provider was running, stop it and detach callbacks
@@ -281,21 +287,27 @@ impl Provider {
                 unsafe {
                     sys::ccap_provider_destroy(self.handle);
                 }
+                self.handle = ptr::null_mut();
+                self.is_opened = false;
+            } else {
+                self.cleanup_callback();
             }
-            let c_name = CString::new(name).map_err(|_| {
-                CcapError::InvalidParameter("device name contains null byte".to_string())
-            })?;
-            let extra_info = optional_c_string(extra_info, "extra info")?;
+
             self.handle = unsafe {
                 sys::ccap_provider_create_with_device(
                     c_name.as_ptr(),
-                    extra_info.as_ref().map_or(ptr::null(), |value| value.as_ptr()),
+                    extra_info
+                        .as_ref()
+                        .map_or(ptr::null(), |value| value.as_ptr()),
                 )
             };
             if self.handle.is_null() {
                 return Err(CcapError::InvalidDevice(name.to_string()));
             }
             self.is_opened = true;
+            if !auto_start {
+                self.stop_capture()?;
+            }
         } else if extra_info.is_some() {
             return self.open_with_index_and_extra_info(-1, extra_info, auto_start);
         } else {
@@ -581,6 +593,8 @@ impl Provider {
         extra_info: Option<&str>,
         auto_start: bool,
     ) -> Result<()> {
+        let extra_info = optional_c_string(extra_info, "extra info")?;
+
         // If the previous provider was running, stop it and detach callbacks
         // before destroying the underlying handle.
         if !self.handle.is_null() {
@@ -590,18 +604,20 @@ impl Provider {
             unsafe {
                 sys::ccap_provider_destroy(self.handle);
             }
+            self.handle = ptr::null_mut();
+            self.is_opened = false;
         } else {
             // Clean up any stale callback allocation even if handle is null.
             self.cleanup_callback();
         }
 
-        let extra_info = optional_c_string(extra_info, "extra info")?;
-
         // Create a new provider with the specified device index
         self.handle = unsafe {
             sys::ccap_provider_create_with_index(
                 device_index,
-                extra_info.as_ref().map_or(ptr::null(), |value| value.as_ptr()),
+                extra_info
+                    .as_ref()
+                    .map_or(ptr::null(), |value| value.as_ptr()),
             )
         };
 
@@ -614,6 +630,9 @@ impl Provider {
 
         // ccap C API contract: create_with_index opens the device.
         self.is_opened = true;
+        if !auto_start {
+            self.stop_capture()?;
+        }
         if auto_start {
             self.start_capture()?;
         }
diff --git a/docs/content/implementation-details.md b/docs/content/implementation-details.md
@@ -218,7 +218,8 @@ Or use the script:
 **Media Foundation Backend:**
 - Preferred on modern Windows systems
 - Uses Source Reader for frame delivery and format negotiation
-- Automatically falls back to DirectShow when Media Foundation is unavailable or device open fails
+- When backend selection is `auto`, `Provider::open()` falls back to DirectShow if Media Foundation is unavailable or device open fails
+- When callers explicitly request `msmf` via `extraInfo` or `CCAP_WINDOWS_BACKEND`, `Provider::open()` returns an error instead of falling back
 
 **DirectShow Backend:**
 - Mature, stable API
diff --git a/include/ccap_core.h b/include/ccap_core.h
@@ -229,34 +229,16 @@ class CCAP_EXPORT Provider final {
 
 
     // ↓ This part is not relevant to the user ↓
-    Provider(Provider&&) = default;
-    Provider& operator=(Provider&&) = default;
+    Provider(Provider&&) noexcept;
+    Provider& operator=(Provider&&) noexcept;
     ~Provider();
 
 private:
     void applyCachedState(ProviderImp* imp) const;
     bool tryOpenWithImplementation(ProviderImp* imp, std::string_view deviceName, bool autoStart) const;
 
 private:
-    std::string m_extraInfo;
-    std::function<bool(const std::shared_ptr<VideoFrame>&)> m_frameCallback;
-    std::function<std::shared_ptr<Allocator>()> m_allocatorFactory;
-    uint32_t m_maxAvailableFrameSize = DEFAULT_MAX_AVAILABLE_FRAME_SIZE;
-    uint32_t m_maxCacheFrameSize = DEFAULT_MAX_CACHE_FRAME_SIZE;
-    int m_requestedWidth = 640;
-    int m_requestedHeight = 480;
-    double m_requestedFrameRate = 0.0;
-    PixelFormat m_requestedInternalFormat = PixelFormat::Unknown;
-    PixelFormat m_requestedOutputFormat{
-#ifdef __APPLE__
-        PixelFormat::BGRA32
-#else
-        PixelFormat::BGR24
-#endif
-    };
-    bool m_hasFrameOrientationOverride = false;
-    FrameOrientation m_requestedFrameOrientation = FrameOrientation::Default;
-    ProviderImp* m_imp;
+    ProviderImp* m_imp = nullptr;
 };
 
 /**
diff --git a/src/ccap_core.cpp b/src/ccap_core.cpp
diff --git a/src/ccap_imp_windows_msmf.cpp b/src/ccap_imp_windows_msmf.cpp
diff --git a/tests/test_windows_backends.cpp b/tests/test_windows_backends.cpp

Original file line number	Diff line number	Diff line change
`@@ -196,6 +196,7 @@ Please vendor the sources into bindings/rust/native/, or set CCAP_SOURCE_DIR to`
`196`	`196`	`{`
`197`	`197`	`build`
`198`	`198`	`.file(ccap_root.join("src/ccap_imp_windows.cpp"))`
	`199`	`+ .file(ccap_root.join("src/ccap_imp_windows_msmf.cpp"))`
`199`	`200`	`.file(ccap_root.join("src/ccap_file_reader_windows.cpp"));`
`200`	`201`	`}`
`201`	`202`
`@@ -368,10 +369,11 @@ Please vendor the sources into bindings/rust/native/, or set CCAP_SOURCE_DIR to`
`368`	`369`
`369`	`370`	`#[cfg(target_os = "windows")]`
`370`	`371`	`{`
	`372`	`+ println!("cargo:rustc-link-lib=mf");`
`371`	`373`	`println!("cargo:rustc-link-lib=strmiids");`
`372`	`374`	`println!("cargo:rustc-link-lib=ole32");`
`373`	`375`	`println!("cargo:rustc-link-lib=oleaut32");`
`374`		`- // Media Foundation libraries for video file playback`
	`376`	`+ // Media Foundation libraries for the MSMF camera backend and video file playback`
`375`	`377`	`println!("cargo:rustc-link-lib=mfplat");`
`376`	`378`	`println!("cargo:rustc-link-lib=mfreadwrite");`
`377`	`379`	`println!("cargo:rustc-link-lib=mfuuid");`