feat(capture): add ZakoVDD direct shared-texture capture backend#609
Open
feat(capture): add ZakoVDD direct shared-texture capture backend#609
Conversation
New 'vdd' capture backend bypasses DXGI Desktop Duplication / WGC by opening the D3D11 NT-shared texture published by the ZakoVDD virtual display driver via Global\ZakoVDD_Frame_<idx> + KeyedMutex (0/1) + Global\ZakoVDD_FrameReady_<idx> event, with metadata (resolution, DXGI format, HDR meta, frame counter, QPC) carried in the Global\ZakoVDD_Meta_<idx> file mapping (SharedFrameMetadata, Magic 'ZVDF', Version 1). Highlights: - Works in SYSTEM/service context and before user logon (no UI session dependency, unlike WGC). - Zero-copy on the same D3D11 device as Sunshine's encoder pipeline: the shared texture is opened against the existing display_base_t device and CopyResource'd into the existing img_d3d_t pool. - Auto self-check on init: if no VDD monitor matches the selected display dimensions, send 'CREATEMONITOR' over the driver's NamedPipe and poll for up to ~3s before failing. Files: - src/platform/windows/display_vdd.cpp (new): vdd_capture_t (KeyedMutex + named-event consumer), monitor-index resolver, and display_vdd_vram_t::init(). - src/platform/windows/display_vram.cpp: display_vdd_vram_t::snapshot / release_snapshot (kept here to access file-local img_d3d_t and texture_lock_helper). - src/platform/windows/display.h: vdd_capture_t + display_vdd_vram_t declarations. - src/platform/windows/display_base.cpp: register 'vdd' as a dxgi backend in the factory. - src_assets/common/assets/web/configs/tabs/Advanced.vue: add 'vdd' option to the Windows capture method dropdown. - cmake/compile_definitions/windows.cmake: compile display_vdd.cpp.
Summary by CodeRabbit
Walkthrough为 Windows 引入 ZakoVDD（VDD）直接共享纹理捕获后端：新增初始化、事件驱动帧获取/释放、快照与 HDR 元数据传递，增加显示工厂选择项并在 UI 中加入该后端选项，同时将实现文件加入 Windows 构建。 Changes
Sequence Diagram(s)sequenceDiagram
participant 用户 as User
participant UI as UI
participant Factory as 显示工厂
participant VddCapture as VDD 捕获层\nvdd_capture_t
participant SharedMem as 共享元数据\nGlobal\ZakoVDD_Meta
participant D3D as D3D11 共享纹理\nIDXGIKeyedMutex
User->>UI: 选择 "vdd" 捕获方法
UI->>Factory: 请求初始化 vdd 后端
Factory->>VddCapture: 创建并 init(d3d_device, monitor_idx)
VddCapture->>SharedMem: 打开命名映射与事件
SharedMem-->>VddCapture: 返回元数据指针（尺寸/格式/HDR/时间）
VddCapture->>D3D: OpenSharedResourceByName 打开共享纹理
D3D-->>VddCapture: 返回纹理与 keyed mutex
loop 帧捕获循环
Producer->>SharedMem: 更新元数据与时间戳
Producer->>D3D: 触发帧就绪事件
VddCapture->>VddCapture: wait event / next_frame(timeout)
VddCapture->>D3D: Acquire keyed mutex
D3D-->>VddCapture: 锁已获得
VddCapture->>SharedMem: 读取帧元数据（HDR、QPC）
VddCapture-->>Factory: 返回纹理 指针 与 时间戳
Factory->>D3D: 在互斥保护下 CopyResource 到 capture 纹理
VddCapture->>D3D: Release keyed mutex
D3D-->>VddCapture: 锁已释放
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/platform/windows/display_vdd.cpp`:
- Around line 268-333: resolve_vdd_monitor_index only compares Width/Height so
when multiple VDD monitors share a resolution it always picks the first match
and display_vdd_vram_t::init runs CREATEMONITOR only after display_base_t::init
(which may fail before creation), causing wrong producer selection or never
creating a monitor; fix by changing resolve_vdd_monitor_index to match a stable
producer identifier exposed by the VDD metadata (e.g., monitor id, device name
or adapter LUID) instead of or in addition to Width/Height, then update
display_vdd_vram_t::init to perform the "ensure VDD exists / send CREATEMONITOR
via display_device::vdd_utils::execute_pipe_command(kVddPipeName, ...)" and
re-enumeration step before calling display_base_t::init (or at least before
relying on a matched index) and use width_before_rotation/height_before_rotation
only as a fallback when identifiers don’t match.
- Around line 342-344: Add a whitelist check in init(): after reading
dup.format() into capture_format, validate it against
display_vram_t::get_supported_capture_formats() and reject (return failure /
propagate error) any format not in that set (e.g., P010) so downstream code
(get_pixel_pitch(), complete_img(), RTV creation, shaders) won’t operate on
unsupported formats; include a clear error log mentioning the rejected
dup.format() and that only formats from get_supported_capture_formats() are
accepted, and document that full support for P010 requires extending capture
texture creation, RTV setup, and color conversion paths before allowing it.
In `@src/platform/windows/display_vram.cpp`:
- Around line 3439-3475: After dup.next_frame(...) returns ok and current_frame
is held, ensure the producer keyed mutex is released and current_frame COM
reference is freed on every error path (size/format mismatch, pull_free_image_cb
failure, complete_img failure, lock failure) by introducing an RAII guard (e.g.,
a small scope-guard object) that calls release_snapshot(current_frame) (and any
keyed-mutex release) in its destructor; alternatively call a shared cleanup
function before each non-ok return. Update locations around dup.next_frame, the
size/format check, the pull_free_image_cb check, complete_img(d3d_img.get()),
and texture_lock_helper lock failure to use that guard so current_frame and the
keyed mutex are always released even on early returns.
In `@src/platform/windows/display.h`:
- Around line 591-635: Implement HDR passthrough on display_vdd_vram_t by
overriding is_hdr() and get_hdr_metadata() to return the VDD capture's metadata
instead of querying DXGI; inside display_vdd_vram_t add overrides for bool
is_hdr() const and void get_hdr_metadata(SS_HDR_METADATA &out) const (or the
project-equivalent getter) that query dup.is_hdr(), dup.max_nits(),
dup.min_nits(), and dup.max_fall() and populate the SS_HDR_METADATA struct
accordingly, ensuring these methods are marked override so the producer
shared-memory HDR metadata is used.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35b3270f-e1f4-4043-b752-24e8c1d233a2
📒 Files selected for processing (6)
cmake/compile_definitions/windows.cmakesrc/platform/windows/display.hsrc/platform/windows/display_base.cppsrc/platform/windows/display_vdd.cppsrc/platform/windows/display_vram.cppsrc_assets/common/assets/web/configs/tabs/Advanced.vue
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (4)
cmake/**
⚙️ CodeRabbit configuration file
cmake/**: CMake 构建系统文件。审查跨平台兼容性、现代 CMake 实践。
Files:
cmake/compile_definitions/windows.cmake
src_assets/**/*.{vue,js,html}
⚙️ CodeRabbit configuration file
src_assets/**/*.{vue,js,html}: 基于 Vue.js 的 Web 配置面板。审查 XSS/CSRF 安全性、 组件设计、状态管理和可访问性。
Files:
src_assets/common/assets/web/configs/tabs/Advanced.vue
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/platform/windows/display_base.cppsrc/platform/windows/display_vdd.cppsrc/platform/windows/display.hsrc/platform/windows/display_vram.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_base.cppsrc/platform/windows/display_vdd.cppsrc/platform/windows/display.hsrc/platform/windows/display_vram.cpp
🔇 Additional comments (3)
src_assets/common/assets/web/configs/tabs/Advanced.vue (1)
173-173: 这里看起来没问题。新增的
vdd选项是静态文本/固定值，且只出现在 Windows 平台分支中；与后端新增的 capture 字符串一致。cmake/compile_definitions/windows.cmake (1)
88-88: 构建集成没问题。
display_vdd.cpp被加入 Windows 专用目标文件列表，范围与新后端的平台约束一致。src/platform/windows/display_base.cpp (1)
1127-1131: 工厂路由本身没问题。
vdd只在 DXGI/VRAM 路径下实例化，符合当前新增后端的实现范围。
Comment on lines
+268
to
+333
| static int | ||
| resolve_vdd_monitor_index(unsigned int target_w, unsigned int target_h, unsigned int max_probe = 16) { | ||
| for (unsigned int i = 0; i < max_probe; ++i) { | ||
| std::wstring meta_name = L"Global\\ZakoVDD_Meta_" + std::to_wstring(i); | ||
| HANDLE h = OpenFileMappingW(FILE_MAP_READ, FALSE, meta_name.c_str()); | ||
| if (!h) continue; | ||
| void *p = MapViewOfFile(h, FILE_MAP_READ, 0, 0, sizeof(SharedFrameMetadata)); | ||
| if (!p) { | ||
| CloseHandle(h); | ||
| continue; | ||
| } | ||
| auto *meta = static_cast<const SharedFrameMetadata *>(p); | ||
| bool match = (meta->Magic == VDD_META_MAGIC) && | ||
| (meta->Width == target_w) && | ||
| (meta->Height == target_h); | ||
| UnmapViewOfFile(p); | ||
| CloseHandle(h); | ||
| if (match) { | ||
| BOOST_LOG(info) << "[vdd] resolved monitor index "sv << i | ||
| << " for "sv << target_w << "x"sv << target_h; | ||
| return static_cast<int>(i); | ||
| } | ||
| } | ||
| return -1; | ||
| } | ||
|
|
||
| int | ||
| display_vdd_vram_t::init(const ::video::config_t &config, const std::string &display_name) { | ||
| if (display_base_t::init(config, display_name)) { | ||
| BOOST_LOG(error) << "[vdd] display_base_t::init failed for "sv << display_name; | ||
| return -1; | ||
| } | ||
|
|
||
| // Try to identify which VDD monitor backs this DXGI output by matching | ||
| // dimensions. width/height come from DXGI DesktopCoordinates / orientation. | ||
| int idx = resolve_vdd_monitor_index(static_cast<unsigned>(width_before_rotation), | ||
| static_cast<unsigned>(height_before_rotation)); | ||
| if (idx < 0) { | ||
| // Self-check: the selected display either isn't a VDD monitor at all, or | ||
| // VDD has no active monitors yet. Try to ask the driver to create one | ||
| // via its NamedPipe, then poll the meta mapping again. | ||
| BOOST_LOG(info) << "[vdd] no matching VDD monitor; sending CREATEMONITOR via NamedPipe..."sv; | ||
| std::string response; | ||
| bool timed_out = false; | ||
| bool sent = display_device::vdd_utils::execute_pipe_command( | ||
| display_device::vdd_utils::kVddPipeName, L"CREATEMONITOR", &response, &timed_out); | ||
| if (!sent) { | ||
| BOOST_LOG(warning) << "[vdd] CREATEMONITOR failed (driver not running or pipe unreachable). "sv | ||
| << "Is the selected display a ZakoVDD virtual display?"sv; | ||
| return -1; | ||
| } | ||
| // Poll for up to ~3s for a producer to appear. | ||
| using namespace std::chrono_literals; | ||
| const auto deadline = std::chrono::steady_clock::now() + 3s; | ||
| while (std::chrono::steady_clock::now() < deadline) { | ||
| std::this_thread::sleep_for(200ms); | ||
| idx = resolve_vdd_monitor_index(static_cast<unsigned>(width_before_rotation), | ||
| static_cast<unsigned>(height_before_rotation)); | ||
| if (idx >= 0) break; | ||
| } | ||
| if (idx < 0) { | ||
| BOOST_LOG(warning) << "[vdd] CREATEMONITOR succeeded but no monitor matches "sv | ||
| << width_before_rotation << "x"sv << height_before_rotation | ||
| << " within 3s."sv; | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
不要只用分辨率解析 VDD monitor。
resolve_vdd_monitor_index() 只匹配 Width/Height，多个 VDD 显示器同分辨率时会稳定选到第一个匹配项，导致用户选择的显示器和实际捕获的 producer 不一致。另外，CREATEMONITOR 发生在 display_base_t::init() 之后；如果登录前/无可枚举输出时 base init 失败，就根本到不了自创建逻辑。
建议用 producer 暴露的稳定标识匹配 DXGI output（例如 monitor id/device name/adapter LUID），并把“无 VDD 时创建并重新枚举”的流程放到依赖具体 output 初始化之前。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/display_vdd.cpp` around lines 268 - 333,
resolve_vdd_monitor_index only compares Width/Height so when multiple VDD
monitors share a resolution it always picks the first match and
display_vdd_vram_t::init runs CREATEMONITOR only after display_base_t::init
(which may fail before creation), causing wrong producer selection or never
creating a monitor; fix by changing resolve_vdd_monitor_index to match a stable
producer identifier exposed by the VDD metadata (e.g., monitor id, device name
or adapter LUID) instead of or in addition to Width/Height, then update
display_vdd_vram_t::init to perform the "ensure VDD exists / send CREATEMONITOR
via display_device::vdd_utils::execute_pipe_command(kVddPipeName, ...)" and
re-enumeration step before calling display_base_t::init (or at least before
relying on a matched index) and use width_before_rotation/height_before_rotation
only as a fallback when identifiers don’t match.
Comment on lines
+591
to
+635
| UINT width() const { return m_width; } | ||
| UINT height() const { return m_height; } | ||
| DXGI_FORMAT format() const { return m_format; } | ||
| bool is_hdr() const { return m_is_hdr; } | ||
| float max_nits() const { return m_max_nits; } | ||
| float min_nits() const { return m_min_nits; } | ||
| float max_fall() const { return m_max_fall; } | ||
|
|
||
| private: | ||
| void close(); | ||
|
|
||
| HANDLE m_hMeta = nullptr; | ||
| void *m_pMeta = nullptr; | ||
| HANDLE m_hEvent = nullptr; | ||
| texture2d_t m_sharedTex; | ||
| keyed_mutex_t m_keyedMutex; | ||
| bool m_holdsKey = false; | ||
|
|
||
| UINT m_width = 0; | ||
| UINT m_height = 0; | ||
| DXGI_FORMAT m_format = DXGI_FORMAT_UNKNOWN; | ||
| bool m_is_hdr = false; | ||
| float m_max_nits = 0.0f; | ||
| float m_min_nits = 0.0f; | ||
| float m_max_fall = 0.0f; | ||
| UINT64 m_lastFrameCounter = 0; | ||
| }; | ||
|
|
||
| /** | ||
| * Display backend that consumes frames from the ZakoVDD virtual display | ||
| * driver via vdd_capture_t. Mirrors display_amd_vram_t / display_wgc_vram_t. | ||
| */ | ||
| class display_vdd_vram_t: public display_vram_t { | ||
| vdd_capture_t dup; | ||
| unsigned int monitor_idx = 0; | ||
| ID3D11Texture2D *current_frame = nullptr; // Borrowed ref from dup.next_frame, released in release_snapshot | ||
|
|
||
| public: | ||
| int | ||
| init(const ::video::config_t &config, const std::string &display_name); | ||
| capture_e | ||
| snapshot(const pull_free_image_cb_t &pull_free_image_cb, std::shared_ptr<platf::img_t> &img_out, std::chrono::milliseconds timeout, bool cursor_visible) override; | ||
| capture_e | ||
| release_snapshot() override; | ||
| }; |
There was a problem hiding this comment.
把 VDD 的 HDR 元数据接到 display 接口上。
这里暴露了 vdd_capture_t 的 HDR 字段，但 display_vdd_vram_t 没有覆盖 is_hdr() / get_hdr_metadata()；当前会继续走 display_base_t 的 DXGI output 查询，导致 producer 共享内存里的 HDR 元数据被忽略。
建议在 display_vdd_vram_t 上覆盖这两个方法，并用 dup.is_hdr() / dup.max_nits() / dup.min_nits() / dup.max_fall() 填充 SS_HDR_METADATA。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/display.h` around lines 591 - 635, Implement HDR
passthrough on display_vdd_vram_t by overriding is_hdr() and get_hdr_metadata()
to return the VDD capture's metadata instead of querying DXGI; inside
display_vdd_vram_t add overrides for bool is_hdr() const and void
get_hdr_metadata(SS_HDR_METADATA &out) const (or the project-equivalent getter)
that query dup.is_hdr(), dup.max_nits(), dup.min_nits(), and dup.max_fall() and
populate the SS_HDR_METADATA struct accordingly, ensuring these methods are
marked override so the producer shared-memory HDR metadata is used.
…t, RAII cleanup) - display_vdd_vram_t: override is_hdr() / get_hdr_metadata() so the producer-reported MaxNits / MinNits / MaxFALL from SharedFrameMetadata are surfaced to clients, instead of falling back to the DXGI output query path (which doesn't reliably propagate static HDR meta on virtual displays). Primaries reported as Rec. 2020 + D65 to match display_base_t::get_hdr_metadata() conventions. - display_vdd_vram_t::init(): validate the producer-reported capture format against display_vram_t::get_supported_capture_formats() and reject anything outside the whitelist (e.g. P010), with a clear log noting that adding such a format requires extending RTV / shader / color-conversion paths. - display_vdd_vram_t::snapshot(): wrap the borrowed COM ref + producer-side keyed-mutex hold in a util::fail_guard so they are released on every early-return path (size/format mismatch, pull_free_image_cb failure, complete_img failure, lock failure), fixing a leak when those errors occur.
Collaborator
Author
|
Addressed CodeRabbit review in 6daea96:
Not adopted (with rationale):
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/platform/windows/display_vdd.cpp (1)
268-333:⚠️ Potential issue | 🟠 Major不要只用分辨率解析 VDD monitor，并把自创建流程提前。
当前只按
Width/Height选择第一个 metadata，多个同分辨率 VDD 或物理显示器同尺寸时会错配；同时CREATEMONITOR发生在display_base_t::init()之后，无 VDD 输出可枚举时可能已经提前失败。建议改用 producer 暴露的稳定标识（monitor id/device name/adapter LUID 等）匹配，并在依赖具体 DXGI output 初始化前完成“确保 VDD 存在 + 重新枚举”流程。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/display_vdd.cpp` around lines 268 - 333, The current logic in display_vdd_vram_t::init and resolve_vdd_monitor_index incorrectly selects VDD monitors solely by Width/Height, causing mismatches when multiple displays share the same resolution and also sends the CREATEMONITOR command only after display_base_t::init has already run; fix by changing the discovery flow so you ensure the VDD producer exists and re-enumerate outputs before calling display_base_t::init: call display_device::vdd_utils::execute_pipe_command(kVddPipeName, L"CREATEMONITOR", ...) earlier (and poll) to guarantee the driver has created producers, then update resolve_vdd_monitor_index to match SharedFrameMetadata against a stable identifier exposed by the producer (e.g., monitor id / device name / adapter LUID present in SharedFrameMetadata) instead of only Width/Height, and use that stable id to pick the correct index when mapping metadata.
🧹 Nitpick comments (1)
src/platform/windows/display_vdd.cpp (1)
33-47: 为共享元数据 ABI 加编译期布局保护。这个 struct 直接映射 producer 的 MMF；字段顺序、填充或大小变化会静默破坏读取。建议加
sizeof/offsetof断言锁住当前二进制布局。建议补充布局断言
+#include <cstddef> + struct SharedFrameMetadata { UINT32 Magic; // 'ZVDF' = 0x5A564446 UINT32 Version; // 1 @@ UINT64 FrameCounter; UINT64 LastPresentQpc; }; + + static_assert(sizeof(SharedFrameMetadata) == 56); + static_assert(offsetof(SharedFrameMetadata, Magic) == 0); + static_assert(offsetof(SharedFrameMetadata, Version) == 4); + static_assert(offsetof(SharedFrameMetadata, Width) == 8); + static_assert(offsetof(SharedFrameMetadata, Height) == 12); + static_assert(offsetof(SharedFrameMetadata, DxgiFormat) == 16); + static_assert(offsetof(SharedFrameMetadata, IsHdr) == 20); + static_assert(offsetof(SharedFrameMetadata, MaxNits) == 24); + static_assert(offsetof(SharedFrameMetadata, MinNits) == 28); + static_assert(offsetof(SharedFrameMetadata, MaxFALL) == 32); + static_assert(offsetof(SharedFrameMetadata, FrameCounter) == 40); + static_assert(offsetof(SharedFrameMetadata, LastPresentQpc) == 48);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/display_vdd.cpp` around lines 33 - 47, The SharedFrameMetadata struct is mapped directly to the producer MMF and needs compile-time layout guards; add static_asserts after the struct (referencing SharedFrameMetadata) that check sizeof(SharedFrameMetadata) equals the expected byte size and use offsetof(SharedFrameMetadata, Magic), offsetof(..., Version), offsetof(..., Width), offsetof(..., Height), offsetof(..., FrameCounter) and offsetof(..., LastPresentQpc) to lock each field's offset, and include the necessary headers (e.g., <cstddef>) so these asserts compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/platform/windows/display_vdd.cpp`:
- Around line 397-403: The HDR luminance floats from dup are not validated
before casting and can be NaN/inf/negative/out-of-range; update the assignment
logic around metadata.maxDisplayLuminance, metadata.minDisplayLuminance, and
metadata.maxFullFrameLuminance to: validate each dup value with std::isfinite(),
treat non-finite or negative values as 0, scale min_nits by 10000.0f before
conversion, and clamp results to the destination integer ranges (0..UINT16_MAX
for maxDisplayLuminance and maxFullFrameLuminance; 0..UINT32_MAX for
minDisplayLuminance) prior to static_cast to avoid undefined behavior. Use
helpers or std::clamp to centralize the finite-check + clamp behavior and update
the three assignments to use those safe, clamped values.
- Around line 342-362: The producer format is reset to dup.format() but
capture_linear_gamma isn't updated, so FP16 scRGB producers get treated as
non-linear; after assigning capture_format in display_vdd (where capture_format
= dup.format()), detect FP16/scRGB formats (e.g., DXGI_FORMAT_R16G16B16A16_FLOAT
and any other FP16 formats your code treats as linear) and set
capture_linear_gamma = true accordingly (so complete_img() and downstream
shaders/analyzers see linear scRGB); update the code near capture_format
assignment to compute/set capture_linear_gamma based on capture_format using the
same DXGI-format checks/helpers used elsewhere.
---
Duplicate comments:
In `@src/platform/windows/display_vdd.cpp`:
- Around line 268-333: The current logic in display_vdd_vram_t::init and
resolve_vdd_monitor_index incorrectly selects VDD monitors solely by
Width/Height, causing mismatches when multiple displays share the same
resolution and also sends the CREATEMONITOR command only after
display_base_t::init has already run; fix by changing the discovery flow so you
ensure the VDD producer exists and re-enumerate outputs before calling
display_base_t::init: call
display_device::vdd_utils::execute_pipe_command(kVddPipeName, L"CREATEMONITOR",
...) earlier (and poll) to guarantee the driver has created producers, then
update resolve_vdd_monitor_index to match SharedFrameMetadata against a stable
identifier exposed by the producer (e.g., monitor id / device name / adapter
LUID present in SharedFrameMetadata) instead of only Width/Height, and use that
stable id to pick the correct index when mapping metadata.
---
Nitpick comments:
In `@src/platform/windows/display_vdd.cpp`:
- Around line 33-47: The SharedFrameMetadata struct is mapped directly to the
producer MMF and needs compile-time layout guards; add static_asserts after the
struct (referencing SharedFrameMetadata) that check sizeof(SharedFrameMetadata)
equals the expected byte size and use offsetof(SharedFrameMetadata, Magic),
offsetof(..., Version), offsetof(..., Width), offsetof(..., Height),
offsetof(..., FrameCounter) and offsetof(..., LastPresentQpc) to lock each
field's offset, and include the necessary headers (e.g., <cstddef>) so these
asserts compile.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff057c80-8471-4b1b-aba1-f18d99117ff4
📒 Files selected for processing (3)
src/platform/windows/display.hsrc/platform/windows/display_vdd.cppsrc/platform/windows/display_vram.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/platform/windows/display.h
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/platform/windows/display_vram.cppsrc/platform/windows/display_vdd.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_vram.cppsrc/platform/windows/display_vdd.cpp
🔇 Additional comments (1)
src/platform/windows/display_vram.cpp (1)
3427-3504: VDD snapshot 的早退清理看起来已经到位。
fail_guard覆盖了dup.next_frame()成功后的所有错误返回路径，成功路径再交给release_snapshot()释放，避免了 producer keyed mutex 和current_frameCOM ref 泄漏。
Comment on lines
+342
to
+362
| // Use producer-reported format as our capture format. complete_img() / image | ||
| // pool will be created against this format. | ||
| capture_format = dup.format(); | ||
|
|
||
| // Validate the producer-reported format against the formats display_vram_t | ||
| // can actually consume (RTV creation, shaders, color conversion). Anything | ||
| // outside this whitelist would crash later in capture/encoding paths. | ||
| { | ||
| auto supported = get_supported_capture_formats(); | ||
| bool ok = false; | ||
| for (auto f : supported) { | ||
| if (f == capture_format) { ok = true; break; } | ||
| } | ||
| if (!ok) { | ||
| BOOST_LOG(error) << "[vdd] producer format "sv | ||
| << dxgi_format_to_string(capture_format) | ||
| << " is not in display_vram_t::get_supported_capture_formats(); "sv | ||
| << "rejecting. Extending RTV/shader paths is required to add it."sv; | ||
| return -1; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 验证 capture_linear_gamma 的赋值路径，以及 img.linear_gamma 的消费点。
rg -n -C3 '\bcapture_linear_gamma\b|img\.linear_gamma|input_is_linear_fp16' --iglob '*.cpp' --iglob '*.h'Repository: AlkaidLab/foundation-sunshine
Length of output: 6755
🏁 Script executed:
# Get more context around the VDD code at lines 342-362
sed -n '320,380p' src/platform/windows/display_vdd.cpp | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 2885
🏁 Script executed:
# Find the class definition and inheritance
rg -n 'class.*VDD|class display_vram_vdd_t|struct display_vram_vdd_t' --iglob '*.cpp' --iglob '*.h' -A5Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
# Check if there's an init() or similar method in VDD that sets capture_linear_gamma
rg -n 'capture_linear_gamma' src/platform/windows/display_vdd.cpp -B5 -A5Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
# Find VDD-related files
fd -i 'vdd' --type fRepository: AlkaidLab/foundation-sunshine
Length of output: 315
🏁 Script executed:
# Search for class definition with less strict pattern
rg -n 'display_vdd_vram_t|display_vram_vdd' --iglob '*.h'Repository: AlkaidLab/foundation-sunshine
Length of output: 159
🏁 Script executed:
# Look at the beginning of display_vdd.cpp to understand structure
head -100 src/platform/windows/display_vdd.cpp | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 3716
🏁 Script executed:
# Look at the display_vdd_vram_t class definition
sed -n '623,700p' src/platform/windows/display.h | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 1129
🏁 Script executed:
# Find the init() method in VDD (looking for the full context around lines 342-362)
grep -n "^\s*int" src/platform/windows/display_vdd.cpp | head -20Repository: AlkaidLab/foundation-sunshine
Length of output: 181
🏁 Script executed:
# Look at more context before and after line 342 to see the method signature
sed -n '300,380p' src/platform/windows/display_vdd.cpp | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 4201
🏁 Script executed:
# Check what display_vram_t init() does and where capture_linear_gamma is normally set
rg -n 'class display_vram_t' --iglob '*.h' -A20Repository: AlkaidLab/foundation-sunshine
Length of output: 1508
🏁 Script executed:
# Look for the parent init() method in display_vram.cpp or display_base.cpp
rg -n 'display_vram_t::init|display_base_t::init' --iglob '*.cpp' -A30 | head -100Repository: AlkaidLab/foundation-sunshine
Length of output: 8570
🏁 Script executed:
# Check if display_vdd_vram_t calls parent init() or super().init()
rg -n 'display_vdd_vram_t::init' src/platform/windows/display_vdd.cpp -A50 | head -80Repository: AlkaidLab/foundation-sunshine
Length of output: 2798
🏁 Script executed:
# Check what vdd_capture_t::format() returns and if there's colorspace metadata
rg -n 'vdd_capture_t::format|\.format()' src/platform/windows/display_vdd.cpp -B2 -A2Repository: AlkaidLab/foundation-sunshine
Length of output: 339
🏁 Script executed:
# Check the SharedFrameMetadata struct and what info is available from the producer
sed -n '35,47p' src/platform/windows/display_vdd.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 384
🏁 Script executed:
# Look for any place where capture_linear_gamma might be set in VDD flow
rg -n 'capture_linear_gamma\s*=' src/platform/windows/ -B2 -A2Repository: AlkaidLab/foundation-sunshine
Length of output: 1704
🏁 Script executed:
# Check what dup.is_hdr() tells us and if it relates to colorspace/gamma
rg -n 'is_hdr\(\)|IsHdr' src/platform/windows/display_vdd.cpp -B3 -A3Repository: AlkaidLab/foundation-sunshine
Length of output: 1348
🏁 Script executed:
# See how is_hdr() is implemented and what it returns
rg -n 'vdd_capture_t::is_hdr' src/platform/windows/ -B2 -A10Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
# Check the complete flow: does display_base_t::init determine colorspace based on DXGI output?
sed -n '800,850p' src/platform/windows/display_base.cpp | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 2768
🏁 Script executed:
# Check what happens when display_base_t::init queries DXGI output for a VDD virtual display
# Look for where output6 is obtained
rg -n 'output6.*GetDesc1|DXGI_OUTPUT_DESC1' src/platform/windows/display_base.cpp -B5 -A5Repository: AlkaidLab/foundation-sunshine
Length of output: 1956
🏁 Script executed:
# Check if VDD provides any colorspace information or if it's only format + IsHdr
grep -n 'ColorSpace\|RGB_FULL_G' src/platform/windows/display_vdd.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
# Understand: does display_base_t::init() work correctly for virtual displays?
# Let's check if there's special handling for VDD in display_base_t::init
rg -n 'vdd|virtual.*display|VDD' src/platform/windows/display_base.cpp -iRepository: AlkaidLab/foundation-sunshine
Length of output: 690
🏁 Script executed:
# Look for similar heuristics that relate format to linear gamma
rg -n 'DXGI_FORMAT_R16G16B16A16_FLOAT.*linear|linear.*R16G16B16A16_FLOAT' src/platform/windows/ -B2 -A2Repository: AlkaidLab/foundation-sunshine
Length of output: 1603
🏁 Script executed:
# Check if there's any documentation or comments about VDD and FP16 format meaning
rg -n 'FP16|R16G16B16A16_FLOAT' src/platform/windows/display_vdd.cpp -B3 -A3Repository: AlkaidLab/foundation-sunshine
Length of output: 475
🏁 Script executed:
# See if complete_img actually uses img.linear_gamma and how it affects downstream
sed -n '3010,3030p' src/platform/windows/display_vram.cpp | cat -nRepository: AlkaidLab/foundation-sunshine
Length of output: 1004
同步设置 FP16 捕获的 capture_linear_gamma。
VDD 在 display_base_t::init() 之后重新设置了 capture_format，但 capture_linear_gamma 已根据 DXGI 输出的 ColorSpace 确定。对于 VDD 虚拟显示器，DXGI 可能报告不准确的 ColorSpace。当生产者发送 FP16 格式时，这总是表示线性 scRGB，但如果 DXGI 报告默认的 G22 colorspace，capture_linear_gamma 会被设为 false，导致 complete_img() 将 img.linear_gamma 设为 false。这会造成下游 RGB→YUV shader 和 HDR luminance analyzer 错误地将 scRGB FP16 数据当作非线性输入处理。
建议修正
// Use producer-reported format as our capture format. complete_img() / image
// pool will be created against this format.
capture_format = dup.format();
+ capture_linear_gamma = (capture_format == DXGI_FORMAT_R16G16B16A16_FLOAT);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/display_vdd.cpp` around lines 342 - 362, The producer
format is reset to dup.format() but capture_linear_gamma isn't updated, so FP16
scRGB producers get treated as non-linear; after assigning capture_format in
display_vdd (where capture_format = dup.format()), detect FP16/scRGB formats
(e.g., DXGI_FORMAT_R16G16B16A16_FLOAT and any other FP16 formats your code
treats as linear) and set capture_linear_gamma = true accordingly (so
complete_img() and downstream shaders/analyzers see linear scRGB); update the
code near capture_format assignment to compute/set capture_linear_gamma based on
capture_format using the same DXGI-format checks/helpers used elsewhere.
Comment on lines
+397
to
+403
| // Producer-reported luminance, in nits. SS_HDR_METADATA expects: | ||
| // maxDisplayLuminance : nits | ||
| // minDisplayLuminance : units of 0.0001 nits | ||
| // maxFullFrameLuminance : nits | ||
| metadata.maxDisplayLuminance = static_cast<uint16_t>(dup.max_nits()); | ||
| metadata.minDisplayLuminance = static_cast<uint32_t>(dup.min_nits() * 10000.0f); | ||
| metadata.maxFullFrameLuminance = static_cast<uint16_t>(dup.max_fall()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查找 VDD HDR luminance 字段的来源和写入 SS_HDR_METADATA 的路径。
rg -n -C3 'MaxNits|MinNits|MaxFALL|max_nits|min_nits|max_fall|maxDisplayLuminance|minDisplayLuminance|maxFullFrameLuminance' --iglob '*.cpp' --iglob '*.h'Repository: AlkaidLab/foundation-sunshine
Length of output: 21429
🏁 Script executed:
# First, check the broader context around lines 397-403 in display_vdd.cpp
cat -n src/platform/windows/display_vdd.cpp | sed -n '370,420p'Repository: AlkaidLab/foundation-sunshine
Length of output: 2253
🏁 Script executed:
# Check if there's any validation in the build_hdr_metadata or related functions
rg -n -B5 -A5 'build_hdr_metadata|maxDisplayLuminance.*=' src/platform/windows/display_vdd.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 673
🏁 Script executed:
# Check where max_nits, min_nits, max_fall are defined and if they have bounds
rg -n 'float.*max_nits|float.*min_nits|float.*max_fall' src/platform/windows/display.h src/platform/windows/display_vdd.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 525
🏁 Script executed:
# Check if there's any validation when reading from shared memory (meta->MaxNits, etc.)
rg -n -B10 'meta->MaxNits|meta->MinNits|meta->MaxFALL' src/platform/windows/display_vdd.cpp | head -50Repository: AlkaidLab/foundation-sunshine
Length of output: 1144
🏁 Script executed:
# Check how values are parsed from query parameters in nvhttp.cpp (std::stof can produce NaN)
cat -n src/nvhttp.cpp | sed -n '460,470p'Repository: AlkaidLab/foundation-sunshine
Length of output: 896
🏁 Script executed:
# Verify the mathematical bound for minDisplayLuminance: uint32_t max = 2^32-1
python3 << 'EOF'
# minDisplayLuminance is uint32_t in units of 0.0001 nits
# Max uint32_t value = 4294967295
# So max actual nits value before multiplying by 10000 = 4294967295 / 10000
max_uint32 = 2**32 - 1
upper_bound_nits = max_uint32 / 10000.0
print(f"uint32_t max: {max_uint32}")
print(f"Upper bound for min_nits before *10000: {upper_bound_nits}")
# For maxDisplayLuminance: uint16_t max = 2^16-1
max_uint16 = 2**16 - 1
print(f"\nuint16_t max: {max_uint16}")
EOFRepository: AlkaidLab/foundation-sunshine
Length of output: 170
🏁 Script executed:
# Check if there's any validation when parsing query params from client
rg -n -B2 -A2 'std::stof.*maxBrightness|std::stof.*minBrightness|std::stof.*maxAverageBrightness' src/nvhttp.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 593
🏁 Script executed:
# Check if display_vdd values are ever validated or clamped elsewhere in the pipeline
rg -n -B5 -A5 'max_nits\(\)|min_nits\(\)|max_fall\(\)' src/platform/windows/Repository: AlkaidLab/foundation-sunshine
Length of output: 2107
🏁 Script executed:
# Check if std::stof is wrapped in try-catch or has error handling
rg -n -B5 -A5 'try|catch.*stof|invalid_argument|out_of_range' src/nvhttp.cpp | grep -A10 -B10 'stof'Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
# Check the actual default values and whether they're reasonable
cat -n src/rtsp.h | sed -n '40,50p'Repository: AlkaidLab/foundation-sunshine
Length of output: 400
🏁 Script executed:
# Look for any clamps or validation in stream setup that uses these values
rg -n -B3 -A3 'launch_session->max_nits|launch_session->min_nits|launch_session->max_full_nits' src/ -t cpp -t h | head -60Repository: AlkaidLab/foundation-sunshine
Length of output: 883
🏁 Script executed:
# Check if there's any exception handling for std::stof in a wider context
cat -n src/nvhttp.cpp | sed -n '450,475p'Repository: AlkaidLab/foundation-sunshine
Length of output: 1867
🏁 Script executed:
# Verify there's no validation in the vdd_utils or other setup code
rg -n 'isfinite|isnan|clamp|std::min|std::max' src/platform/windows/display_vdd.cpp src/display_device/Repository: AlkaidLab/foundation-sunshine
Length of output: 244
在无符号整型转换前，对 float HDR 亮度值添加有限性和范围校验。
这些值来自两个不可信源：客户端查询参数（通过 std::stof() 解析，可能失败）和共享内存（来自 VDD 驱动）。直接转换到 uint16_t/uint32_t 时，若值为 NaN、无穷大、负数或超出目标整数范围，会导致未定义行为和可移植性问题，进而向编码器发送错误的 HDR 元数据。
建议添加有限性检查和安全上界夹紧：
建议修正
+#include <algorithm>
+#include <cmath>
+
// Producer-reported luminance, in nits. SS_HDR_METADATA expects:
// maxDisplayLuminance : nits
// minDisplayLuminance : units of 0.0001 nits
// maxFullFrameLuminance : nits
- metadata.maxDisplayLuminance = static_cast<uint16_t>(dup.max_nits());
- metadata.minDisplayLuminance = static_cast<uint32_t>(dup.min_nits() * 10000.0f);
- metadata.maxFullFrameLuminance = static_cast<uint16_t>(dup.max_fall());
+ auto clamp_nits = [](float value, float upper) {
+ if (!std::isfinite(value) || value < 0.0f) {
+ return 0.0f;
+ }
+ return std::min(value, upper);
+ };
+
+ metadata.maxDisplayLuminance = static_cast<uint16_t>(clamp_nits(dup.max_nits(), 65535.0f));
+ metadata.minDisplayLuminance = static_cast<uint32_t>(clamp_nits(dup.min_nits(), 429496.7295f) * 10000.0f);
+ metadata.maxFullFrameLuminance = static_cast<uint16_t>(clamp_nits(dup.max_fall(), 65535.0f));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/display_vdd.cpp` around lines 397 - 403, The HDR
luminance floats from dup are not validated before casting and can be
NaN/inf/negative/out-of-range; update the assignment logic around
metadata.maxDisplayLuminance, metadata.minDisplayLuminance, and
metadata.maxFullFrameLuminance to: validate each dup value with std::isfinite(),
treat non-finite or negative values as 0, scale min_nits by 10000.0f before
conversion, and clamp results to the destination integer ranges (0..UINT16_MAX
for maxDisplayLuminance and maxFullFrameLuminance; 0..UINT32_MAX for
minDisplayLuminance) prior to static_cast to avoid undefined behavior. Use
helpers or std::clamp to centralize the finite-check + clamp behavior and update
the three assignments to use those safe, clamped values.
- Remove the in-init CREATEMONITOR + 3s NamedPipe poll. Sunshine's display_device::prepare_vdd already manages monitor lifecycle, so triggering CREATEMONITOR per encoder probe just wasted ~21s every startup with no real benefit. - resolve_vdd_monitor_index() now logs each probed Meta_<i> and falls back to the only valid producer when no exact W*H match exists. The producer only updates SharedFrameMetadata.Width/Height inside EnsureSharedTexture (called from PushFrame), so the meta still shows the prior mode until the first frame after a mode change. Returning the sole producer lets dup.init succeed; vdd_capture_t::next_frame already issues capture_e::reinit when producer dimensions change, letting the upper layer renegotiate without manual probing. - Drop the now-unused vdd_utils.h include.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/platform/windows/display_vdd.cpp (1)
200-220:WaitForSingleObject与AcquireSync各自消耗全量timeout，最坏总等待可达 2×timeout。当
WaitForSingleObject接近超时才返回，AcquireSync(1, ms)又用原ms起算，上层snapshot()的预算会被放大一倍，影响帧率/延迟预算。建议改为使用剩余预算（记录起始时刻，ms2 = max(0, timeout - elapsed)），或在事件已被信号时用极短超时（例如 0）尝试获取，失败再按剩余预算等待。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/display_vdd.cpp` around lines 200 - 220, The current flow uses the full timeout twice (WaitForSingleObject(m_hEvent, ms) then m_keyedMutex->AcquireSync(1, ms)), which can double the effective wait; change it to compute remaining time after the WaitForSingleObject call and pass that to AcquireSync: record a start timestamp before WaitForSingleObject, after it compute elapsed and derive ms2 = max(0, timeout - elapsed) and use ms2 for m_keyedMutex->AcquireSync(1, ms2); alternatively, if WaitForSingleObject returned signaled immediately, attempt AcquireSync with a short/zero timeout first and only fall back to the remaining budget; update any references in snapshot() call sites if they assume the original single-budget semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/platform/windows/display_vdd.cpp`:
- Around line 242-245: 将 display.h 中 current_frame 的注释从 "Borrowed ref from
dup.next_frame, released in release_snapshot" 改为表明这是一个已拥有的引用，例如 "Owned ref from
dup.next_frame (via AddRef), must Release in release_snapshot"；相关位置可通过
vdd_capture_t::next_frame()（调用 m_sharedTex->AddRef()）和
display_vdd_vram_t::release_snapshot()（调用 Release()）来确认语义，确保注释明确说明调用方必须在
release_snapshot 中配对 Release().
---
Nitpick comments:
In `@src/platform/windows/display_vdd.cpp`:
- Around line 200-220: The current flow uses the full timeout twice
(WaitForSingleObject(m_hEvent, ms) then m_keyedMutex->AcquireSync(1, ms)), which
can double the effective wait; change it to compute remaining time after the
WaitForSingleObject call and pass that to AcquireSync: record a start timestamp
before WaitForSingleObject, after it compute elapsed and derive ms2 = max(0,
timeout - elapsed) and use ms2 for m_keyedMutex->AcquireSync(1, ms2);
alternatively, if WaitForSingleObject returned signaled immediately, attempt
AcquireSync with a short/zero timeout first and only fall back to the remaining
budget; update any references in snapshot() call sites if they assume the
original single-budget semantics.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba3e7afa-5a70-441b-b2ae-daaf2afcafae
📒 Files selected for processing (1)
src/platform/windows/display_vdd.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/platform/windows/display_vdd.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_vdd.cpp
🔇 Additional comments (3)
src/platform/windows/display_vdd.cpp (3)
355-357: FP16 格式下capture_linear_gamma未同步更新（历史评审未修复）。Line 357 用
dup.format()覆写capture_format，但capture_linear_gamma仍停留在display_base_t::init()基于 DXGI 虚拟输出 ColorSpace 推断的结果。当 producer 推送DXGI_FORMAT_R16G16B16A16_FLOAT（scRGB）而 DXGI 又报默认 G22 colorspace 时，capture_linear_gamma=false会让complete_img()把img.linear_gamma设为 false，导致下游 RGB→YUV shader 与 HDR luminance analyzer 把线性 scRGB 当非线性处理。♻️ 建议修正
capture_format = dup.format(); + // FP16 from ZakoVDD is always linear scRGB; DXGI ColorSpace on the virtual + // output may be unreliable, so override based on the producer-reported format. + capture_linear_gamma = (capture_format == DXGI_FORMAT_R16G16B16A16_FLOAT);
410-416: HDR 亮度值未做有限性/范围校验即窄化转换（历史评审未修复）。
dup.max_nits() / min_nits() / max_fall()源自共享内存的float，对其直接static_cast<uint16_t>/uint32_t在出现 NaN、±Inf、负数或超UINT16_MAX/UINT32_MAX时属未定义行为，会向编码器写入脏 HDR 元数据。特别是min_nits * 10000.0f很容易越过uint32_t上限。🛡️ 建议修正
+ auto clamp_nits = [](float v, float upper) { + if (!std::isfinite(v) || v < 0.0f) return 0.0f; + return std::min(v, upper); + }; - metadata.maxDisplayLuminance = static_cast<uint16_t>(dup.max_nits()); - metadata.minDisplayLuminance = static_cast<uint32_t>(dup.min_nits() * 10000.0f); - metadata.maxFullFrameLuminance = static_cast<uint16_t>(dup.max_fall()); + metadata.maxDisplayLuminance = static_cast<uint16_t>(clamp_nits(dup.max_nits(), 65535.0f)); + metadata.minDisplayLuminance = static_cast<uint32_t>(clamp_nits(dup.min_nits(), 429496.7295f) * 10000.0f); + metadata.maxFullFrameLuminance = static_cast<uint16_t>(clamp_nits(dup.max_fall(), 65535.0f));记得补上
<algorithm>与<cmath>头文件。
362-375: 格式白名单校验 LGTM。init 前按
get_supported_capture_formats()显式拒绝不兼容格式，并留有清晰日志，能有效避免后续 RTV/shader 路径在 P010 等不支持格式上崩溃，符合历史评审建议。
Comment on lines
+242
to
+245
| if (out) { | ||
| m_sharedTex->AddRef(); | ||
| *out = m_sharedTex.get(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C5 '\b(display_vdd_vram_t::(snapshot|release_snapshot)|current_frame)\b' src/platform/windows/display_vram.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 2786
🏁 Script executed:
# Search for callers of snapshot() and release_snapshot()
rg -n 'snapshot|release_snapshot' src/platform/windows/ -t cpp | grep -E '\.(snapshot|release_snapshot)\(' | head -20Repository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
# Find the display_vdd_vram_t class definition to understand m_sharedTex
fd -t f display.h src/platform/windows/ -exec cat -n {} +Repository: AlkaidLab/foundation-sunshine
Length of output: 303
🏁 Script executed:
# Look for the context around display_vdd.cpp lines 242-245
sed -n '235,255p' src/platform/windows/display_vdd.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 583
🏁 Script executed:
# Get more context to see which method contains lines 242-245
sed -n '220,260p' src/platform/windows/display_vdd.cppRepository: AlkaidLab/foundation-sunshine
Length of output: 1396
🏁 Script executed:
# Search for m_sharedTex->Release() calls
rg 'm_sharedTex->Release|m_sharedTex.*Release' src/platform/windows/ -nRepository: AlkaidLab/foundation-sunshine
Length of output: 55
🏁 Script executed:
# Find and show display.h content
find src/platform/windows -name 'display.h' -type f -exec cat -n {} +Repository: AlkaidLab/foundation-sunshine
Length of output: 27433
将 current_frame 的注释从"Borrowed ref"改为"Owned ref"，以准确反映其引用计数语义。
代码的 AddRef/Release 配对是正确的：vdd_capture_t::next_frame() 在第 242-245 行对 m_sharedTex 执行 AddRef()，而 display_vdd_vram_t::release_snapshot() 在第 3500-3501 行释放该引用。然而，display.h 第 626 行的注释"Borrowed ref from dup.next_frame, released in release_snapshot"具有误导性。
"Borrowed ref"通常意味着借用指针（如 std::string_view），调用方无需释放。但此处实际上是已增加引用计数的拥有式指针，调用方必须配对调用 Release()。此注释不匹配实际语义，可能导致维护人员在修改 snapshot/release_snapshot 时忽视 AddRef/Release 契约，引入引用泄漏。
建议将注释改为"Owned ref from dup.next_frame（via AddRef），must Release in release_snapshot"，以明确调用方的责任。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/platform/windows/display_vdd.cpp` around lines 242 - 245, 将 display.h 中
current_frame 的注释从 "Borrowed ref from dup.next_frame, released in
release_snapshot" 改为表明这是一个已拥有的引用，例如 "Owned ref from dup.next_frame (via AddRef),
must Release in release_snapshot"；相关位置可通过 vdd_capture_t::next_frame()（调用
m_sharedTex->AddRef()）和 display_vdd_vram_t::release_snapshot()（调用
Release()）来确认语义，确保注释明确说明调用方必须在 release_snapshot 中配对 Release().
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/video.cpp (2)
78-90: 可选优化：用std::optional<std::string>取代bool active + std::string previous_capture当前实现用两个成员维护覆盖状态，语义由两者组合表达。用
std::optional可把“是否激活”与“保存的旧值”合并为单一状态，可读性更好，也避免忘记同步更新两个字段的潜在风险。析构时直接if (previous) config::video.capture = std::move(*previous);。♻️ 参考改动
private: - bool active = false; - std::string previous_capture; + std::optional<std::string> previous_capture;以及构造/析构相应改为：
if (capture_override && config::video.capture != *capture_override) { previous_capture = config::video.capture; config::video.capture = *capture_override; } // ~dtor if (previous_capture) { config::video.capture = std::move(*previous_capture); }顺带提示：析构函数里
config::video.capture = previous_capture;是std::string赋值，理论上可能因分配失败抛出，而析构函数不应抛异常。使用std::move可规避绝大多数分配路径；若要更严格，可在析构中加try/catch(...)。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/video.cpp` around lines 78 - 90, Replace the pair of members "bool active" and "std::string previous_capture" in scoped_capture_override_t with a single std::optional<std::string> previous_capture; update constructor/override logic in scoped_capture_override_t so you set previous_capture = config::video.capture when applying an override (only if capture_override is present and differs) and assign config::video.capture = *capture_override; in the destructor, restore with if (previous_capture) config::video.capture = std::move(*previous_capture); and consider wrapping that assignment in a try/catch(...) to ensure the destructor remains noexcept; keep the deleted copy ctor/operator= as-is.
92-100: 将硬编码字符串提升为具名常量，并添加解释注释虽然
"ddx"确实是合法的后端类型，但该映射缺乏必要的文档说明。建议：
- 定义具名常量（如
constexpr std::string_view kDdxCaptureBackend = "ddx"sv;），与"vdd"一起集中定义，降低拼写漂移风险- 在函数注释中说明上下文：VDD 虚拟显示驱动的初始化通常由后续的 prep-cmd 脚本完成，而编码器探测在此之前执行。因此探测阶段 VDD 可能尚未就绪，导致 "Failed to locate output" 错误。临时切换到 DDX 确保探测不被未初始化的 VDD 阻断，探测完成后配置的真实捕获后端仍由
config::video.capture控制。这样后续维护者能理解该 override 的必要性和作用边界。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/video.cpp` around lines 92 - 100, Replace the hard-coded "ddx" in capture_override_for_encoder_probe() with a named constexpr like kDdxCaptureBackend (and likewise define kVddCaptureBackend for "vdd") and update the function to return that constant; add a brief comment above capture_override_for_encoder_probe() explaining that VDD (config::video.capture == "vdd") may not be initialized during encoder probe because VDD initialization is performed later by prep-cmd, so we temporarily override to the DDX backend to avoid "Failed to locate output" during probe while leaving config::video.capture unchanged for runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/video.cpp`:
- Around line 68-100: The code races on global config::video.capture because
scoped_capture_override_t's ctor/dtor write it without synchronization; fix by
either (A) adding synchronization (e.g., a lightweight shared_mutex in
config::video and take exclusive lock inside scoped_capture_override_t and
shared locks for readers in display_base.cpp, capture_async and any readers) or
(B) avoid mutating global state: change
capture_override_for_encoder_probe()/validate_encoder()/probe_encoders() to pass
an override parameter down the call chain (into reset_display / platf::display /
probe paths) so no global write is needed; update call sites in
probe_encoders(), validate_encoder and any display/capture readers accordingly
and document the chosen threading guarantee in comments if you keep the global
mutation.
---
Nitpick comments:
In `@src/video.cpp`:
- Around line 78-90: Replace the pair of members "bool active" and "std::string
previous_capture" in scoped_capture_override_t with a single
std::optional<std::string> previous_capture; update constructor/override logic
in scoped_capture_override_t so you set previous_capture = config::video.capture
when applying an override (only if capture_override is present and differs) and
assign config::video.capture = *capture_override; in the destructor, restore
with if (previous_capture) config::video.capture = std::move(*previous_capture);
and consider wrapping that assignment in a try/catch(...) to ensure the
destructor remains noexcept; keep the deleted copy ctor/operator= as-is.
- Around line 92-100: Replace the hard-coded "ddx" in
capture_override_for_encoder_probe() with a named constexpr like
kDdxCaptureBackend (and likewise define kVddCaptureBackend for "vdd") and update
the function to return that constant; add a brief comment above
capture_override_for_encoder_probe() explaining that VDD (config::video.capture
== "vdd") may not be initialized during encoder probe because VDD initialization
is performed later by prep-cmd, so we temporarily override to the DDX backend to
avoid "Failed to locate output" during probe while leaving config::video.capture
unchanged for runtime.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5ac0d8b-4ff0-4249-8eb9-b2b652e1551f
📒 Files selected for processing (1)
src/video.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/video.cpp
🪛 Cppcheck (2.20.0)
src/video.cpp
[error] 83-83: Cannot find end of expression
(internalError)
🔇 Additional comments (1)
src/video.cpp (1)
3616-3629: LGTM：日志与作用域清晰
- 先取
configured_capture_backend快照、再计算 override、由 RAII 守卫接管恢复，fail_guard和scoped_capture_override_t的析构顺序是逆序的（fg先析构记录失败日志，再capture_override_guard恢复配置），符合预期。- 仅当真正产生覆盖时才打印切换日志，避免日志噪音。
一个小建议：在
validate_encoder的BOOST_LOG(info) << "Trying encoder ..."之后再打印 override 信息，顺序上更自然（先说在测哪个编码器，再说为此编码器探测临时切到了哪个 capture 后端）——当前顺序对阅读日志者影响不大，可自行决定。
Comment on lines
+68
to
+100
| class scoped_capture_override_t { | ||
| public: | ||
| explicit scoped_capture_override_t(const std::optional<std::string> &capture_override) { | ||
| if (capture_override && config::video.capture != *capture_override) { | ||
| previous_capture = config::video.capture; | ||
| config::video.capture = *capture_override; | ||
| active = true; | ||
| } | ||
| } | ||
|
|
||
| ~scoped_capture_override_t() { | ||
| if (active) { | ||
| config::video.capture = previous_capture; | ||
| } | ||
| } | ||
|
|
||
| scoped_capture_override_t(const scoped_capture_override_t &) = delete; | ||
| scoped_capture_override_t &operator=(const scoped_capture_override_t &) = delete; | ||
|
|
||
| private: | ||
| bool active = false; | ||
| std::string previous_capture; | ||
| }; | ||
|
|
||
| std::optional<std::string> | ||
| capture_override_for_encoder_probe() { | ||
| #ifdef _WIN32 | ||
| if (config::video.capture == "vdd") { | ||
| return std::string { "ddx" }; | ||
| } | ||
| #endif | ||
| return std::nullopt; | ||
| } |
There was a problem hiding this comment.
线程安全：未同步地修改全局 config::video.capture
config::video.capture 是 extern video_t video 中的一个 std::string 成员（见 src/config.h Line 91 与 Line 240），没有任何互斥/原子保护。scoped_capture_override_t 在构造/析构时对它进行写入，与其它线程对同一字段的读取构成数据竞争（对 std::string 而言即未定义行为）。
虽然 validate_encoder 一般在 probe_encoders() 内调用，看起来在启动期或重新枚举期执行，但：
- 重新探测（
needs_encoder_reenumeration()为 true 时）可能在 Sunshine 已经运行、其它线程可能仍会读取config::video.capture的场景下触发； display_base.cpp的工厂选择、capture_async线程中的相关字段读取都直接访问config::video.*，并不拥有video_t的所有权快照。
建议任选其一落地：
- 在
config::video上引入一个轻量 shared_mutex（或至少在此字段上），让探测期间的覆盖与其它读者互斥； - 或者改为不修改全局配置，而是将 override 作为参数透传到
reset_display/platf::display(...)等调用链上，仅影响探测路径，避免全局可见副作用； - 若确认探测期间不可能有并发读者，请在类的注释中明确这一前置条件，便于后续维护者不会在并发语境复用。
如保留当前实现，建议至少在头注释中写明：“仅在 probe_encoders() 单线程阶段调用，调用期间全局配置读者必须停止”。
作为 CodeRabbit 编码指南要求：内存安全、线程安全、RAII 资源管理、安全漏洞。
🧰 Tools
🪛 Cppcheck (2.20.0)
[error] 83-83: Cannot find end of expression
(internalError)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/video.cpp` around lines 68 - 100, The code races on global
config::video.capture because scoped_capture_override_t's ctor/dtor write it
without synchronization; fix by either (A) adding synchronization (e.g., a
lightweight shared_mutex in config::video and take exclusive lock inside
scoped_capture_override_t and shared locks for readers in display_base.cpp,
capture_async and any readers) or (B) avoid mutating global state: change
capture_override_for_encoder_probe()/validate_encoder()/probe_encoders() to pass
an override parameter down the call chain (into reset_display / platf::display /
probe paths) so no global write is needed; update call sites in
probe_encoders(), validate_encoder and any display/capture readers accordingly
and document the chosen threading guarantee in comments if you keep the global
mutation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
新增
vddcapture 后端：直接消费 ZakoVDD 虚拟显示驱动通过共享 D3D11 NT 句柄发布的桌面纹理，绕过 DXGI Desktop Duplication / WGC。工作原理
VDD 驱动 producer 端发布：
Global\ZakoVDD_Frame_<idx>：D3D11 NT-shared 纹理（KeyedMutex 0=producer / 1=consumer）Global\ZakoVDD_FrameReady_<idx>：每帧 SetEvent 命名事件Global\ZakoVDD_Meta_<idx>：MMF，承载SharedFrameMetadata（Magic'ZVDF'、Version 1、Width/Height、DxgiFormat、HDR meta、FrameCounter、LastPresentQpc）Sunshine 侧
display_vdd_vram_t在已有display_base_t的同一 D3D11 device 上OpenSharedResourceByName该纹理，等事件 + 拿 KeyedMutex(1) 后CopyResource进现有img_d3d_tpool，零跨设备拷贝。Highlights
\\.\pipe\ZakoVDDPipe发CREATEMONITOR并轮询 ~3s 重试文件变更
src/platform/windows/display_vdd.cpp(新建)vdd_capture_t（KeyedMutex + 命名事件 consumer）、monitor-index resolver（按尺寸匹配Global\ZakoVDD_Meta_*）、display_vdd_vram_t::initsrc/platform/windows/display_vram.cppdisplay_vdd_vram_t::snapshot/release_snapshot（必须放这里以访问文件局部img_d3d_t与texture_lock_helper）src/platform/windows/display.hvdd_capture_t+display_vdd_vram_t声明src/platform/windows/display_base.cppelse if (type == "vdd" && hwdevice_type == dxgi)分支src_assets/common/assets/web/configs/tabs/Advanced.vuevdd选项cmake/compile_definitions/windows.cmakedisplay_vdd.cpp兼容性 / 不变量
vdd（与 wgc/ddx/amd 同等地位）SharedFrameMetadata二进制布局必须与 VDD producer 严格一致（ZakoVDD 仓库Driver.cpp）测试
sunshine.exelink 通过（mingw ucrt64 / g++ 15.2 / C++23）用法
Web UI → Configuration → Advanced → Capture method 选择 "ZakoVDD Direct Shared Texture (beta)"。
若当前没有 VDD monitor，Sunshine 会自动通过 NamedPipe 触发创建。