Conversation
Add an extensible capture plugin system that allows external DLLs to provide screen capture backends for Sunshine on Windows. Plugin system components: - capture_plugin_api.h: Pure C ABI interface (v1) for plugin DLLs - capture_plugin_loader: Discovery and loading of plugin DLLs from plugins/ dir - display_plugin: Adapter bridging plugin C ABI to display_t interface - display() factory: Falls back to plugin loader for unknown capture types Example plugin: - examples/capture_plugin_nvfbc/: NvFBC skeleton plugin with CMakeLists.txt Usage: Set 'capture = <plugin_name>' in config, place DLL in plugins/ dir.
There was a problem hiding this comment.
Pull request overview
This PR adds an extensible capture plugin framework for Sunshine on Windows, allowing external DLLs to provide screen capture backends via a C ABI interface. The architecture includes a pure C API header for cross-compiler compatibility, a runtime DLL loader that discovers plugins from a plugins/ directory, and an adapter class that bridges the plugin's C ABI to Sunshine's internal display_t interface. An example NvFBC skeleton plugin is included for reference.
Changes:
- New C ABI interface (
capture_plugin_api.h) defining plugin structures, enums, and function signatures for capture session management - Plugin loader (
capture_plugin_loader.cpp) for discovering and loading DLLs, plus adisplay_plugin_tadapter class implementing Sunshine'sdisplay_tinterface - Integration into the
display()factory function to fall back to plugin loading for unrecognized capture type names, with build integration in the Windows CMake config
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/platform/windows/capture_plugin/capture_plugin_api.h |
Defines the C ABI interface (v1) with plugin info, session management, frame capture, and vtable structures |
src/platform/windows/capture_plugin/capture_plugin_loader.h |
Header for plugin discovery, loading, and lookup functions |
src/platform/windows/capture_plugin/capture_plugin_loader.cpp |
Implementation of DLL loading via LoadLibraryExW, ABI version validation, and directory scanning |
src/platform/windows/capture_plugin/display_plugin.h |
Header for display_plugin_t adapter and plugin_img_t image wrapper |
src/platform/windows/capture_plugin/display_plugin.cpp |
Implementation of plugin-to-display_t bridge, capture loop, and frame management |
src/platform/windows/display_base.cpp |
Extended display() factory to fall back to plugin loader for unknown capture types |
cmake/compile_definitions/windows.cmake |
Added capture plugin source files to Windows platform build |
examples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cpp |
Skeleton NvFBC plugin demonstrating the full plugin API |
examples/capture_plugin_nvfbc/CMakeLists.txt |
Standalone CMake build file for the example plugin |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct loaded_plugin_t { | ||
| std::string name; ///< Plugin name from get_info() | ||
| std::string version; ///< Plugin version string | ||
| std::string dll_path; ///< Full path to the DLL | ||
| uint32_t supported_mem_types; ///< Bitmask of supported memory types | ||
|
|
||
| sunshine_capture_plugin_vtable_t vtable; ///< Function table | ||
|
|
||
| void *dll_handle; ///< Platform-specific DLL handle (HMODULE on Windows) | ||
| }; |
There was a problem hiding this comment.
loaded_plugin_t holds a raw dll_handle (HMODULE) but has no destructor to call FreeLibrary. When the unique_ptr<loaded_plugin_t> objects in the static vector are destroyed at program shutdown, the DLL handles leak. While the OS will reclaim them at process exit, this is still poor resource management.
Consider either adding a destructor to loaded_plugin_t that calls FreeLibrary, or using a custom deleter on the unique_ptr that calls unload_plugin.
| for (const auto &entry : std::filesystem::directory_iterator(plugins_dir)) { | ||
| if (!entry.is_regular_file()) continue; | ||
|
|
||
| auto ext = entry.path().extension().string(); | ||
| std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower); | ||
| if (ext != ".dll") continue; | ||
|
|
||
| auto plugin = load_plugin(entry.path()); | ||
| if (plugin) { | ||
| plugins.push_back(std::move(plugin)); | ||
| } | ||
| } | ||
|
|
||
| BOOST_LOG(info) << "Discovered " << plugins.size() << " capture plugin(s)"; | ||
| return plugins; | ||
| } |
There was a problem hiding this comment.
All DLLs in the plugins/ directory are loaded eagerly when discover_plugins() is first called, even if only one specific plugin was requested. Loading a DLL executes its DllMain, which means any DLL placed in the plugins directory gets arbitrary code execution.
Consider only loading the specific plugin DLL that matches the requested capture type name (e.g., look for sunshine_<name>.dll), rather than enumerating and loading all DLLs. This reduces the attack surface significantly and also improves startup time.
| auto frame_interval = std::chrono::nanoseconds(1'000'000'000 / framerate_); | ||
| auto next_frame_time = std::chrono::steady_clock::now(); | ||
|
|
||
| while (!stop_flag_) { | ||
| // Wait for frame timing | ||
| auto now = std::chrono::steady_clock::now(); | ||
| if (now < next_frame_time) { | ||
| std::this_thread::sleep_for(next_frame_time - now); | ||
| } | ||
| next_frame_time += frame_interval; |
There was a problem hiding this comment.
The capture loop implements its own sleep-based frame pacing (sleep_for + next_frame_time) on top of calling the plugin's next_frame() with a hardcoded 100ms timeout. This double-timing approach adds unnecessary latency: the loop sleeps until the next frame interval, then blocks for up to 100ms more in next_frame().
If the plugin's next_frame() blocks until a frame is available (which is the common pattern for capture APIs), the adapter's sleep pacing is counterproductive. Consider letting the plugin handle its own timing via next_frame() with an appropriate timeout derived from framerate_, and removing the sleep-based pacing. This would be more consistent with how the existing display_base_t::capture works, where the underlying capture API (dup.next_frame()) handles timing.
| bool | ||
| is_codec_supported(std::string_view name, const ::video::config_t &config) override; | ||
|
|
||
| private: |
There was a problem hiding this comment.
display_plugin_t does not override make_avcodec_encode_device() or make_nvenc_encode_device(). The base display_t implementations return nullptr, which causes make_encode_device() in src/video.cpp:2806-2811 to return an empty unique_ptr. This in turn causes make_synced_session() to fail (line 2842-2843), preventing the encoding pipeline from functioning at all.
For system memory plugins, you should override make_avcodec_encode_device to return a basic avcodec_encode_device_t (same as display_ram_t::make_avcodec_encode_device does at src/platform/windows/display_ram.cpp:402-403). For GPU memory types (DXGI/CUDA), appropriate encode device creation would be needed as well.
| private: | |
| private: | |
| /** | |
| * @brief Create an AVCodec encode device for this display backend. | |
| * | |
| * For plugin-backed capture that delivers frames in system memory, we can | |
| * use a basic avcodec_encode_device_t, equivalent to the RAM display | |
| * implementation. | |
| */ | |
| std::unique_ptr<avcodec_encode_device_t> | |
| make_avcodec_encode_device(const avcodec_t &, const ::video::config_t &) override { | |
| return std::make_unique<avcodec_encode_device_t>(); | |
| } |
| display_plugin_t::~display_plugin_t() { | ||
| stop_flag_ = true; | ||
|
|
||
| if (session_) { | ||
| if (plugin_->vtable.interrupt) { | ||
| plugin_->vtable.interrupt(session_); | ||
| } | ||
| plugin_->vtable.destroy_session(session_); | ||
| session_ = nullptr; | ||
| } | ||
| } |
There was a problem hiding this comment.
There's a potential use-after-free issue. display_plugin_t::~display_plugin_t() destroys the plugin session via destroy_session(session_), but plugin_img_t objects in the image pool may still hold references to this session (via plugin_img_t::session). When those images are eventually destroyed, their ~plugin_img_t() calls release() which attempts to call vtable->release_frame(session, ...) on the already-destroyed session.
To fix this, either:
- Ensure all outstanding
plugin_img_tobjects are released before destroying the session (e.g., clear the image pool), or - Use a shared ownership mechanism (e.g.,
shared_ptr) for the session, or - Set a flag that prevents
plugin_img_t::release()from calling into the plugin after the session is destroyed.
| std::shared_ptr<img_t> | ||
| display_plugin_t::alloc_img() { | ||
| auto img = std::make_shared<plugin_img_t>(); | ||
| img->width = width; | ||
| img->height = height; | ||
| img->pixel_pitch = 4; // Default BGRA | ||
| img->row_pitch = img->width * img->pixel_pitch; | ||
| return img; | ||
| } | ||
|
|
||
| int | ||
| display_plugin_t::dummy_img(img_t *img) { | ||
| // Fill with black | ||
| if (img->data && img->row_pitch > 0 && img->height > 0) { | ||
| std::memset(img->data, 0, img->row_pitch * img->height); | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
alloc_img() creates a plugin_img_t but never allocates a backing buffer for data, leaving it as nullptr (the default from img_t). The callers of alloc_img() / dummy_img() in src/video.cpp (lines 2653-2654, 2935-2936, 3302) expect alloc_img() to return an image that, after dummy_img(), has valid data for use in session->convert().
Currently, dummy_img() just checks img->data which is null, does nothing, and returns success. This will cause downstream code to operate on a null data pointer.
The fix is to allocate a frame buffer in alloc_img() (e.g., img->data = new uint8_t[width * height * 4]{}) or have dummy_img() allocate the buffer when it's null. You'll also need to ensure the buffer is freed in plugin_img_t's destructor.
| else { | ||
| // Try loading as a capture plugin | ||
| static auto plugins = capture_plugin::discover_plugins(); | ||
| auto *plugin = capture_plugin::find_plugin(plugins, type); | ||
| if (plugin) { | ||
| auto disp = std::make_shared<capture_plugin::display_plugin_t>(plugin, hwdevice_type); | ||
| if (!disp->init(config, display_name)) { | ||
| ret = disp; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The else block will also be reached for known capture type names that don't match their additional conditions. For example, type == "amd" with hwdevice_type != mem_type_e::dxgi, or type == "wgc" when is_running_as_system_user is true, will fall through to the plugin loader and attempt to find plugins named "amd" or "wgc".
While this is unlikely to match an actual plugin, it's confusing and could cause unintended behavior if someone names a plugin "amd" or "wgc". Consider adding an explicit check that the type is not a known built-in name before attempting plugin lookup, or restructuring the logic so that plugin lookup only occurs for truly unrecognized type names.
| // Frame buffer for ToSys capture | ||
| std::vector<uint8_t> frame_buffer; | ||
| bool frame_ready; | ||
| bool interrupted; |
There was a problem hiding this comment.
The interrupted field is a plain bool, but per the API documentation in capture_plugin_api.h (line 201), sunshine_capture_interrupt is "called from a different thread." This creates a data race when next_frame reads s->interrupted on the capture thread while interrupt writes it from another thread. This should be std::atomic<bool> to ensure thread safety.
Even though this is example/skeleton code, it will likely be copied by plugin authors and should demonstrate correct patterns.
- Remove all hardcoded magic private data / bypass mechanisms - Load NvFBC private data from external file (nvfbc_auth.bin or env var) - Remove INvFBCToSys vtable definition (requires official Capture SDK) - Add NOTICE.md with legal disclaimer and usage requirements - Keep NvFBC_CreateEx flow as reference implementation - Mark INvFBCToSys method calls as TODO pending official SDK headers
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add NvFBC (Plugin) option to Windows capture dropdown - Add install button with status check (GPU, registry, wrapper, plugin) - Add Tauri backend: check_nvfbc_status, setup_nvfbc commands - Add i18n keys for zh/en locales - Update sunshine-control-panel submodule
qiin2333
force-pushed
the
master
branch
2 times, most recently
from
e6bbe21 to
d74f9ed
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 Walkthrough总体概述该拉取请求为 Windows 上的 Sunshine 添加了可插拔式屏幕捕捉插件系统，包括通用插件加载器基础设施、稳定的 C ABI 接口定义以及 NVIDIA NvFBC 作为第一个参考实现，并集成了相应的 UI 组件用于插件管理和状态检查。 变更
时序图sequenceDiagram
participant App as Sunshine App
participant Loader as 插件加载器
participant DLL as 插件 DLL<br/>(NvFBC)
participant Display as Display Plugin
participant NvFBC as NvFBC 库
App->>Loader: discover_plugins()
Loader->>DLL: LoadLibraryExW()
DLL-->>Loader: DLL 句柄
Loader->>DLL: 解析 sunshine_capture_get_info
DLL-->>Loader: 函数指针 + 插件元数据
Loader->>Loader: 校验 ABI 版本
Loader-->>App: loaded_plugin_t
App->>Display: display_plugin_t::init()
Display->>DLL: vtable.create_session()
DLL->>NvFBC: NvFBC_CreateEx()
NvFBC-->>DLL: 会话句柄
DLL-->>Display: 会话 ID
Display-->>App: 初始化成功
loop 帧捕捉
App->>Display: capture()
Display->>DLL: vtable.next_frame()
DLL->>NvFBC: 捕捉帧数据
NvFBC-->>DLL: 帧缓冲
DLL-->>Display: sunshine_frame_t
Display->>Display: 填充 plugin_img_t
Display-->>App: 图像回调
end
App->>Display: 销毁
Display->>DLL: vtable.destroy_session()
DLL->>NvFBC: 释放会话
DLL->>DLL: FreeLibrary()
代码审查工作量估计🎯 4 (复杂) | ⏱️ ~50 分钟 该变更涉及多个相互关联的新组件（插件加载器、显示适配器、NvFBC 实现），包含复杂的 DLL 加载、符号解析、会话管理和帧处理逻辑。跨越多个不同文件类型（C++ 核心库、示例插件、UI、CMake），需要对插件 ABI、生命周期、内存管理和错误处理的综合理解。 诗歌
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
src_assets/common/assets/web/configs/tabs/Advanced.vue (2)
108-141: 考虑合并重复的状态检查调用。
installNvfbc中使用了两次setTimeout来延迟检查状态（3秒和8秒），与 WGC 的实现模式相似。但可以考虑抽取为可复用的工具函数：♻️ 可选重构建议
// 可复用的延迟重检查函数 const scheduleRechecks = (checkFn, delays = [3000, 8000]) => { delays.forEach(delay => setTimeout(checkFn, delay)) } // 在 installNvfbc 中使用 const installNvfbc = async () => { // ... existing code ... scheduleRechecks(checkNvfbcStatus) }这是一个可选的代码整洁性改进，当前实现功能正确。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src_assets/common/assets/web/configs/tabs/Advanced.vue` around lines 108 - 141, The installNvfbc function currently schedules duplicate rechecks with two setTimeout calls; extract a small helper like scheduleRechecks(checkFn, delays = [3000, 8000]) that iterates delays and calls setTimeout(checkFn, delay), then replace the two setTimeouts in installNvfbc with a single call scheduleRechecks(checkNvfbcStatus); update any other places (e.g., WGC-related code) to reuse scheduleRechecks to avoid duplication and keep checkNvfbcStatus unchanged.
249-286: 按钮状态条件过长，建议抽取为计算属性。多处重复检查
nvfbcStatus && nvfbcStatus.wrapper_installed && nvfbcStatus.registry_enabled && nvfbcStatus.plugin_installed，可读性较差。♻️ 建议的重构
// 在 script setup 中添加计算属性 const isNvfbcReady = computed(() => { return nvfbcStatus.value && nvfbcStatus.value.wrapper_installed && nvfbcStatus.value.registry_enabled && nvfbcStatus.value.plugin_installed }) // 模板中使用 // :class="['btn', isNvfbcReady ? 'btn-success' : 'btn-primary']"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src_assets/common/assets/web/configs/tabs/Advanced.vue` around lines 249 - 286, Extract the repeated long condition into a computed property named isNvfbcReady in the component's script setup (use nvfbcStatus as the source) and then replace all occurrences of "nvfbcStatus && nvfbcStatus.wrapper_installed && nvfbcStatus.registry_enabled && nvfbcStatus.plugin_installed" in the template with isNvfbcReady; update the button's :class expression, the icon v-else-if, and the text conditional that currently checks nvfbcStatus to use isNvfbcReady alongside the existing isInstallingNvfbc, isCheckingNvfbc and installNvfbc handler to preserve behavior.examples/capture_plugin_nvfbc/CMakeLists.txt (1)
7-17: 考虑验证SUNSHINE_SOURCE_DIR路径有效性。当前实现假设插件在
examples/capture_plugin_nvfbc/目录下构建，../..指向 Sunshine 源码根目录。如果从其他位置构建，可能找不到capture_plugin_api.h。建议添加路径验证：
🛠️ 建议的修改
set(SUNSHINE_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../.." CACHE PATH "Path to Sunshine source root") + +if(NOT EXISTS "${SUNSHINE_SOURCE_DIR}/src/platform/windows/capture_plugin/capture_plugin_api.h") + message(FATAL_ERROR "Cannot find capture_plugin_api.h. Please set SUNSHINE_SOURCE_DIR correctly.") +endif()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/capture_plugin_nvfbc/CMakeLists.txt` around lines 7 - 17, Validate the SUNSHINE_SOURCE_DIR before using it: check that "${SUNSHINE_SOURCE_DIR}" exists and that the expected header (e.g., capture_plugin_api.h) can be found before calling target_include_directories for target sunshine_nvfbc, and if not present emit a clear CMake error (message(FATAL_ERROR)) with guidance on how to set SUNSHINE_SOURCE_DIR (or optionally try to locate the header with find_path and fallback). Ensure references to SUNSHINE_SOURCE_DIR, capture_plugin_api.h and the target name sunshine_nvfbc are used so the validation ties directly to the include setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cpp`:
- Around line 271-286: Do not mark the session as valid until we can actually
grab frames: change the logic around sunshine_nvfbc_plugin.cpp so
s->session_valid is not set to true and a successful 0 is not returned
immediately in the setup path (where *session is assigned and return 0 currently
occurs); instead leave session_valid false and return an appropriate "not ready"
error (or propagate the GrabFrame timeout behavior) and only set
s->session_valid = true inside the first successful GrabFrame/ToSys setup
completion (the code path currently returning SUNSHINE_CAPTURE_TIMEOUT in lines
~311-359 should be the state the caller sees until a real frame is obtained);
reference s->session_valid, sunshine_capture_session_t, and the
GrabFrame/INvFBCToSys setup code when making the change.
- Around line 223-229: The code sets displays[0].width/height using
GetSystemMetrics(SM_CXSCREEN/SM_CYSCREEN) which returns only the primary monitor
size; replace those calls with GetSystemMetrics(SM_CXVIRTUALSCREEN) and
GetSystemMetrics(SM_CYVIRTUALSCREEN) so displays[0] reports the full virtual
desktop size (matching the approach in src/platform/windows/display_base.cpp);
update the two calls that assign displays[0].width and displays[0].height
accordingly.
In `@src/platform/windows/capture_plugin/capture_plugin_api.h`:
- Line 124: The public ABI lacks an explicit Windows calling convention which
can break cross-compiler/plugin calls; define a calling-convention macro (e.g.,
SUNSHINE_CAPTURE_CALL set to __cdecl or chosen convention) and apply it
uniformly to all function pointer types and exported functions (for example,
update sunshine_capture_get_info_fn and any other sunshine_capture_* function
pointer typedefs and export declarations to use SUNSHINE_CAPTURE_CALL) so the
host and plugins share the same calling convention and avoid
GetProcAddress/stack corruption issues.
- Around line 84-102: sunshine_frame_t.timestamp_ns currently claims to be a
"steady clock nanoseconds" value but exposing steady_clock epoch across C ABI is
not portable; stop reconstructing std::chrono::steady_clock::time_point directly
from frame.timestamp_ns in display_plugin.cpp (the plugin_img->frame_timestamp
assignment). Instead, change the ABI semantics: document timestamp_ns as either
(a) 0 meaning "unset—host must fill timestamp using its local monotonic clock
via existing qpc_time_difference logic" or (b) an OS-level monotonic tick value
(e.g., QueryPerformanceCounter ticks or nanoseconds tied to the host's monotonic
clock) and then convert it to a steady_clock time_point using host-specific
conversion (use qpc_time_difference and QPC frequency where available). Update
the sunshine_frame_t comment to remove any claim about std::chrono::steady_clock
epoch, modify the consumer code around plugin_img->frame_timestamp to check for
timestamp_ns == 0 and use host qpc_time_difference to populate the timestamp,
and if non-zero perform an OS-specific conversion rather than direct
std::chrono::steady_clock reconstruction.
In `@src/platform/windows/capture_plugin/capture_plugin_loader.cpp`:
- Around line 35-40: The call to GetModuleFileNameW in get_plugins_dir does not
check its return value and can silently truncate paths longer than MAX_PATH;
update get_plugins_dir to call GetModuleFileNameW with a dynamically sized
wchar_t buffer (e.g., std::vector<wchar_t>) and loop, doubling the buffer size
until the returned length is less than the buffer size (and nonzero), or stop on
error; if GetModuleFileNameW returns zero, handle/check GetLastError and
propagate or log an error instead of using a truncated module_path; reference
get_plugins_dir, GetModuleFileNameW, MAX_PATH and module_path when locating the
code to change.
- Around line 128-139: The directory iteration over
std::filesystem::directory_iterator in the plugin loading loop can throw (e.g.,
permissions or missing path); wrap the iteration in a try-catch that catches
std::filesystem::filesystem_error (and optionally std::exception) around the
loop that uses directory_iterator, log a clear error via your logger and either
abort plugin loading or return/continue gracefully, and ensure resources remain
consistent; update the block that calls load_plugin(entry.path()) and pushes
into plugins to run only when no exception was thrown.
In `@src/platform/windows/capture_plugin/display_plugin.cpp`:
- Around line 39-48: The destructor currently calls
plugin_->vtable.destroy_session(session_) immediately, which can race with
in-flight frames returned via capture() because plugin_img_t::~plugin_img_t()
still calls release_frame() using the cached session/vtable; instead, set
stop_flag_ and call plugin_->vtable.interrupt(session_) as now, then wait for
any background next_frame() to exit (join the worker thread or wait on a
condition) and for in-flight frames to be returned (track outstanding frame
count or use a shared ownership/weak_ptr scheme), only after outstanding frames
are zero and next_frame() has stopped call
plugin_->vtable.destroy_session(session_) and then set session_ = nullptr;
update display_plugin_t::~display_plugin_t(), any thread/worker join logic, and
ensure plugin_img_t holds only safe references (weak/shared) to session/vtable
to avoid use-after-free.
- Around line 19-30: The adapter must reject or explicitly handle memory types
it doesn't support instead of silently mapping them to system memory; update the
code path that converts and forwards mem_type_ to the plugin by (1) changing
to_plugin_mem_type(mem_type_e) to not default to SUNSHINE_MEM_SYSTEM — return an
explicit invalid/unsupported value or assert/error on unknown enums, and (2) add
a runtime check before calling the plugin (where mem_type_ is passed) to
validate adapter support for that mem type and return an error if unsupported.
Also ensure the plugin buffer population (the block that currently only copies
CPU pointer and pitch and mentions gpu_handle/gpu_subresource) copies or
forwards gpu_handle and gpu_subresource for DXGI/CUDA paths so GPU-backed memory
isn’t misrouted; locate references to mem_type_, to_plugin_mem_type(), and the
code that sets gpu_handle/gpu_subresource and update those to propagate GPU
fields for non-system types and to fail early for unsupported/unknown types.
---
Nitpick comments:
In `@examples/capture_plugin_nvfbc/CMakeLists.txt`:
- Around line 7-17: Validate the SUNSHINE_SOURCE_DIR before using it: check that
"${SUNSHINE_SOURCE_DIR}" exists and that the expected header (e.g.,
capture_plugin_api.h) can be found before calling target_include_directories for
target sunshine_nvfbc, and if not present emit a clear CMake error
(message(FATAL_ERROR)) with guidance on how to set SUNSHINE_SOURCE_DIR (or
optionally try to locate the header with find_path and fallback). Ensure
references to SUNSHINE_SOURCE_DIR, capture_plugin_api.h and the target name
sunshine_nvfbc are used so the validation ties directly to the include setup.
In `@src_assets/common/assets/web/configs/tabs/Advanced.vue`:
- Around line 108-141: The installNvfbc function currently schedules duplicate
rechecks with two setTimeout calls; extract a small helper like
scheduleRechecks(checkFn, delays = [3000, 8000]) that iterates delays and calls
setTimeout(checkFn, delay), then replace the two setTimeouts in installNvfbc
with a single call scheduleRechecks(checkNvfbcStatus); update any other places
(e.g., WGC-related code) to reuse scheduleRechecks to avoid duplication and keep
checkNvfbcStatus unchanged.
- Around line 249-286: Extract the repeated long condition into a computed
property named isNvfbcReady in the component's script setup (use nvfbcStatus as
the source) and then replace all occurrences of "nvfbcStatus &&
nvfbcStatus.wrapper_installed && nvfbcStatus.registry_enabled &&
nvfbcStatus.plugin_installed" in the template with isNvfbcReady; update the
button's :class expression, the icon v-else-if, and the text conditional that
currently checks nvfbcStatus to use isNvfbcReady alongside the existing
isInstallingNvfbc, isCheckingNvfbc and installNvfbc handler to preserve
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2035ff09-ed1e-438d-ad74-bdcf5d5bc6f6
📒 Files selected for processing (15)
cmake/compile_definitions/windows.cmakeexamples/capture_plugin_nvfbc/CMakeLists.txtexamples/capture_plugin_nvfbc/NOTICE.mdexamples/capture_plugin_nvfbc/nvfbc_win_defs.hexamples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cppsrc/platform/windows/capture_plugin/capture_plugin_api.hsrc/platform/windows/capture_plugin/capture_plugin_loader.cppsrc/platform/windows/capture_plugin/capture_plugin_loader.hsrc/platform/windows/capture_plugin/display_plugin.cppsrc/platform/windows/capture_plugin/display_plugin.hsrc/platform/windows/display_base.cppsrc_assets/common/assets/web/configs/tabs/Advanced.vuesrc_assets/common/assets/web/public/assets/locale/en.jsonsrc_assets/common/assets/web/public/assets/locale/zh.jsonsrc_assets/common/sunshine-control-panel
| // NvFBC captures the full desktop; expose one display | ||
| if (displays && max_displays > 0) { | ||
| strncpy(displays[0].name, "NvFBC Desktop", sizeof(displays[0].name) - 1); | ||
| displays[0].name[sizeof(displays[0].name) - 1] = '\0'; | ||
| displays[0].width = GetSystemMetrics(SM_CXSCREEN); | ||
| displays[0].height = GetSystemMetrics(SM_CYSCREEN); | ||
| displays[0].is_primary = 1; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GetSystemMetrics SM_CXSCREEN vs SM_CXVIRTUALSCREEN Windows API documentation
💡 Result:
SM_CXSCREEN returns the width of the primary display monitor's screen in pixels. SM_CXVIRTUALSCREEN returns the width of the entire virtual screen, which is the bounding rectangle encompassing all display monitors in a multi-monitor setup. In single-monitor configurations, these values are identical. GetSystemMetrics is not DPI-aware by default; for per-monitor DPI-aware apps, use GetSystemMetricsForDpi instead.
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getsystemmetrics
- 2: https://learn.microsoft.com/en-us/windows/win32/gdi/multiple-monitor-system-metrics
🏁 Script executed:
# Search for the specific file mentioned in the review
fd "sunshine_nvfbc_plugin.cpp" --type fRepository: AlkaidLab/foundation-sunshine
Length of output: 128
🏁 Script executed:
# Also search for any related display enumeration code
rg "GetSystemMetrics.*SM_CXSCREEN|SM_CXVIRTUALSCREEN" -A 3 -B 3Repository: AlkaidLab/foundation-sunshine
Length of output: 1773
全桌面尺寸获取错误，多显示器场景下会报告错误分辨率。
代码注释说"NvFBC captures the full desktop"，但 SM_CXSCREEN/SM_CYSCREEN 只返回主显示器的尺寸。在多显示器扩展桌面配置中，应使用 SM_CXVIRTUALSCREEN/SM_CYVIRTUALSCREEN 来获取整个虚拟屏幕的尺寸。项目中 src/platform/windows/display_base.cpp 已正确采用这一模式。
建议修改
- displays[0].width = GetSystemMetrics(SM_CXSCREEN);
- displays[0].height = GetSystemMetrics(SM_CYSCREEN);
+ displays[0].width = GetSystemMetrics(SM_CXVIRTUALSCREEN);
+ displays[0].height = GetSystemMetrics(SM_CYVIRTUALSCREEN);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cpp` around lines 223 -
229, The code sets displays[0].width/height using
GetSystemMetrics(SM_CXSCREEN/SM_CYSCREEN) which returns only the primary monitor
size; replace those calls with GetSystemMetrics(SM_CXVIRTUALSCREEN) and
GetSystemMetrics(SM_CYVIRTUALSCREEN) so displays[0] reports the full virtual
desktop size (matching the approach in src/platform/windows/display_base.cpp);
update the two calls that assign displays[0].width and displays[0].height
accordingly.
| // Step 4: SetUp and GrabFrame require NVIDIA Capture SDK headers | ||
| // to properly call the INvFBCToSys interface methods. | ||
| // | ||
| // With official SDK headers: | ||
| // auto *toSys = static_cast<INvFBCToSys *>(s->nvfbc_interface); | ||
| // NVFBC_TOSYS_SETUP_PARAMS setup = { ... }; | ||
| // setup.eBufferFormat = NVFBC_BUFFER_FORMAT_BGRA; | ||
| // setup.ppBuffer = &s->frame_buffer; | ||
| // toSys->NvFBCToSysSetUp(&setup); | ||
| // | ||
| // Without SDK headers, the interface pointer cannot be used. | ||
| // TODO: Add official Capture SDK headers and uncomment the above. | ||
|
|
||
| s->session_valid = true; | ||
| *session = reinterpret_cast<sunshine_capture_session_t>(s); | ||
| return 0; |
There was a problem hiding this comment.
在真正接通 GrabFrame 前，不要把会话标记为可用。
Line 284 把 session_valid 设成了 true 并返回成功，但 Lines 311-359 目前只会返回 SUNSHINE_CAPTURE_TIMEOUT。结合 src/platform/windows/capture_plugin/display_plugin.cpp 的 Lines 138-143，上层会一直循环空帧，用户看到的是“初始化成功但永远没画面”。
🛠️ 一种更安全的临时处理
- s->session_valid = true;
- *session = reinterpret_cast<sunshine_capture_session_t>(s);
- return 0;
+ FreeLibrary(s->nvfbc_dll);
+ delete s;
+ return -1;Also applies to: 311-359
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cpp` around lines 271 -
286, Do not mark the session as valid until we can actually grab frames: change
the logic around sunshine_nvfbc_plugin.cpp so s->session_valid is not set to
true and a successful 0 is not returned immediately in the setup path (where
*session is assigned and return 0 currently occurs); instead leave session_valid
false and return an appropriate "not ready" error (or propagate the GrabFrame
timeout behavior) and only set s->session_valid = true inside the first
successful GrabFrame/ToSys setup completion (the code path currently returning
SUNSHINE_CAPTURE_TIMEOUT in lines ~311-359 should be the state the caller sees
until a real frame is obtained); reference s->session_valid,
sunshine_capture_session_t, and the GrabFrame/INvFBCToSys setup code when making
the change.
| /** Frame data returned by sunshine_capture_next_frame(). */ | ||
| typedef struct { | ||
| uint8_t *data; /**< Pointer to frame data (system memory) or NULL for GPU frames */ | ||
| int width; /**< Frame width */ | ||
| int height; /**< Frame height */ | ||
| int pixel_pitch; /**< Bytes per pixel */ | ||
| int row_pitch; /**< Bytes per row */ | ||
| int64_t timestamp_ns; /**< Frame timestamp in nanoseconds (steady clock) */ | ||
|
|
||
| /** | ||
| * For DXGI memory type: ID3D11Texture2D* handle. | ||
| * For CUDA memory type: CUdeviceptr handle. | ||
| * Cast to appropriate type based on memory type. | ||
| */ | ||
| void *gpu_handle; | ||
|
|
||
| /** For DXGI: subresource index within the texture. */ | ||
| uint32_t gpu_subresource; | ||
| } sunshine_frame_t; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "capture_plugin_api.h" -o -name "display_plugin.cpp" | head -20Repository: AlkaidLab/foundation-sunshine
Length of output: 188
🏁 Script executed:
cat -n src/platform/windows/capture_plugin/capture_plugin_api.h | sed -n '84,102p'Repository: AlkaidLab/foundation-sunshine
Length of output: 927
🏁 Script executed:
cat -n src/platform/windows/capture_plugin/display_plugin.cpp | sed -n '120,135p'Repository: AlkaidLab/foundation-sunshine
Length of output: 784
🏁 Script executed:
rg "timestamp_ns" --type cpp -B 3 -A 3Repository: AlkaidLab/foundation-sunshine
Length of output: 1610
🏁 Script executed:
rg "steady_clock" src/platform/windows/ -B 2 -A 2Repository: AlkaidLab/foundation-sunshine
Length of output: 14812
🏁 Script executed:
rg "timestamp_ns\s*=" --type cpp -B 5 -A 2Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
find . -name "*.cpp" -path "*/capture_plugin/*" | head -20Repository: AlkaidLab/foundation-sunshine
Length of output: 193
🏁 Script executed:
grep -r "sunshine_frame_t" --include="*.cpp" -B 2 -A 2Repository: AlkaidLab/foundation-sunshine
Length of output: 1494
🏁 Script executed:
cat -n examples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cpp | grep -A 30 "timestamp_ns"Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
rg "\.timestamp_ns\s*=" --type cpp -B 3 -A 1Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
cat -n examples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cpp | head -200Repository: AlkaidLab/foundation-sunshine
Length of output: 8054
🏁 Script executed:
rg "frame\." src/platform/windows/capture_plugin/ --type cpp -A 2 | head -50Repository: AlkaidLab/foundation-sunshine
Length of output: 1823
🏁 Script executed:
find examples -name "*.cpp" | xargs grep -l "timestamp_ns" 2>/dev/nullRepository: AlkaidLab/foundation-sunshine
Length of output: 57
🏁 Script executed:
cat -n examples/capture_plugin_nvfbc/sunshine_nvfbc_plugin.cpp | sed -n '200,350p'Repository: AlkaidLab/foundation-sunshine
Length of output: 5709
🏁 Script executed:
rg "steady_clock" --type cpp -l | head -20Repository: AlkaidLab/foundation-sunshine
Length of output: 589
🏁 Script executed:
cat -n src/platform/windows/capture_plugin/display_plugin.cpp | sed -n '100,150p'Repository: AlkaidLab/foundation-sunshine
Length of output: 2163
🏁 Script executed:
cat -n src/platform/windows/capture_plugin/display_plugin.cpp | sed -n '1,50p'Repository: AlkaidLab/foundation-sunshine
Length of output: 1534
🏁 Script executed:
grep -r "next_frame" src/platform/windows/capture_plugin/ --include="*.h" -A 3Repository: AlkaidLab/foundation-sunshine
Length of output: 2279
🏁 Script executed:
rg "plugin.*vtable" src/platform/windows/capture_plugin/ -B 2 -A 2Repository: AlkaidLab/foundation-sunshine
Length of output: 7179
🏁 Script executed:
rg "time_since_epoch|steady_clock::now" src/platform/windows/ --type cpp -B 2 -A 2 | head -60Repository: AlkaidLab/foundation-sunshine
Length of output: 3892
🏁 Script executed:
cat -n src/platform/windows/capture_plugin/capture_plugin_api.h | sed -n '1,50p'Repository: AlkaidLab/foundation-sunshine
Length of output: 2295
🏁 Script executed:
grep -r "QPC\|QueryPerformance" src/platform/windows/ --include="*.cpp" --include="*.h" | head -20Repository: AlkaidLab/foundation-sunshine
Length of output: 500
🏁 Script executed:
cat -n src/platform/windows/misc.cpp | grep -A 10 -B 10 "QueryPerformance"Repository: AlkaidLab/foundation-sunshine
Length of output: 1178
🏁 Script executed:
cat -n src/platform/windows/display_ram.cpp | grep -A 5 -B 5 "qpc_time_difference"Repository: AlkaidLab/foundation-sunshine
Length of output: 634
🏁 Script executed:
rg "qpc_time_difference|qpc_counter" src/platform/windows/ --type cpp --type h -B 3 -A 3Repository: AlkaidLab/foundation-sunshine
Length of output: 3777
C ABI 中不应暴露 steady_clock 的 epoch 表示。
timestamp_ns 被定义为"steady clock 纳秒"，但此头文件声称支持跨编译器兼容性（MSVC、MinGW、Clang）。然而 std::chrono::steady_clock::time_since_epoch().count() 的具体值在不同工具链中并无标准保证，不同编译器可能使用不同的 epoch 表示。
在 display_plugin.cpp 第 126-128 行，宿主直接将 int64_t 值重建为 steady_clock::time_point：
plugin_img->frame_timestamp = std::chrono::steady_clock::time_point(
std::chrono::nanoseconds(frame.timestamp_ns));这样做假设了插件和宿主使用相同的 steady_clock 内部表示，跨编译器时此假设不成立。
建议改用 OS 级时间语义，例如用 QueryPerformanceCounter 的 ticks 加频率表达时间，或允许 timestamp_ns = 0 让宿主负责填充时间戳。宿主代码已有相应基础设施（qpc_time_difference）可支持此方案。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/capture_plugin/capture_plugin_api.h` around lines 84 -
102, sunshine_frame_t.timestamp_ns currently claims to be a "steady clock
nanoseconds" value but exposing steady_clock epoch across C ABI is not portable;
stop reconstructing std::chrono::steady_clock::time_point directly from
frame.timestamp_ns in display_plugin.cpp (the plugin_img->frame_timestamp
assignment). Instead, change the ABI semantics: document timestamp_ns as either
(a) 0 meaning "unset—host must fill timestamp using its local monotonic clock
via existing qpc_time_difference logic" or (b) an OS-level monotonic tick value
(e.g., QueryPerformanceCounter ticks or nanoseconds tied to the host's monotonic
clock) and then convert it to a steady_clock time_point using host-specific
conversion (use qpc_time_difference and QPC frequency where available). Update
the sunshine_frame_t comment to remove any claim about std::chrono::steady_clock
epoch, modify the consumer code around plugin_img->frame_timestamp to check for
timestamp_ns == 0 and use host qpc_time_difference to populate the timestamp,
and if non-zero perform an OS-specific conversion rather than direct
std::chrono::steady_clock reconstruction.
| * | ||
| * Plugin DLL must export as: sunshine_capture_get_info | ||
| */ | ||
| typedef int (*sunshine_capture_get_info_fn)(sunshine_capture_plugin_info_t *info); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "capture_plugin_api.h" -type fRepository: AlkaidLab/foundation-sunshine
Length of output: 131
🏁 Script executed:
head -250 src/platform/windows/capture_plugin/capture_plugin_api.h | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 10813
🏁 Script executed:
grep -n "CALL\|__stdcall\|__cdecl\|__fastcall\|calling" src/platform/windows/capture_plugin/capture_plugin_api.hRepository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
sed -n '120,130p' src/platform/windows/capture_plugin/capture_plugin_api.h | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 569
🏁 Script executed:
sed -n '130,180p' src/platform/windows/capture_plugin/capture_plugin_api.h | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 2240
🏁 Script executed:
sed -n '180,240p' src/platform/windows/capture_plugin/capture_plugin_api.h | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 2713
公开 ABI 缺少固定调用约定。
此处只有 extern "C" 但未指定 Windows 调用约定。extern "C" 仅处理名称修饰，不控制调用约定。若插件以 /Gz (stdcall) 或其他调用约定编译，与宿主的默认调用约定不符，会导致 GetProcAddress 解析失败或调用栈损坏。头部声称"跨编译器兼容性"但未通过明确的调用约定保证。建议定义 SUNSHINE_CAPTURE_CALL 宏（如 #define SUNSHINE_CAPTURE_CALL __cdecl），并将所有函数指针类型和导出函数声明统一采用此宏。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/capture_plugin/capture_plugin_api.h` at line 124, The
public ABI lacks an explicit Windows calling convention which can break
cross-compiler/plugin calls; define a calling-convention macro (e.g.,
SUNSHINE_CAPTURE_CALL set to __cdecl or chosen convention) and apply it
uniformly to all function pointer types and exported functions (for example,
update sunshine_capture_get_info_fn and any other sunshine_capture_* function
pointer typedefs and export declarations to use SUNSHINE_CAPTURE_CALL) so the
host and plugins share the same calling convention and avoid
GetProcAddress/stack corruption issues.
| std::filesystem::path | ||
| get_plugins_dir() { | ||
| wchar_t module_path[MAX_PATH]; | ||
| GetModuleFileNameW(nullptr, module_path, MAX_PATH); | ||
| return std::filesystem::path(module_path).parent_path() / "plugins"; | ||
| } |
There was a problem hiding this comment.
GetModuleFileNameW 返回值未检查，可能存在路径截断风险。
当路径超过 MAX_PATH 字符时，GetModuleFileNameW 会截断路径但不会报错。建议检查返回值：
🛡️ 建议的修复
std::filesystem::path
get_plugins_dir() {
wchar_t module_path[MAX_PATH];
- GetModuleFileNameW(nullptr, module_path, MAX_PATH);
+ DWORD len = GetModuleFileNameW(nullptr, module_path, MAX_PATH);
+ if (len == 0 || len >= MAX_PATH) {
+ BOOST_LOG(error) << "Failed to get module path or path too long";
+ return {};
+ }
return std::filesystem::path(module_path).parent_path() / "plugins";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| std::filesystem::path | |
| get_plugins_dir() { | |
| wchar_t module_path[MAX_PATH]; | |
| GetModuleFileNameW(nullptr, module_path, MAX_PATH); | |
| return std::filesystem::path(module_path).parent_path() / "plugins"; | |
| } | |
| std::filesystem::path | |
| get_plugins_dir() { | |
| wchar_t module_path[MAX_PATH]; | |
| DWORD len = GetModuleFileNameW(nullptr, module_path, MAX_PATH); | |
| if (len == 0 || len >= MAX_PATH) { | |
| BOOST_LOG(error) << "Failed to get module path or path too long"; | |
| return {}; | |
| } | |
| return std::filesystem::path(module_path).parent_path() / "plugins"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/capture_plugin/capture_plugin_loader.cpp` around lines
35 - 40, The call to GetModuleFileNameW in get_plugins_dir does not check its
return value and can silently truncate paths longer than MAX_PATH; update
get_plugins_dir to call GetModuleFileNameW with a dynamically sized wchar_t
buffer (e.g., std::vector<wchar_t>) and loop, doubling the buffer size until the
returned length is less than the buffer size (and nonzero), or stop on error; if
GetModuleFileNameW returns zero, handle/check GetLastError and propagate or log
an error instead of using a truncated module_path; reference get_plugins_dir,
GetModuleFileNameW, MAX_PATH and module_path when locating the code to change.
| for (const auto &entry : std::filesystem::directory_iterator(plugins_dir)) { | ||
| if (!entry.is_regular_file()) continue; | ||
|
|
||
| auto ext = entry.path().extension().string(); | ||
| std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower); | ||
| if (ext != ".dll") continue; | ||
|
|
||
| auto plugin = load_plugin(entry.path()); | ||
| if (plugin) { | ||
| plugins.push_back(std::move(plugin)); | ||
| } | ||
| } |
There was a problem hiding this comment.
目录遍历缺少异常处理。
std::filesystem::directory_iterator 在权限不足或路径不可访问时可能抛出异常。建议添加 try-catch：
🛡️ 建议的修复
- for (const auto &entry : std::filesystem::directory_iterator(plugins_dir)) {
- if (!entry.is_regular_file()) continue;
-
- auto ext = entry.path().extension().string();
- std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower);
- if (ext != ".dll") continue;
-
- auto plugin = load_plugin(entry.path());
- if (plugin) {
- plugins.push_back(std::move(plugin));
+ try {
+ for (const auto &entry : std::filesystem::directory_iterator(plugins_dir)) {
+ if (!entry.is_regular_file()) continue;
+
+ auto ext = entry.path().extension().string();
+ std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower);
+ if (ext != ".dll") continue;
+
+ auto plugin = load_plugin(entry.path());
+ if (plugin) {
+ plugins.push_back(std::move(plugin));
+ }
}
+ } catch (const std::filesystem::filesystem_error &e) {
+ BOOST_LOG(error) << "Error scanning plugins directory: " << e.what();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const auto &entry : std::filesystem::directory_iterator(plugins_dir)) { | |
| if (!entry.is_regular_file()) continue; | |
| auto ext = entry.path().extension().string(); | |
| std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower); | |
| if (ext != ".dll") continue; | |
| auto plugin = load_plugin(entry.path()); | |
| if (plugin) { | |
| plugins.push_back(std::move(plugin)); | |
| } | |
| } | |
| try { | |
| for (const auto &entry : std::filesystem::directory_iterator(plugins_dir)) { | |
| if (!entry.is_regular_file()) continue; | |
| auto ext = entry.path().extension().string(); | |
| std::transform(ext.begin(), ext.end(), ext.begin(), ::tolower); | |
| if (ext != ".dll") continue; | |
| auto plugin = load_plugin(entry.path()); | |
| if (plugin) { | |
| plugins.push_back(std::move(plugin)); | |
| } | |
| } | |
| } catch (const std::filesystem::filesystem_error &e) { | |
| BOOST_LOG(error) << "Error scanning plugins directory: " << e.what(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/capture_plugin/capture_plugin_loader.cpp` around lines
128 - 139, The directory iteration over std::filesystem::directory_iterator in
the plugin loading loop can throw (e.g., permissions or missing path); wrap the
iteration in a try-catch that catches std::filesystem::filesystem_error (and
optionally std::exception) around the loop that uses directory_iterator, log a
clear error via your logger and either abort plugin loading or return/continue
gracefully, and ensure resources remain consistent; update the block that calls
load_plugin(entry.path()) and pushes into plugins to run only when no exception
was thrown.
| sunshine_mem_type_e | ||
| to_plugin_mem_type(mem_type_e type) { | ||
| switch (type) { | ||
| case mem_type_e::system: | ||
| return SUNSHINE_MEM_SYSTEM; | ||
| case mem_type_e::dxgi: | ||
| return SUNSHINE_MEM_DXGI; | ||
| case mem_type_e::cuda: | ||
| return SUNSHINE_MEM_CUDA; | ||
| default: | ||
| return SUNSHINE_MEM_SYSTEM; | ||
| } |
There was a problem hiding this comment.
这里应先拒绝当前适配器还吃不下的内存类型。
Line 66 会把 mem_type_ 直接传给插件，但 Lines 114-124 只拷贝了 CPU 指针和 pitch，gpu_handle/gpu_subresource 完全没被消费；同时 to_plugin_mem_type() 的默认分支还会把未知类型静默回落到 SUNSHINE_MEM_SYSTEM。这会把 DXGI/CUDA 或未来新增类型伪装成 system-memory 路径，最终在编码链路里失配。
Also applies to: 66-70, 114-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/capture_plugin/display_plugin.cpp` around lines 19 - 30,
The adapter must reject or explicitly handle memory types it doesn't support
instead of silently mapping them to system memory; update the code path that
converts and forwards mem_type_ to the plugin by (1) changing
to_plugin_mem_type(mem_type_e) to not default to SUNSHINE_MEM_SYSTEM — return an
explicit invalid/unsupported value or assert/error on unknown enums, and (2) add
a runtime check before calling the plugin (where mem_type_ is passed) to
validate adapter support for that mem type and return an error if unsupported.
Also ensure the plugin buffer population (the block that currently only copies
CPU pointer and pitch and mentions gpu_handle/gpu_subresource) copies or
forwards gpu_handle and gpu_subresource for DXGI/CUDA paths so GPU-backed memory
isn’t misrouted; locate references to mem_type_, to_plugin_mem_type(), and the
code that sets gpu_handle/gpu_subresource and update those to propagate GPU
fields for non-system types and to fail early for unsupported/unknown types.
| display_plugin_t::~display_plugin_t() { | ||
| stop_flag_ = true; | ||
|
|
||
| if (session_) { | ||
| if (plugin_->vtable.interrupt) { | ||
| plugin_->vtable.interrupt(session_); | ||
| } | ||
| plugin_->vtable.destroy_session(session_); | ||
| session_ = nullptr; | ||
| } |
There was a problem hiding this comment.
不要在析构里立刻销毁 session_。
capture() 会把 plugin_img_t 通过 shared_ptr 交给外部，而 plugin_img_t::~plugin_img_t() 仍会用缓存的 session/vtable 调 release_frame()（见 src/platform/windows/capture_plugin/display_plugin.h 的 Lines 19-42）。这里直接 destroy_session(session_)，会让在途帧或稍后归还的帧回调到已销毁会话；另外析构也没有等待正在执行的 next_frame() 退出。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/capture_plugin/display_plugin.cpp` around lines 39 - 48,
The destructor currently calls plugin_->vtable.destroy_session(session_)
immediately, which can race with in-flight frames returned via capture() because
plugin_img_t::~plugin_img_t() still calls release_frame() using the cached
session/vtable; instead, set stop_flag_ and call
plugin_->vtable.interrupt(session_) as now, then wait for any background
next_frame() to exit (join the worker thread or wait on a condition) and for
in-flight frames to be returned (track outstanding frame count or use a shared
ownership/weak_ptr scheme), only after outstanding frames are zero and
next_frame() has stopped call plugin_->vtable.destroy_session(session_) and then
set session_ = nullptr; update display_plugin_t::~display_plugin_t(), any
thread/worker join logic, and ensure plugin_img_t holds only safe references
(weak/shared) to session/vtable to avoid use-after-free.
Summary
Add an extensible capture plugin system that allows external DLLs to provide screen capture backends for Sunshine on Windows.
Changes
Plugin System Core
capture_plugin_api.h: Pure C ABI interface (v1) for cross-compiler compatibility (MSVC/MinGW/Clang)capture_plugin_loader: Runtime discovery and loading of plugin DLLs fromplugins/directorydisplay_plugin: Adapter bridging plugin C ABI to Sunshine'sdisplay_tinterfacedisplay()factory: Extended to fall back to plugin loader for unrecognized capture type namesExample Plugin
examples/capture_plugin_nvfbc/: NvFBC skeleton plugin with standalone CMakeLists.txt, demonstrating the full plugin APIBuild Integration
Usage
capture_plugin_api.hplugins/directorycapture = <plugin_name>insunshine.confArchitecture
Next Steps
Summary by CodeRabbit
发布说明
新功能