Conversation
|
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 (1)src/**/*.{cpp,c,h}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
Summary by CodeRabbit
Walkthrough扩展 Changes
Sequence Diagram(s)sequenceDiagram
participant Session as Session (调用者)
participant VDD as vdd_utils::create_vdd_monitor
participant Driver as VDD Driver (NamedPipe)
Session->>VDD: request create_vdd_monitor(pipe, ...)
VDD->>Driver: write CREATEMONITOR command (over pipe)
Driver-->>VDD: response ("OK", "OK_PENDING", "FAIL", or other) or timeout
alt response == "FAIL"
VDD-->>Session: return false (log error)
Session->>Session: try_recover_vdd_device() / disable_enable_vdd()
else response == "OK" or "OK_PENDING"
VDD-->>Session: return true (log success/pending)
Session->>Session: continue normal flow (sleep/restore)
else timeout or other
VDD-->>Session: handle as legacy/other response (may treat timeout as success)
Session->>Session: follow legacy handling or additional checks
end
预估代码审查工作量🎯 4 (Complex) | ⏱️ ~45 分钟 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
src/display_device/vdd_utils.cpp (2)
359-362: 日志消息中return标签含义不清。此处记录的是
read_timed_out变量，但日志显示[return=...]，可能造成维护时的困惑。建议改为更直观的标签名：📝 建议修改
- BOOST_LOG(info) << "创建虚拟显示器完成，响应: " << response << " [return=" << (read_timed_out ? 1 : 0) << "]"; + BOOST_LOG(info) << "创建虚拟显示器完成，响应: " << response << " [read_timed_out=" << (read_timed_out ? 1 : 0) << "]";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/display_device/vdd_utils.cpp` around lines 359 - 362, The log message in the else branch that prints response uses an ambiguous label "[return=...]" while actually showing the boolean variable read_timed_out; update the log to use a clear label (e.g., "[read_timed_out=...]" or "[timed_out=...]") so it accurately reflects the variable, by modifying the BOOST_LOG(info) statement that prints response and read_timed_out in vdd_utils.cpp (the branch that treats old drivers as successful).
45-45: 静态变量last_used_client_uuid缺少线程同步。该静态变量在
create_vdd_monitor中被读写（行 282、321-323），但没有同步保护。如果存在多线程调用场景，可能导致数据竞争。如果确认此函数仅在单线程上下文中调用，可忽略此建议。否则建议添加
std::mutex保护或使用std::atomic/thread_local。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/display_device/vdd_utils.cpp` at line 45, The static variable last_used_client_uuid is accessed from create_vdd_monitor and lacks synchronization; protect it by either making it thread_local (if each thread should have its own value) or adding a mutex (e.g., declare a static std::mutex last_used_client_uuid_mtx) and lock_guard it around every read/write of last_used_client_uuid inside create_vdd_monitor (and any other accessors) to eliminate data races; ensure both reads (checks) and writes (updates) use the same synchronization primitive.
🤖 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/vdd_utils.cpp`:
- Around line 348-351: create_vdd_monitor now returns false on driver "FAIL" but
two callers in session.cpp (the call before wait_for_vdd_device() and the
headless-host creation call before is_display_on() polling) ignore the return;
update both call sites to check the boolean result from
vdd_utils::create_vdd_monitor(...) and handle failures immediately (e.g., log an
error via BOOST_LOG/error with context, perform any needed cleanup, and
return/propagate failure instead of continuing to rely solely on
wait_for_vdd_device() or is_display_on() polling). Ensure you reference the
existing call sites that invoke create_vdd_monitor, and keep the existing
subsequent polling logic as a fallback only after the create_vdd_monitor success
check.
---
Nitpick comments:
In `@src/display_device/vdd_utils.cpp`:
- Around line 359-362: The log message in the else branch that prints response
uses an ambiguous label "[return=...]" while actually showing the boolean
variable read_timed_out; update the log to use a clear label (e.g.,
"[read_timed_out=...]" or "[timed_out=...]") so it accurately reflects the
variable, by modifying the BOOST_LOG(info) statement that prints response and
read_timed_out in vdd_utils.cpp (the branch that treats old drivers as
successful).
- Line 45: The static variable last_used_client_uuid is accessed from
create_vdd_monitor and lacks synchronization; protect it by either making it
thread_local (if each thread should have its own value) or adding a mutex (e.g.,
declare a static std::mutex last_used_client_uuid_mtx) and lock_guard it around
every read/write of last_used_client_uuid inside create_vdd_monitor (and any
other accessors) to eliminate data races; ensure both reads (checks) and writes
(updates) use the same synchronization primitive.
🪄 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: 031eca75-351d-488c-8d5d-824468a5620b
📒 Files selected for processing (2)
src/display_device/vdd_utils.cppsrc/display_device/vdd_utils.h
📜 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/display_device/vdd_utils.hsrc/display_device/vdd_utils.cpp
🔇 Additional comments (4)
src/display_device/vdd_utils.h (1)
69-70: API 变更设计合理。新增
read_timeout_ms参数使用默认值 0，保证了向后兼容性。现有调用方无需修改即可保持原有行为。src/display_device/vdd_utils.cpp (3)
143-144: LGTM！超时参数实现正确。新增
read_timeout_ms参数与头文件声明一致，配合effective_read_timeout计算逻辑，允许调用方按需定制读取超时。
326-329: 超时设置合理。根据 PR 描述，驱动端 CCD 准备最多需要 15 秒，20 秒超时提供了适当的缓冲。仅针对
CREATEMONITOR命令使用此超时，不影响其他管道命令。
346-362: 响应字符串比较可能因 null 字符而失败（需确认驱动行为）。
response由std::string(buffer, bytesRead)构造（第 196-197、206-207 行），该构造函数会取 buffer 中恰好 bytesRead 个字符，包括任何嵌入的 null 字符。如果 ZakoVDD 驱动在响应中发送 null 终止符（如"OK\0"计为 3 字节），则 bytesRead=3，response 将包含长度为 3 的字符串，导致第 353、356 行的比较response == "OK"失败。代码结构（ReadFile 预留一个字节、之后手动添加 null）表明设计上预期接收非 null 终止的原始数据。若驱动遵循此协议，当前代码正确；若驱动包含 null 字节在长度计数内，则需要修复。
建议在字符串比较前添加 null 及尾部空白字符的清理，或采用
starts_with()等更宽松的匹配方式，以增强鲁棒性。
1 participant
背景
配合 ZakoVDD 驱动端修改（CREATEMONITOR 命令现在返回 CCD 就绪状态），更新 Sunshine 管道通信层以利用新响应。
修改
vdd_utils.h / vdd_utils.cpp：
execute_pipe_command新增read_timeout_ms参数（默认0= 使用kPipeTimeoutMs），允许调用者指定自定义读取超时create_vdd_monitor使用 20 秒读取超时（kCreateMonitorReadTimeoutMs），适配驱动端最多 15 秒的 CCD 等待OK：驱动确认 CCD 已就绪，后续wait_for_vdd_device会立即成功OK_PENDING：创建成功但 CCD 未就绪，走原有等待逻辑FAIL：创建失败，create_vdd_monitor返回false，触发try_recover_vdd_device重试向后兼容
read_timed_out=true→ 与之前行为一致CREATEMONITOR命令，其他命令不受影响