fix: add explicit target mode fallback for non-standard resolutions (#594)#595
fix: add explicit target mode fallback for non-standard resolutions (#594)#595
Conversation
…594) When Windows CCD mode matching fails for non-standard resolutions (e.g. 16:10 like 2560x1600) on newly created virtual display paths, the SetDisplayConfig API returns ERROR_GEN_FAILURE because it cannot find a valid source-to-target mode mapping in its topology database. This adds a third fallback attempt in set_display_modes() that explicitly sets target mode parameters (activeSize, totalSize, vSyncFreq, hSyncFreq, pixelRate) in-place instead of clearing the target mode index and relying on Windows' automatic matching. The fallback only triggers when both existing attempts (with and without SDC_ALLOW_CHANGES) fail, so it has zero impact on environments where mode setting already works correctly. Closes #594
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent 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)
🧰 Additional context used📓 Path-based instructions (2)src/**/*.{cpp,c,h}⚙️ CodeRabbit configuration file
Files:
src/platform/**⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
Summary by CodeRabbit
Walkthrough在 set_display_modes 中新增更严格的重试与显式目标回退路径：首次设置后进行验证；若失败则切换 allow_changes 再试；仍不匹配时重查询当前拓扑、就地更新目标 mode 的 targetVideoSignalInfo、清除桌面图像索引并以 SDC_USE_SUPPLIED_DISPLAY_CONFIG 调用 SetDisplayConfig 并再次验证，否则回滚失败。 Changes
Sequence Diagram(s)sequenceDiagram
participant Manager as ModeManager
participant OS as WindowsDisplaySubsystem
participant Topo as ActiveTopologyStore
Manager->>OS: do_set_modes(modes, allow_changes)
alt first attempt OK & verified
OS-->>Manager: ERROR_SUCCESS (verified)
else
Manager->>OS: do_set_modes(modes, !allow_changes)
alt second attempt OK & verified
OS-->>Manager: ERROR_SUCCESS (verified)
else
Manager->>OS: QueryDisplayConfig()
OS-->>Topo: return current paths & mode infos
Topo-->>Manager: current topology
Manager->>Topo: modify source resolution & targetMode.targetVideoSignalInfo
Manager->>Topo: clear desktop image index
Manager->>OS: SetDisplayConfig(modified topology, SDC_USE_SUPPLIED_DISPLAY_CONFIG...)
alt ERROR_SUCCESS & verified
OS-->>Manager: success (verified)
else
OS-->>Manager: failure -> rollback/fail
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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.
🧹 Nitpick comments (1)
src/platform/windows/display_device/device_modes.cpp (1)
252-275: 建议在 target_idx 无效时添加调试日志当
target_idx无效或超出范围时，代码会静默跳过目标模式更新。考虑到这是一个用于调试非标准分辨率问题的回退路径，添加日志可以帮助诊断回退失败的原因。💡 建议添加调试日志
const UINT32 target_idx { path->targetInfo.targetModeInfoIdx }; if (target_idx != DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID && target_idx < display_data->modes.size()) { auto &target_mode_info = display_data->modes[target_idx]; if (target_mode_info.infoType == DISPLAYCONFIG_MODE_INFO_TYPE_TARGET) { // ... existing code ... + } else { + BOOST_LOG(debug) << "Target mode info type mismatch for device: " << device_id; } + } else { + BOOST_LOG(debug) << "Invalid target mode index for device: " << device_id << ", idx=" << target_idx; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/display_device/device_modes.cpp` around lines 252 - 275, 当 target_idx 等于 DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID 或者 target_idx >= display_data->modes.size() 时当前静默跳过目标模式更新；在这条分支处（在对 target_idx 的 if 判断外或其 else 分支里）添加一条调试日志，记录 target_idx 值、display_data->modes.size() 以及相关 mode（如 mode.resolution.width/height 和 mode.refresh_rate.numerator/denominator）的信息，以便追踪为何未应用目标模式（引用符号：target_idx、DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID、display_data->modes、mode、path->targetInfo.targetModeInfoIdx、target_mode_info）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/platform/windows/display_device/device_modes.cpp`:
- Around line 252-275: 当 target_idx 等于
DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID 或者 target_idx >=
display_data->modes.size() 时当前静默跳过目标模式更新；在这条分支处（在对 target_idx 的 if 判断外或其 else
分支里）添加一条调试日志，记录 target_idx 值、display_data->modes.size() 以及相关 mode（如
mode.resolution.width/height 和
mode.refresh_rate.numerator/denominator）的信息，以便追踪为何未应用目标模式（引用符号：target_idx、DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID、display_data->modes、mode、path->targetInfo.targetModeInfoIdx、target_mode_info）。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f28aef29-aa7b-4215-a010-562a132e4865
📒 Files selected for processing (1)
src/platform/windows/display_device/device_modes.cpp
📜 Review details
🧰 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_device/device_modes.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/device_modes.cpp
🔇 Additional comments (4)
src/platform/windows/display_device/device_modes.cpp (4)
277-290: 桌面图像信息更新逻辑正确正确地只更新了
right和bottom字段（left和top保持为 0），这符合 Windows RECT 结构的原点坐标惯例。
443-453: 控制流变更正确，遵循现有模式新增的回退逻辑正确地遵循了函数中已建立的模式：
- 仅在推荐模式和严格模式都失败后才尝试
- 在尝试前记录信息日志
- 成功后验证模式是否匹配
- 失败时继续执行原有的恢复逻辑
这种渐进式回退策略确保了现有工作场景不受影响。
299-301: 日志级别使用合理这里使用
warning而非error级别是合理的，因为这是回退路径，失败后还会尝试恢复原始模式。这与主路径do_set_modes中使用error级别形成了适当的区分。
262-271: 这些信号参数计算是 Windows 虚拟显示驱动 (VDD) 的标准做法，与 Microsoft 官方 IddCx 示例代码一致，无需进一步验证。设置activeSize与totalSize相同（无消隐间隔），以及使用hSyncFreq = vSync * Height和pixelRate = vSync * Width * Height / Denominator的公式，都是 VDD 实现的规范方式。
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_device/device_modes.cpp`:
- Around line 429-438: The code currently returns false immediately when
do_set_modes(modes, allow_changes) fails (e.g., ERROR_GEN_FAILURE), so the later
fallback do_set_modes_with_explicit_target(modes) only runs for the
"applied-but-mismatch" case; change set_display_modes() so that both failure
cases (initial do_set_modes returned false) and the "applied but
all_modes_match() is false" path flow into the same fallback chain: attempt
do_set_modes_with_explicit_target(modes), then re-query via
get_current_display_modes(device_ids) and check all_modes_match(current_modes)
before returning; ensure you do not early-return on the first failure, propagate
and log Windows error codes consistently, and keep resource cleanup/error
handling identical to the existing mismatch path (use same logging and return
semantics only after fallback finishes).
🪄 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: 02d4a3b8-cb7e-49ae-a4a5-1e4cbe223299
📒 Files selected for processing (1)
src/platform/windows/display_device/device_modes.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_device/device_modes.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/device_modes.cpp
🔇 Additional comments (1)
src/platform/windows/display_device/device_modes.cpp (1)
233-275: 这里的targetModeInfoIdx读取在本代码库中是正确的，无需额外的DISPLAYCONFIG_PATH_SUPPORT_VIRTUAL_MODE门控。根据
windows_utils.cpp的代码文档（第 494-499、516-521 行等），微软文档虽然声称该字段仅在设置DISPLAYCONFIG_PATH_SUPPORT_VIRTUAL_MODE标志时有效，但这个结论经验证是"BS"（文档的复制粘贴错误）。实际的联合体可访问性取决于查询时是否指定了QDC_VIRTUAL_MODE_AWARE标志，本函数已满足此要求（query_display_config()使用该标志）。代码已通过以下方式防御不可用的索引：
target_idx != DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID检查target_idx < display_data->modes.size()边界检查infoType == DISPLAYCONFIG_MODE_INFO_TYPE_TARGET类型验证关于桌面图像同步的顾虑在本代码库中不适用，因为代码中无相关的
DISPLAYCONFIG_DESKTOP_IMAGE_INFO处理。
The explicit target mode fallback was only reachable when the first do_set_modes() call succeeded but mode validation failed. When the first call itself fails with ERROR_GEN_FAILURE (the primary scenario in #594), the old early return prevented any fallback from running. Remove the early return, store the first attempt result, and flatten all three attempts into a single unconditional fallback chain so that do_set_modes_with_explicit_target() is tried regardless of whether the previous calls failed or returned mismatched modes.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/windows/display_device/device_modes.cpp (1)
430-437:⚠️ Potential issue | 🟠 Major调整 rollback 前后的目标模式信息捕获范围，避免显式 target timing 的持久化数据不完整恢复。
do_set_modes_with_explicit_target()详细修改targetVideoSignalInfo内多个字段（activeSize、totalSize、vSyncFreq、hSyncFreq、pixelRate、scanLineOrdering），并以SDC_SAVE_TO_DATABASE持久化到 Windows 注册表；但 Line 377 的回滚快照original_modes只是device_display_mode_t结构体，仅包含分辨率和刷新率两个参数。若
do_set_modes_with_explicit_target()在 Line 430 返回成功但 Line 432 验证失败，Line 437 的回滚do_set_modes(original_modes, allow_changes)会清空目标模式索引，由 Windows 基于分辨率和刷新率自动生成新的targetVideoSignalInfo并持久化，导致数据库内留存与原始状态不一致的配置。建议在进入显式 target 回退路径前保存完整的
DISPLAYCONFIG_PATH_INFO[]和DISPLAYCONFIG_MODE_INFO[]快照，验证失败时按完整快照回滚，避免部分持久化数据的丢失。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/display_device/device_modes.cpp` around lines 430 - 437, The rollback snapshot is incomplete: do_set_modes_with_explicit_target() mutates multiple fields inside targetVideoSignalInfo and persists them (SDC_SAVE_TO_DATABASE) but original_modes only holds resolution/refresh; capture full DISPLAYCONFIG_PATH_INFO[] and DISPLAYCONFIG_MODE_INFO[] snapshots before calling do_set_modes_with_explicit_target(modes) (use the same structures the API writes to), then if get_current_display_modes(device_ids) indicates mismatch after the explicit-target attempt, call a new restore routine that reapplies the saved DISPLAYCONFIG_PATH_INFO/ DISPLAYCONFIG_MODE_INFO arrays (rather than do_set_modes(original_modes,...)) to fully restore the persisted targetVideoSignalInfo state and avoid partial/default regeneration by Windows.
🤖 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_device/device_modes.cpp`:
- Around line 233-283: You updated source mode, path->targetInfo.refreshRate and
targetVideoSignalInfo but did not clear the desktop-image index, so when using
SDC_VIRTUAL_MODE_AWARE a stale DISPLAYCONFIG_MODE_INFO_TYPE_DESKTOP_IMAGE entry
(with old size) can make SetDisplayConfig fail with ERROR_GEN_FAILURE; fix by
calling w_utils::set_desktop_index(*path, boost::none) after applying the
in-place target-mode updates (same place do_set_modes() used) so Windows will
reselect the desktop image entry before calling SetDisplayConfig with the
SDC_VIRTUAL_MODE_AWARE flags.
---
Outside diff comments:
In `@src/platform/windows/display_device/device_modes.cpp`:
- Around line 430-437: The rollback snapshot is incomplete:
do_set_modes_with_explicit_target() mutates multiple fields inside
targetVideoSignalInfo and persists them (SDC_SAVE_TO_DATABASE) but
original_modes only holds resolution/refresh; capture full
DISPLAYCONFIG_PATH_INFO[] and DISPLAYCONFIG_MODE_INFO[] snapshots before calling
do_set_modes_with_explicit_target(modes) (use the same structures the API writes
to), then if get_current_display_modes(device_ids) indicates mismatch after the
explicit-target attempt, call a new restore routine that reapplies the saved
DISPLAYCONFIG_PATH_INFO/ DISPLAYCONFIG_MODE_INFO arrays (rather than
do_set_modes(original_modes,...)) to fully restore the persisted
targetVideoSignalInfo state and avoid partial/default regeneration by Windows.
🪄 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: 6916f8eb-1ab2-48b2-bc58-31b4e5713fe3
📒 Files selected for processing (1)
src/platform/windows/display_device/device_modes.cpp
📜 Review details
🧰 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_device/device_modes.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/device_modes.cpp
A stale DISPLAYCONFIG_MODE_INFO_TYPE_DESKTOP_IMAGE entry (with the old resolution dimensions) in the modes array can cause SetDisplayConfig to return ERROR_GEN_FAILURE when SDC_VIRTUAL_MODE_AWARE is used, even when the target mode entry itself is correctly updated in-place. Call w_utils::set_desktop_index(*path, boost::none) after applying the in-place target signal updates so Windows reselects an appropriate desktop image entry, matching the pattern already used in do_set_modes().
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_device/device_modes.cpp`:
- Around line 233-281: The code sets changes_applied when updating source_mode
or path refresh rate but only calls w_utils::set_desktop_index(*path,
boost::none) when path->targetInfo.targetModeInfoIdx is valid and a
DISPLAYCONFIG_MODE_INFO_TYPE_TARGET, leaving stale desktop-image entries when
targetModeInfoIdx is invalid; fix by either (A) if
path->targetInfo.targetModeInfoIdx == DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID
or out-of-range or not a TARGET, return false immediately (so SetDisplayConfig()
isn't called with stale desktop entries), or (B) always clear the desktop image
index whenever changes_applied is set (call w_utils::set_desktop_index(*path,
boost::none) whenever resolution_changed || refresh_rate_changed) before
proceeding to SetDisplayConfig(); reference path->targetInfo.targetModeInfoIdx,
DISPLAYCONFIG_PATH_TARGET_MODE_IDX_INVALID, DISPLAYCONFIG_MODE_INFO_TYPE_TARGET,
changes_applied, w_utils::set_desktop_index, and SetDisplayConfig in your
change.
🪄 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: 5b563b86-d11e-4cff-88ca-a96ecdd30e97
📒 Files selected for processing (1)
src/platform/windows/display_device/device_modes.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_device/device_modes.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/device_modes.cpp
🔇 Additional comments (1)
src/platform/windows/display_device/device_modes.cpp (1)
390-439: 这段回退链修正方向是对的。现在“首轮
SetDisplayConfig()直接失败”和“调用成功但校验不匹配”都会继续进入严格重试，再进入 explicit-target fallback，已经覆盖了#594的主故障路径。
Previously the function could proceed to SetDisplayConfig() with a stale desktop image entry (from old resolution) when targetModeInfoIdx was INVALID or the mode entry was not of TARGET type. This would cause ERROR_GEN_FAILURE under SDC_VIRTUAL_MODE_AWARE. Change the nested if-guards into early returns so the function bails out immediately if no valid TARGET-type entry exists, ensuring SetDisplayConfig is only called when the full explicit target configuration is valid.
1 participant
Problem
When streaming to devices with non-standard resolutions like 2560x1600 (16:10 aspect ratio), the
SetDisplayConfigAPI returnsERROR_GEN_FAILUREbecause Windows CCD (Connecting and Configuring Displays) cannot find a valid source-to-target mode mapping in its topology database for newly created virtual display paths.The failure occurs in
set_display_modes()indevice_modes.cpp:SDC_ALLOW_CHANGES): Windows adjusts the mode to something else (e.g., 2560x1440), fuzzy compare detects mismatchSetDisplayConfigreturnsERROR_GEN_FAILUREbecause the CCD database has no mapping for the non-standard resolutionStandard resolutions like 2560x1440 work because Windows has built-in mode matching support for common 16:9 modes.
Root Cause
The current code clears the
targetModeInfoIdxanddesktopModeInfoIdxin the path, telling Windows to automatically match source mode to target mode. For newly created VDD paths with non-standard resolutions (especially when the CCD topology was itself just created via fallback), Windows' automatic mode matching algorithm fails.Solution
Add a third fallback attempt in
set_display_modes()via a newdo_set_modes_with_explicit_target()function that:activeSize,totalSize,vSyncFreq,hSyncFreq,pixelRate)This bypasses Windows CCD's automatic mode matching entirely by providing a complete, self-consistent display configuration.
Impact
device_modes.cppis modifiedCloses #594