fix(vrr): limit input boost to keyboard events#727
Merged
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 (3)
💤 Files with no reviewable changes (2)
📜 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 (3)
Summary by CodeRabbit发行说明
Walkthrough收缩 activity tracker 为仅处理键盘事件；将 input_activity_event 的触发从入队时移至 passthrough_next_message 批处理并分发后；头文件移除 reset 与控制器状态相关声明，入队处即时 evaluate/raise 被移除。 输入活动检测与信号时机重构
Sequence Diagram(s)sequenceDiagram
participant EnqueueThread as 入队线程
participant WorkerThread as 工作线程
participant ActivityEvent as input_activity_event
participant PlatformSwitch as 平台分发
EnqueueThread->>WorkerThread: enqueue payload (passthrough)
WorkerThread->>WorkerThread: 批处理并计算最终 payload
WorkerThread->>WorkerThread: activity_tracker.evaluate(payload)（受 config::video.input_activity_boost 控制）
WorkerThread->>ActivityEvent: raise(steady_clock::now()) 若 should_raise_input_activity 为真
WorkerThread->>PlatformSwitch: 分发 / switch 调用完成
🎯 代码审查工作量评估🎯 3 (中等) | ⏱️ ~20 分钟 可能相关的 PR
🚥 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/input.cpp (1)
1585-1639:⚠️ Potential issue | 🟠 Major修正：活动时机逻辑正确，但
activity_tracker需要同步以避免数据竞争
should_raise_input_activity在批处理后的最终 payload 上评估，OS 分发完成后再触发input_activity_event->raise()，时序符合 PR 目标。- 线程安全问题：
activity_tracker.evaluate()（src/input.cpp 1587）在task_poolworker 中执行，而activity_tracker.reset()（src/input.cpp 1661）在stream::session::join()调用input::reset()的线程直接执行；tracker_t（src/input_activity.h/.cpp）仅通过controllers_成员读写且未加锁/原子，因此若passthrough_next_message()与reset()重叠会产生未同步的并发访问（UB）。- 建议：将
activity_tracker.reset()也通过task_pool串行化执行并等待完成；或在tracker_t内部加入互斥/原子保护，并确保会话停用时不会再有未完成的passthrough_next_message()任务与 reset 并发运行。🤖 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/input.cpp` around lines 1585 - 1639, The activity_tracker is accessed concurrently (activity_tracker.evaluate() in passthrough_next_message/task_pool and activity_tracker.reset() called directly in input::reset()/stream::session::join()), causing data races on tracker_t/controllers_; fix by serializing reset on the same task_pool: change input::reset() (called from stream::session::join()) to post a synchronous/waiting task to the same task_pool that runs activity_tracker.reset(), or otherwise dispatch a task that performs activity_tracker.reset() and wait for its completion before returning; alternatively, protect tracker_t internal state with a mutex/atomics, but prefer posting reset to the task_pool to ensure no passthrough_next_message()/activity_tracker.evaluate() tasks run concurrently with reset (reference symbols: activity_tracker.evaluate, activity_tracker.reset, passthrough_next_message, input::reset, stream::session::join, tracker_t).
🤖 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.
Outside diff comments:
In `@src/input.cpp`:
- Around line 1585-1639: The activity_tracker is accessed concurrently
(activity_tracker.evaluate() in passthrough_next_message/task_pool and
activity_tracker.reset() called directly in
input::reset()/stream::session::join()), causing data races on
tracker_t/controllers_; fix by serializing reset on the same task_pool: change
input::reset() (called from stream::session::join()) to post a
synchronous/waiting task to the same task_pool that runs
activity_tracker.reset(), or otherwise dispatch a task that performs
activity_tracker.reset() and wait for its completion before returning;
alternatively, protect tracker_t internal state with a mutex/atomics, but prefer
posting reset to the task_pool to ensure no
passthrough_next_message()/activity_tracker.evaluate() tasks run concurrently
with reset (reference symbols: activity_tracker.evaluate,
activity_tracker.reset, passthrough_next_message, input::reset,
stream::session::join, tracker_t).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98c71e97-e893-4461-9d71-5b56bb372666
📒 Files selected for processing (2)
src/input.cppsrc/input_activity.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 (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/input_activity.cppsrc/input.cpp
🔇 Additional comments (3)
src/input_activity.cpp (2)
20-44: LGTM!
78-87: LGTM!src/input.cpp (1)
1647-1654: LGTM!
qiin2333
force-pushed
the
codex/fix-controller-boost-renewal
branch
2 times, most recently
from
9a76a01 to
1d432ba
Compare
qiin2333
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/input_activity.cpp (1)
129-130: ⚡ Quick win建议添加注释说明禁用控制器活动检测的原因。
SS_CONTROLLER_TOUCH_MAGIC（第 119-122 行）有详细注释解释为何返回false，而MULTI_CONTROLLER_MAGIC_GEN5缺少类似说明。鉴于此更改完全禁用了控制器输入对 boost 窗口的影响，建议添加注释以便后续维护者理解设计意图。♻️ 建议的修改
case MULTI_CONTROLLER_MAGIC_GEN5: + // Controller input is disabled from activity boost to avoid the boost window + // being continuously renewed by analog stick/trigger jitter, which competes + // with game rendering and causes input lag. See PR `#727`. 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/input_activity.cpp` around lines 129 - 130, 在 input_activity.cpp 的 switch 分支中针对 case MULTI_CONTROLLER_MAGIC_GEN5 添加一段简短注释，说明为什么要返回 false 并禁用该控制器对 boost 窗口的输入影响；注释风格应与现有的 SS_CONTROLLER_TOUCH_MAGIC（第 119-122 行）的注释一致，包含设计意图和任何相关硬件/兼容性或行为理由，以便后续维护者理解此决策；确保注释放在 return false 之前并清晰指明 MULTI_CONTROLLER_MAGIC_GEN5 的特殊处理。
🤖 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/input_activity.cpp`:
- Around line 78-87: tracker_t::evaluate_controller is dead code because
tracker_t::evaluate() immediately returns false in the
MULTI_CONTROLLER_MAGIC_GEN5 branch; either call evaluate_controller from that
branch and return its result so multi-controller trigger logic runs (look for
tracker_t::evaluate() and invoke
evaluate_controller(_NV_MULTI_CONTROLLER_PACKET*) where the branch currently
returns), or remove the unused evaluate_controller function (or add a clear
comment explaining why it is intentionally kept) so the codebase has no
unreachable functions.
---
Nitpick comments:
In `@src/input_activity.cpp`:
- Around line 129-130: 在 input_activity.cpp 的 switch 分支中针对 case
MULTI_CONTROLLER_MAGIC_GEN5 添加一段简短注释，说明为什么要返回 false 并禁用该控制器对 boost
窗口的输入影响；注释风格应与现有的 SS_CONTROLLER_TOUCH_MAGIC（第 119-122
行）的注释一致，包含设计意图和任何相关硬件/兼容性或行为理由，以便后续维护者理解此决策；确保注释放在 return false 之前并清晰指明
MULTI_CONTROLLER_MAGIC_GEN5 的特殊处理。
🪄 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: b6a2b3a7-c313-4899-889b-209bdf3c1126
📒 Files selected for processing (2)
src/input.cppsrc/input_activity.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/input.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 (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/input_activity.cpp
🔇 Additional comments (3)
src/input_activity.cpp (3)
25-46: LGTM!
119-122: LGTM!
136-141: LGTM!
qiin2333
force-pushed
the
codex/fix-controller-boost-renewal
branch
from
1d432ba to
d67fae8
Compare
qiin2333
changed the title
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.
改了啥呀
KEY_DOWN_EVENT_MAGIC、KEY_UP_EVENT_MAGIC、UTF8_TEXT_EVENT_MAGIC。activity_tracker已简化为无状态判断，删除了 controller 追踪状态和 reset API；之前 review 提到的 reset/evaluate 并发访问点因此不再存在。为啥要改
#722 的 boost 会在 VRR 无新帧时临时 encode stale frame，用来改善静态画面下输入后的视觉反馈。但手柄实测已经证明，连续输入流触发这个机制会和 RD / 游戏渲染争资源，导致 Android / 鸿蒙客户端出现输入响应变慢、RD 帧率突然下降又恢复。
为了先拿到稳定、低风险的行为，这版只保留键盘事件触发 boost。键盘输入是离散事件，基本不会被高频流持续续命；鼠标/touch/pen/controller 这些可能高频连续的输入先全部排除，杂鱼高频事件不要再拖着编码跑呀。
验证
git diff --check -- src/input.cpp src/input_activity.cpp src/input_activity.hcmake --build build --target sunshine --parallelbuild/cpack_artifacts/_CPack_Packages/win64/ZIP/Sunshine/sunshine.exe --helpcpack -G ZIP --config .\CPackConfig.cmake --verbose，生成build/cpack_artifacts/Sunshine.zip