fix(vdd): avoid persisting VDD-only restore baseline#754
Merged
Conversation
Summary by CodeRabbit
WalkthroughPR 重写了 VDD 请求判定、 ChangesVDD 选择、准备与持久化
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/display_device/session.cpp`:
- Line 557: `prepare_vdd` is currently swallowing failures from VDD recovery,
missing `device_zako`, and `apply_vdd_prep`, which lets callers continue with an
invalid `use_vdd=true` config. Restore explicit success/failure propagation in
`prepare_vdd` by returning a boolean/result instead of silently returning, and
have `make_parsed_config` check that return value and yield `boost::none` when
VDD preparation fails. Also update the call sites in the parsing flow to stop
continuing after a failed VDD prep so `device_zako` and `apply_vdd_prep` errors
are surfaced correctly.
In `@src/display_device/vdd_utils.cpp`:
- Around line 1076-1094: The fallback in vdd_utils.cpp is currently collecting
every non-VDD device into physical_devices, which can reintroduce displays that
were inactive before VDD creation. Update the logic around pre_vdd_devices and
enum_available_devices so that only physical displays that were active/primary
at the time of VDD creation are retained, or derive the list from the current
active topology instead. Make sure original_primary_id and the topology-building
path in vdd_as_primary/vdd_as_secondary continue to use only the filtered active
devices.
🪄 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: 036f7954-f25d-4893-8e9f-eef125b48287
📒 Files selected for processing (11)
src/display_device/parsed_config.cppsrc/display_device/parsed_config.hsrc/display_device/session.cppsrc/display_device/session.hsrc/display_device/vdd_utils.cppsrc/display_device/vdd_utils.hsrc/nvhttp_stream_start.cppsrc/platform/windows/display_device/settings.cppsrc/platform/windows/display_device/settings_topology.cppsrc/platform/windows/display_device/settings_topology.htests/unit/test_vdd_safety.cpp
💤 Files with no reviewable changes (1)
- src/display_device/parsed_config.h
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/platform/windows/display_device/settings_topology.hsrc/display_device/session.hsrc/platform/windows/display_device/settings_topology.cppsrc/display_device/parsed_config.cppsrc/platform/windows/display_device/settings.cppsrc/display_device/vdd_utils.hsrc/nvhttp_stream_start.cppsrc/display_device/vdd_utils.cppsrc/display_device/session.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/settings_topology.hsrc/platform/windows/display_device/settings_topology.cppsrc/platform/windows/display_device/settings.cpp
tests/**
⚙️ CodeRabbit configuration file
tests/**: 测试文件。验证测试覆盖率、边界情况和断言正确性。
Files:
tests/unit/test_vdd_safety.cpp
🔇 Additional comments (6)
src/display_device/parsed_config.cpp (1)
524-524: LGTM!Also applies to: 698-710
src/nvhttp_stream_start.cpp (1)
245-245: LGTM!src/platform/windows/display_device/settings_topology.h (1)
69-77: LGTM!tests/unit/test_vdd_safety.cpp (1)
5-15: LGTM!src/platform/windows/display_device/settings_topology.cpp (1)
318-326: 🩺 Stability & Availability该实现符合项目 C++23 标准，无需修改。
经核实项目构建配置，代码库目标标准为 C++23，明确允许使用
std::unordered_set::contains。该方法语义清晰且优于传统的count() > 0写法，无需为了风格一致性而改回旧式代码。src/platform/windows/display_device/settings.cpp (1)
952-964: 🎯 Functional Correctness确认
config.device_id为 VDD 设备标识，且空值处理逻辑正确。经核实，
config.device_id已正确传递给is_vdd_only_topology函数用于比对。该函数（settings_topology.cpp）内部已包含针对空 ID 的防御性检查，当 ID 为空时直接返回false，避免了无效跳过。因此，当config.device_id为空时不执行跳过逻辑属于预期行为，确保无法识别 VDD 时的配置安全。
| } | ||
|
|
||
| bool | ||
| void |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
恢复 VDD 准备失败的显式传播。
prepare_vdd 现在在 VDD 恢复失败或 device_zako 为空时直接返回，apply_vdd_prep 失败也被吞掉；调用方随后仍会继续使用 use_vdd=true 的配置。这会让后续显示配置在 VDD 未就绪或拓扑未应用时继续执行。请恢复布尔/结果返回，并让解析阶段在失败时返回 boost::none。
建议修复方向
- void
+ bool
session_t::prepare_vdd(parsed_config_t &config, const rtsp_stream::launch_session_t &session) {
@@
- return;
+ return false;
@@
if (device_zako.empty()) {
- return;
+ return false;
@@
- if (vdd_utils::apply_vdd_prep(device_zako, config.vdd_prep, pre_vdd_devices)) {
- BOOST_LOG(info) << "已应用VDD屏幕布局设置";
- std::this_thread::sleep_for(200ms);
+ if (!vdd_utils::apply_vdd_prep(device_zako, config.vdd_prep, pre_vdd_devices)) {
+ BOOST_LOG(error) << "应用VDD屏幕布局设置失败";
+ return false;
}
+ BOOST_LOG(info) << "已应用VDD屏幕布局设置";
+ std::this_thread::sleep_for(200ms);
@@
+ return true;
}并同步让 make_parsed_config 检查返回值。
Also applies to: 666-671, 698-701, 714-717
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/display_device/session.cpp` at line 557, `prepare_vdd` is currently
swallowing failures from VDD recovery, missing `device_zako`, and
`apply_vdd_prep`, which lets callers continue with an invalid `use_vdd=true`
config. Restore explicit success/failure propagation in `prepare_vdd` by
returning a boolean/result instead of silently returning, and have
`make_parsed_config` check that return value and yield `boost::none` when VDD
preparation fails. Also update the call sites in the parsing flow to stop
continuing after a failed VDD prep so `device_zako` and `apply_vdd_prep` errors
are surfaced correctly.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/display_device/vdd_utils.cpp (1)
1156-1157: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win不要把未知的
vdd_prep当作成功处理。
no_operation已在 Line 1066 提前返回；这里的default返回true会让下游误记录“已应用VDD屏幕布局设置”，但实际上没有应用任何拓扑。建议改为记录错误并返回false，或移除default让新增枚举值触发编译期检查。建议调整
- default: - return true; + default: + BOOST_LOG(error) << "未知的vdd_prep设置"; + return false;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/display_device/vdd_utils.cpp` around lines 1156 - 1157, The default branch in the vdd_prep handling currently returns true for unknown enum values, which can make callers believe a VDD layout was applied when no topology change happened. Update the switch in the vdd_prep flow in vdd_utils.cpp to reject unknown values by logging an error and returning false, or remove the default case so new enum values are caught at compile time; keep the existing no_operation early return behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/display_device/vdd_utils.cpp`:
- Around line 1156-1157: The default branch in the vdd_prep handling currently
returns true for unknown enum values, which can make callers believe a VDD
layout was applied when no topology change happened. Update the switch in the
vdd_prep flow in vdd_utils.cpp to reject unknown values by logging an error and
returning false, or remove the default case so new enum values are caught at
compile time; keep the existing no_operation early return behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 782153c6-5927-4aaa-ba87-fdc7e18566bf
📒 Files selected for processing (1)
src/display_device/vdd_utils.cpp
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/display_device/vdd_utils.cpp
🔇 Additional comments (3)
src/display_device/vdd_utils.cpp (3)
1075-1106: LGTM!
1116-1118: LGTM!
1160-1171: LGTM!
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.
改了啥呀
为啥要改
The previous guard protected restore baselines, but it also made resume fail after disconnect when physical displays were already inactive and VDD was primary. That turns a recoverable VDD-only state into a client-visible reconnect failure, 杂鱼 bug 状态自己把门锁上了。
验证
git diff --check -- src/display_device src/platform/windows/display_device src/nvhttp_stream_start.cpp tests/unit/test_vdd_safety.cppcmake --build build --target test_sunshine --config Debug, but the existing Ninja/MSYS2 build stops before reaching this change while compiling Boost dependency objects such asboost_container/src/alloc_lib.c, with no compiler diagnostic emitted.