Fix 120Hz keepalive in smart framerate mode#35
Open
qiin2333 wants to merge 2 commits into
Open
Conversation
Contributor
|
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 (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthrough该PR为Moonlight在HarmonyOS NEXT智能帧率模式下120fps流播放时可能降至60Hz的问题，引入DisplaySoloist显示层高刷保活与帧率提示持续刷新机制，从Native API加载、生命周期管理，到应用层控制，完整实现显示层高刷保持链路。 ChangesDisplaySoloist帧率保活与动态刷新
🎯 3 (Moderate) | ⏱️ ~25 minutes 可能相关的 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
nativelib/src/main/cpp/native_render.cpp (1)
332-350:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift缺少“降级/清除”路径，120Hz 提示会在降到 60fps 或停流后残留。
当前逻辑只在
configuredFps_ > 60时重新应用提示；一旦从 120→60，或者 ArkTS 仅调用SetDisplayFramePacerEnabled(false)，这里只会停掉DisplaySoloist，不会把已经写到NativeWindow/NativeVSync的高刷提示恢复为默认值。页面保留 surface 显示战报或流内切到 60fps 时，设备仍可能继续维持高刷新率。Also applies to: 419-439, 478-500
🤖 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 `@nativelib/src/main/cpp/native_render.cpp` around lines 332 - 350, SetConfiguredFps currently only re-applies high-FPS hints when fps>60 and simply releases DisplaySoloist when fps<=60, leaving previously-applied NativeVSync/NativeWindow high-refresh hints in place; modify SetConfiguredFps to explicitly clear or reset frame-rate hints when downgrading (e.g., when configuredFps_ <= 60 or when displayFramePacerEnabled_ becomes false) by calling the same mechanisms that apply defaults (or explicit clear functions) in ApplyFrameRateRange and ApplyNativeWindowFrameRate (or add ClearFrameRateRange/ClearNativeWindowFrameRate helpers) so that any high-refresh hints written to NativeVSync/NativeWindow are reverted; ensure ReleaseDisplaySoloist and the branch where displayFramePacerEnabled_.load() is false also trigger these clear/reset calls to remove lingering 120Hz hints.nativelib/src/main/cpp/moonlight_bridge.cpp (1)
1998-2018:⚠️ Potential issue | 🟠 MajorXComponent 帧率设置：API20 调用失败时应继续回退到旧路径
MoonBridge_SetXComponentFrameRate() 在 19982005 检测到2018 的g_pfnXCSetFrameRateNew存在后，无论xcRet成功与否都会立刻return，导致下面 2008g_pfnGetNativeXC/g_pfnXCSetFrameRateOld旧路径永远不会执行。建议仅在 API20 调用成功时返回；失败时移除无条件 return，让代码继续尝试旧路径（并以 HarmonyOS 对该返回码的约定作为成功判定）。🤖 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 `@nativelib/src/main/cpp/moonlight_bridge.cpp` around lines 1998 - 2018, MoonBridge_SetXComponentFrameRate(): currently when g_pfnXCSetFrameRateNew is present the code calls it and unconditionally returns, preventing the fallback path; change the flow so you only return (GetUndefined(env)) when the new call succeeds (check xcRet against the HarmonyOS success convention), and if xcRet indicates failure log the error and do NOT return so execution falls through to the g_pfnGetNativeXC / g_pfnXCSetFrameRateOld fallback; keep existing logging for both paths and retain the GetUndefined(env) return after the fallback.
🤖 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 `@entry/src/main/ets/pages/StreamPage.ets`:
- Around line 1277-1314: launchStream enables DisplaySoloist/XComponent hints
via applyDisplayFrameRateHints(true) before awaiting viewModel.startStreaming,
but failures leave the pacer/hints enabled; change launchStream to wrap the
await this.viewModel.startStreaming(...) in a try/catch/finally (or at minimum
catch) and on error disable the pacer/hints by calling
applyDisplayFrameRateHints(false) (or directly call
MoonBridge.setDisplayFramePacerEnabled(false) and reset XComponent/frame hints)
so any startup exception cleans up state; keep the original logging/throw
behavior but ensure applyDisplayFrameRateHints(false) is invoked on the error
path (also apply same fix to the other launch path referenced around lines
1381-1431).
In `@nativelib/src/main/cpp/native_render.cpp`:
- Around line 242-297: Add a mutex to serialize access to
DisplaySoloist/window/frame-rate state and update the lifecycle functions to use
it: create a member std::mutex (e.g., displayMutex_) and protect reads/writes of
displaySoloist_, displaySoloistStarted_, window_, lastFrameRateHintTime_, and
configuredFps_ with std::lock_guard in InitDisplaySoloist,
ReleaseDisplaySoloist, SetDisplayFramePacerEnabled,
ApplyDisplaySoloistFrameRate, ApplyNativeWindowFrameRate, RefreshFrameRateHints,
SubmitFrame, SetNativeWindow, and SetConfiguredFps; for calls into external APIs
(g_pfnDisplaySoloistStart/Stop/Destroy) copy the pointer/flags under the lock to
local variables, release the lock, then invoke the external function to avoid
deadlocks while still preventing UAF/race windows. Ensure all state transitions
use the same mutex so no concurrent destroy/start or updates occur.
---
Outside diff comments:
In `@nativelib/src/main/cpp/moonlight_bridge.cpp`:
- Around line 1998-2018: MoonBridge_SetXComponentFrameRate(): currently when
g_pfnXCSetFrameRateNew is present the code calls it and unconditionally returns,
preventing the fallback path; change the flow so you only return
(GetUndefined(env)) when the new call succeeds (check xcRet against the
HarmonyOS success convention), and if xcRet indicates failure log the error and
do NOT return so execution falls through to the g_pfnGetNativeXC /
g_pfnXCSetFrameRateOld fallback; keep existing logging for both paths and retain
the GetUndefined(env) return after the fallback.
In `@nativelib/src/main/cpp/native_render.cpp`:
- Around line 332-350: SetConfiguredFps currently only re-applies high-FPS hints
when fps>60 and simply releases DisplaySoloist when fps<=60, leaving
previously-applied NativeVSync/NativeWindow high-refresh hints in place; modify
SetConfiguredFps to explicitly clear or reset frame-rate hints when downgrading
(e.g., when configuredFps_ <= 60 or when displayFramePacerEnabled_ becomes
false) by calling the same mechanisms that apply defaults (or explicit clear
functions) in ApplyFrameRateRange and ApplyNativeWindowFrameRate (or add
ClearFrameRateRange/ClearNativeWindowFrameRate helpers) so that any high-refresh
hints written to NativeVSync/NativeWindow are reverted; ensure
ReleaseDisplaySoloist and the branch where displayFramePacerEnabled_.load() is
false also trigger these clear/reset calls to remove lingering 120Hz hints.
🪄 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: 2e6c796f-e176-4c0f-9899-2e50c69b7a76
📒 Files selected for processing (7)
entry/src/main/ets/pages/StreamPage.etsentry/src/main/ets/service/streaming/MoonBridge.etsnativelib/src/main/cpp/moonlight_bridge.cppnativelib/src/main/cpp/moonlight_bridge.hnativelib/src/main/cpp/napi_init.cppnativelib/src/main/cpp/native_render.cppnativelib/src/main/cpp/native_render.h
Comment on lines
1277
to
1314
| /** | ||
| * 设置 XComponent 帧率 + 获取 surfaceId + 启动串流 | ||
| * 应用显示链路帧率提示。 | ||
| * 120fps 串流时启动 native DisplaySoloist，并重申 XComponent / native 层帧率范围。 | ||
| */ | ||
| private async launchStream(): Promise<void> { | ||
| const surfaceId = this.xComponentController.getXComponentSurfaceId(); | ||
|
|
||
| // 设置 XComponent 期望帧率（通过 FrameNode → ArkUI_NodeHandle） | ||
| // 解决 MatePad 等设备渲染帧率被锁定在 60fps 的问题 | ||
| private applyDisplayFrameRateHints(enablePacer: boolean = true): void { | ||
| const streamConfig = this.viewModel.streamConfig; | ||
| if (streamConfig && streamConfig.fps > 60) { | ||
| try { | ||
| if (enablePacer) { | ||
| MoonBridge.setDisplayFramePacerEnabled(true); | ||
| } | ||
| const frameNode = this.getUIContext().getFrameNodeById('streamSurface'); | ||
| if (frameNode) { | ||
| MoonBridge.setXComponentFrameRate(frameNode, streamConfig.fps); | ||
| console.info(`XComponent 帧率已设置为 ${streamConfig.fps} fps`); | ||
| } | ||
| MoonBridge.refreshFrameRateHints(); | ||
| } catch (e) { | ||
| console.warn(`设置 XComponent 帧率失败: ${e}`); | ||
| console.warn(`设置显示帧率提示失败: ${e}`); | ||
| } | ||
| } else if (enablePacer) { | ||
| try { | ||
| MoonBridge.setDisplayFramePacerEnabled(false); | ||
| } catch (_) {} | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 设置 XComponent 帧率 + 获取 surfaceId + 启动串流 | ||
| */ | ||
| private async launchStream(): Promise<void> { | ||
| const surfaceId = this.xComponentController.getXComponentSurfaceId(); | ||
|
|
||
| // 设置 XComponent / DisplaySoloist 期望帧率，解决智能帧率下 120fps 回落到 60Hz 的问题 | ||
| this.applyDisplayFrameRateHints(true); | ||
|
|
||
| await this.viewModel.startStreaming(this.computerId, this.appId, surfaceId, this.context, this.displayGuid, this.useVdd); | ||
|
|
Contributor
There was a problem hiding this comment.
启动前先打开 pacer，但失败路径没有回滚。
launchStream() 在真正 startStreaming() 之前就启用了 DisplaySoloist/XComponent/native hints，而 startStreaming() 的 catch 里只打印日志。这样只要连接或启动阶段抛错，页面停在错误态时高刷保持仍可能继续生效，直到用户离开页面才被动清理。
建议改法
async startStreaming(): Promise<void> {
try {
...
await this.launchStream();
this.streamStartedAtMs = Date.now();
this.applyDisplayFrameRateHints(true);
...
} catch (err) {
+ try {
+ MoonBridge.setDisplayFramePacerEnabled(false);
+ } catch (_) {}
console.error('开始串流失败:', err);
}
}Also applies to: 1381-1431
🤖 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 `@entry/src/main/ets/pages/StreamPage.ets` around lines 1277 - 1314,
launchStream enables DisplaySoloist/XComponent hints via
applyDisplayFrameRateHints(true) before awaiting viewModel.startStreaming, but
failures leave the pacer/hints enabled; change launchStream to wrap the await
this.viewModel.startStreaming(...) in a try/catch/finally (or at minimum catch)
and on error disable the pacer/hints by calling
applyDisplayFrameRateHints(false) (or directly call
MoonBridge.setDisplayFramePacerEnabled(false) and reset XComponent/frame hints)
so any startup exception cleans up state; keep the original logging/throw
behavior but ensure applyDisplayFrameRateHints(false) is invoked on the error
path (also apply same fix to the other launch path referenced around lines
1381-1431).
Contributor
Author
|
Addressed the actionable review feedback:
Verification:
|
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.
Summary
Closes #34
Verification
Summary by CodeRabbit
发布说明
新功能
bug修复