fix(vdd): 修复切换设备串流时 VDD 黑屏及优化 VDD 生命周期管理#578
Merged
Conversation
|
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:
Walkthrough在会话配置中，非重配置且检测到不同 Changes
估计代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ShadowLemoon
force-pushed
the
fix/client-switch
branch
from
45c0c64 to
d7328ae
Compare
![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 the current code and only fix it if needed.
Inline comments:
In `@src/display_device/session.cpp`:
- Around line 290-296: 当前实现只保留 current_vdd_client_id 而没有重置其它会话状态，导致
current_device_prep、current_vdd_prep、current_use_vdd 和 original_output_name
等旧值沿用并在 make_parsed_config() 失败后让 restore_state_impl() 错误恢复旧会话状态；请在保留
current_vdd_client_id 的同时恢复对其它会话字段的重置（等同于 clear_vdd_state() 的效果，重置
current_device_prep、current_vdd_prep、current_use_vdd、original_output_name 等），确保
prepare_vdd() 能基于干净的会话状态运行并且旧 output_name 不会泄露到新会话。
In `@src/platform/windows/display_device/settings.cpp`:
- Around line 204-222: The code currently reuses previous_display_modes as
original_display_modes and only removes stale IDs from new_display_modes, which
lets stale IDs persist into current_settings.original_modes and later
set_display_modes/revert_settings(); to fix: before calling
determine_new_display_modes, filter previous_display_modes (the symbol
previous_display_modes) against valid_device_ids/valid_ids_set and if that
filtered map is empty call get_current_display_modes(valid_device_ids) to
rebuild original_display_modes (symbols: get_current_display_modes,
valid_device_ids); pass this cleaned original_display_modes into
determine_new_display_modes (symbol: determine_new_display_modes) and use the
same cleaned map when updating current_settings.original_modes so stale IDs are
never written back; additionally, ensure callers like
set_display_modes/revert_settings() skip or handle the case where the resulting
map is empty to avoid failures.
🪄 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: d890677e-85f5-48ef-82d9-11ab08679c81
📒 Files selected for processing (2)
src/display_device/session.cppsrc/platform/windows/display_device/settings.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/display_device/session.cppsrc/platform/windows/display_device/settings.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/settings.cpp
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/platform/windows/display_device/settings.cpp (1)
209-231:⚠️ Potential issue | 🟠 Major
original_display_modes仍会回写陈旧设备 ID，回滚链路可再次失败当前只清洗了
new_display_modes，但 Line 231 返回的是未清洗的original_display_modes。这会把 stale 设备 ID 再写回持久化状态，后续恢复流程仍可能命中不存在设备。🔧 建议修复
- const auto original_display_modes { previous_display_modes.empty() ? get_current_display_modes(valid_device_ids) : previous_display_modes }; - auto new_display_modes { determine_new_display_modes(resolution, refresh_rate, original_display_modes, metadata) }; + device_display_mode_map_t original_display_modes { + previous_display_modes.empty() ? get_current_display_modes(valid_device_ids) : previous_display_modes + }; + for (auto it = original_display_modes.begin(); it != original_display_modes.end();) { + if (valid_ids_set.find(it->first) == valid_ids_set.end()) { + BOOST_LOG(warning) << "Removing stale device from original display modes: " << it->first; + it = original_display_modes.erase(it); + } else { + ++it; + } + } + if (original_display_modes.empty()) { + original_display_modes = get_current_display_modes(valid_device_ids); + } + auto new_display_modes { determine_new_display_modes(resolution, refresh_rate, original_display_modes, metadata) }; @@ - return original_display_modes; + return original_display_modes;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/display_device/settings.cpp` around lines 209 - 231, original_display_modes may still contain stale device IDs and is returned as-is, reintroducing removed devices into persistence; before returning from the function that calls determine_new_display_modes/get_current_display_modes/previous_display_modes, filter original_display_modes with the same valid_ids_set cleanup used for new_display_modes (remove entries whose keys are not in valid_ids_set), optionally log removed IDs (similar message used when erasing from new_display_modes), and return the cleaned original_display_modes so rollback won't write back stale device IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/platform/windows/display_device/settings.cpp`:
- Around line 209-231: original_display_modes may still contain stale device IDs
and is returned as-is, reintroducing removed devices into persistence; before
returning from the function that calls
determine_new_display_modes/get_current_display_modes/previous_display_modes,
filter original_display_modes with the same valid_ids_set cleanup used for
new_display_modes (remove entries whose keys are not in valid_ids_set),
optionally log removed IDs (similar message used when erasing from
new_display_modes), and return the cleaned original_display_modes so rollback
won't write back stale device IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cc974a1f-e544-4489-89aa-adaa5fe4d03b
📒 Files selected for processing (2)
src/display_device/session.cppsrc/platform/windows/display_device/settings.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/display_device/session.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/settings.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/settings.cpp
🔇 Additional comments (1)
src/platform/windows/display_device/settings.cpp (1)
235-253: 回滚分支的 stale 过滤与空集合保护实现正确这里先过滤再调用
set_display_modes，并在空集合时跳过调用，能有效避免无效设备回滚失败。
ShadowLemoon
force-pushed
the
fix/client-switch
branch
from
44506e9 to
887ad65
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/platform/windows/display_device/settings.cpp (1)
210-231:⚠️ Potential issue | 🟠 Major
original_display_modes仍可能保留 stale 设备并写回持久状态Line 210 在
previous_display_modes非空时直接复用旧 map，Line 231 又将其原样返回，导致 stale device id 继续留在current_settings.original_modes。这会把问题延后到后续恢复阶段再失败。建议先清洗/必要时重建original_display_modes，再用于determine_new_display_modes()和返回值。🔧 建议修改
- const auto original_display_modes { previous_display_modes.empty() ? get_current_display_modes(valid_device_ids) : previous_display_modes }; - auto new_display_modes { determine_new_display_modes(resolution, refresh_rate, original_display_modes, metadata) }; + auto original_display_modes { previous_display_modes.empty() ? get_current_display_modes(valid_device_ids) : previous_display_modes }; + for (auto it = original_display_modes.begin(); it != original_display_modes.end();) { + if (valid_ids_set.find(it->first) == valid_ids_set.end()) { + BOOST_LOG(warning) << "Removing stale device from original display modes: " << it->first; + it = original_display_modes.erase(it); + } + else { + ++it; + } + } + if (original_display_modes.empty()) { + original_display_modes = get_current_display_modes(valid_device_ids); + } + auto new_display_modes { determine_new_display_modes(resolution, refresh_rate, original_display_modes, metadata) };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/windows/display_device/settings.cpp` around lines 210 - 231, The code currently reuses previous_display_modes into original_display_modes which can contain stale device IDs; before calling determine_new_display_modes (and before returning original_display_modes) filter or rebuild original_display_modes to only include IDs in valid_ids_set (or call get_current_display_modes(valid_device_ids) when any stale IDs are found) so stale entries are removed; update the logic around original_display_modes, previous_display_modes, get_current_display_modes and valid_ids_set to ensure determine_new_display_modes and the returned original_display_modes contain only current valid_device_ids.src/display_device/session.cpp (1)
290-295:⚠️ Potential issue | 🟠 Major客户端切换时未清空待恢复任务，可能把旧会话恢复逻辑带入新会话
Line 293 只调用了
stop_timer_and_clear_vdd_state()，但没有同步清理pending_restore_和 unlock 队列任务。若上个会话残留恢复任务，它可能在新会话期间被触发并回滚错误的显示状态。建议在该分支显式取消待恢复任务。🔧 建议修改
const auto saved_client_id = current_vdd_client_id; stop_timer_and_clear_vdd_state(); + pending_restore_ = false; + SessionEventListener::clear_unlock_task(); current_vdd_client_id = saved_client_id;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/display_device/session.cpp` around lines 290 - 295, When switching clients you call stop_timer_and_clear_vdd_state() but don't clear queued restore/unlock work; explicitly cancel/clear pending_restore_ and the unlock queue when swapping client IDs so old-session timers/callbacks cannot run in the new session. Update the branch around current_vdd_client_id by invoking/adding logic to cancel pending_restore_ (e.g., set pending_restore_ to null/false and unschedule its callback) and clear any unlock queue/pending unlock tasks (e.g., clear pending_unlocks_ or unlock_queue_) immediately after stop_timer_and_clear_vdd_state() and before resetting current_vdd_client_id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/display_device/session.cpp`:
- Around line 290-295: When switching clients you call
stop_timer_and_clear_vdd_state() but don't clear queued restore/unlock work;
explicitly cancel/clear pending_restore_ and the unlock queue when swapping
client IDs so old-session timers/callbacks cannot run in the new session. Update
the branch around current_vdd_client_id by invoking/adding logic to cancel
pending_restore_ (e.g., set pending_restore_ to null/false and unschedule its
callback) and clear any unlock queue/pending unlock tasks (e.g., clear
pending_unlocks_ or unlock_queue_) immediately after
stop_timer_and_clear_vdd_state() and before resetting current_vdd_client_id.
In `@src/platform/windows/display_device/settings.cpp`:
- Around line 210-231: The code currently reuses previous_display_modes into
original_display_modes which can contain stale device IDs; before calling
determine_new_display_modes (and before returning original_display_modes) filter
or rebuild original_display_modes to only include IDs in valid_ids_set (or call
get_current_display_modes(valid_device_ids) when any stale IDs are found) so
stale entries are removed; update the logic around original_display_modes,
previous_display_modes, get_current_display_modes and valid_ids_set to ensure
determine_new_display_modes and the returned original_display_modes contain only
current valid_device_ids.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3282376b-e816-4a56-aba0-7d8c36a20ca5
⛔ Files ignored due to path filters (1)
third-party/nvapi-open-source-sdkis excluded by!third-party/**
📒 Files selected for processing (2)
src/display_device/session.cppsrc/platform/windows/display_device/settings.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/display_device/session.cppsrc/platform/windows/display_device/settings.cpp
src/platform/**
⚙️ CodeRabbit configuration file
src/platform/**: 平台抽象层代码（Windows/Linux/macOS）。确保各平台实现一致， 注意 Windows API 调用的错误处理和资源释放。
Files:
src/platform/windows/display_device/settings.cpp
🔇 Additional comments (1)
src/platform/windows/display_device/settings.cpp (1)
235-253: 回滚分支的 stale 过滤与空集保护实现合理先过滤无效设备再执行
set_display_modes()，并在空结果时跳过调用，这个防护是必要的。
Collaborator
Author
|
核心修复只有session.cpp里面的保存覆盖。其他的更改是防御性代码：防止对[无法设置分辨率/刷新率]的屏幕（不在拓扑中）设置刷新率/分辨率 |
问题： configure_display() 检测到新客户端时，stop_timer_and_clear_vdd_state() 会提前清空 current_vdd_client_id，导致 prepare_vdd() 中客户端切换的 VDD 重建逻辑被跳过（该逻辑自 d6e3f1e 引入以来一直是死代码）。 后果链： 1. VDD 重建被跳过 → 旧 VDD 被 reload_driver() 意外销毁 2. recovery 创建新 VDD，但 persistent_data 中残留旧 device ID 3. set_display_modes 尝试配置已不存在的设备 → 整体失败 → 黑屏 修复： - 将 current_vdd_client_id 的生命周期与 VDD 设备绑定， 仅在 destroy_vdd_monitor() 中清除，不再随 clear_vdd_state() 清理 - 客户端切换时取消旧会话的待恢复任务（pending_restore_ + unlock task）， 防止旧 restore 干扰新会话 - settings.cpp: 在 set_display_modes 前过滤 stale device ID（正常/回滚分支） - 无头主机优化：VDD 是唯一显示设备时跳过销毁，避免无意义的 销毁→重建循环（device ID 变化导致 persistent_data 失效） 触发条件：无头主机 + 多客户端轮流连接（VDD 跨会话存活时切换客户端）
ShadowLemoon
force-pushed
the
fix/client-switch
branch
from
e753e0c to
558406d
Compare
ShadowLemoon
changed the title
This was referenced Apr 8, 2026
Merged
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
2 participants
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.
问题：
configure_display() 检测到新客户端时，stop_timer_and_clear_vdd_state()
会提前清空 current_vdd_client_id，导致 prepare_vdd() 中客户端切换的
VDD 重建逻辑被跳过（该逻辑自 d6e3f1e 引入以来一直是死代码）。
后果链：
修复：
仅在 destroy_vdd_monitor() 中清除，不再随 clear_vdd_state() 清理
防止旧 restore 干扰新会话
销毁→重建循环（device ID 变化导致 persistent_data 失效）
触发条件：无头主机 + 多客户端轮流连接（VDD 跨会话存活时切换客户端）