feat(hdr): add end-to-end HDR10+ support - #483
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughHDR output handling now negotiates display modes by HDR type, supports HDR10+ decoder profiles and metadata observation, tracks typed stream formats, and reports HDR format details in overlays and diagnostics. ChangesHDR output pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The PR adds HDR10+ capability detection, profile selection, metadata observation, display-mode handling, and fallback behavior; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ConnectionCallbackHandler
participant Game
participant DisplayModeManager
participant MediaCodecDecoderRenderer
participant Hdr10PlusOutputObserver
participant PerformanceOverlayManager
ConnectionCallbackHandler->>Game: prepareInitialHdrOutput()
Game->>DisplayModeManager: selectBestDisplayMode(..., acceptableHdrTypes)
Game->>MediaCodecDecoderRenderer: configure requested HDR10+
MediaCodecDecoderRenderer->>Hdr10PlusOutputObserver: query output metadata
Hdr10PlusOutputObserver-->>MediaCodecDecoderRenderer: report observed HDR10+ state
MediaCodecDecoderRenderer-->>Game: provide negotiated HDR format
Game->>PerformanceOverlayManager: provide hdrFormat
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end Android HDR10+ negotiation, decoding, observation, display-mode selection, diagnostics, and overlay reporting.
Changes:
- Adds HDR10+ codec profile probing with HDR10/automatic fallback.
- Tracks dynamic metadata and reports SDR, HDR10, HDR10+, or HLG.
- Selects HDR-capable Android 14 display modes and preserves HEVC SEI metadata.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
StreamHdrFormatPolicyTest.kt |
Tests HDR format resolution. |
HdrDecoderProfilePolicyTest.kt |
Tests profile fallback ordering. |
Hdr10PlusMetadataTrackerTest.kt |
Tests metadata tracking. |
values/strings.xml |
Adds HDR overlay label. |
values-zh-rCN/strings.xml |
Adds Chinese HDR label. |
HdrCapabilityHelper.kt |
Checks mode-specific HDR support. |
CapabilityDiagnosticActivity.kt |
Reports HEVC/AV1 HDR profiles. |
PerformanceOverlayManager.kt |
Displays precise HDR format. |
MoonBridge.java |
Adds HEVC SEI capability flag. |
Game.kt |
Integrates HDR negotiation and display selection. |
DisplayModeManager.kt |
Filters and pins HDR-capable modes. |
ConnectionCallbackHandler.kt |
Separates HDR output preparation. |
StreamHdrFormat.kt |
Defines HDR format policy. |
RendererException.kt |
Adds HDR10+ diagnostics. |
PerformanceInfo.kt |
Stores typed HDR state. |
MediaCodecHelper.kt |
Bounds Qualcomm fallback attempts. |
MediaCodecDecoderRenderer.kt |
Implements HDR10+ configuration and observation. |
HdrDecoderProfilePolicy.kt |
Defines profile candidates. |
Hdr10PlusMetadataTracker.kt |
Tracks metadata statistics. |
Suppressed comments (1)
app/src/main/java/com/limelight/Game.kt:839
- The AV1 path has the same fallback regression: a decoder exposing Main10 but not the dedicated HDR10 constant is rejected before the profile-less candidate can be attempted. Use the broader Main10 capability here; exact HDR profile selection still occurs later in
HdrDecoderProfilePolicy.
decoderRenderer?.isAv1Main10Hdr10Supported() == true
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| val hevcHdrSupported = if (prefConfig.hdrMode == MoonBridge.HDR_MODE_HLG) { | ||
| decoderRenderer?.isHevcMain10Supported() == true | ||
| } else { | ||
| decoderRenderer?.isHevcMain10Hdr10Supported() == true |
| val shouldProbe = if (hdr10PlusMetadataObserved) { | ||
| hdr10PlusOutputFramesObserved % HDR10_PLUS_SAMPLE_INTERVAL_FRAMES == 0L | ||
| } else { | ||
| hdr10PlusOutputFramesObserved <= HDR10_PLUS_INITIAL_PROBE_FRAMES || | ||
| hdr10PlusOutputFramesObserved % HDR10_PLUS_SAMPLE_INTERVAL_FRAMES == 0L |
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 (1)
app/src/main/java/com/limelight/Game.kt (1)
799-814: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSynchronize HDR10+ state after HDR fallback.
When the decoder check sets
willStreamHdrtofalse, also clear the renderer’srequestedHdr10Plusstate. The renderer retains this constructor value, uses it for HDR10+ profile selection, and may advertiseCAPABILITY_PRESERVE_HEVC_SEIafter HDR is disabled.🤖 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 `@app/src/main/java/com/limelight/Game.kt` around lines 799 - 814, Update the decoder fallback logic in Game so that whenever willStreamHdr is set to false, the decoder renderer’s requestedHdr10Plus state is also cleared. Ensure the MediaCodecDecoderRenderer constructor receives the synchronized HDR10+ value, preventing HDR10+ profile selection or CAPABILITY_PRESERVE_HEVC_SEI advertisement after HDR is disabled.
🤖 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 `@app/src/main/java/com/limelight/ConnectionCallbackHandler.kt`:
- Around line 229-231: Replace the app-capability check around
prepareInitialHdrOutput in ConnectionCallbackHandler with the negotiated HDR
result from Game.buildStreamConfiguration(), persisting that willStreamHdr
outcome on Game. Call prepareInitialHdrOutput() only when the persisted
negotiated state is true and HDR preferences remain enabled.
In `@app/src/main/java/com/limelight/DisplayModeManager.kt`:
- Around line 92-94: Initialize bestMode from display.mode rather than selecting
an eligible mode before constraint evaluation. In the mode-selection loop,
assign a candidate only after it passes all existing resolution, refresh-rate,
and HDR constraints, preserving display.mode when no candidate is accepted.
---
Outside diff comments:
In `@app/src/main/java/com/limelight/Game.kt`:
- Around line 799-814: Update the decoder fallback logic in Game so that
whenever willStreamHdr is set to false, the decoder renderer’s
requestedHdr10Plus state is also cleared. Ensure the MediaCodecDecoderRenderer
constructor receives the synchronized HDR10+ value, preventing HDR10+ profile
selection or CAPABILITY_PRESERVE_HEVC_SEI advertisement after HDR is disabled.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b1dc7dc-d4cd-4fd7-8ec5-3d4675247c2f
📒 Files selected for processing (19)
app/src/main/java/com/limelight/ConnectionCallbackHandler.ktapp/src/main/java/com/limelight/DisplayModeManager.ktapp/src/main/java/com/limelight/Game.ktapp/src/main/java/com/limelight/PerformanceOverlayManager.ktapp/src/main/java/com/limelight/binding/video/Hdr10PlusMetadataTracker.ktapp/src/main/java/com/limelight/binding/video/HdrDecoderProfilePolicy.ktapp/src/main/java/com/limelight/binding/video/MediaCodecDecoderRenderer.ktapp/src/main/java/com/limelight/binding/video/MediaCodecHelper.ktapp/src/main/java/com/limelight/binding/video/PerformanceInfo.ktapp/src/main/java/com/limelight/binding/video/RendererException.ktapp/src/main/java/com/limelight/binding/video/StreamHdrFormat.ktapp/src/main/java/com/limelight/nvstream/jni/MoonBridge.javaapp/src/main/java/com/limelight/preferences/CapabilityDiagnosticActivity.ktapp/src/main/java/com/limelight/utils/HdrCapabilityHelper.ktapp/src/main/res/values-zh-rCN/strings.xmlapp/src/main/res/values/strings.xmlapp/src/test/java/com/limelight/binding/video/Hdr10PlusMetadataTrackerTest.ktapp/src/test/java/com/limelight/binding/video/HdrDecoderProfilePolicyTest.ktapp/src/test/java/com/limelight/binding/video/StreamHdrFormatPolicyTest.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/limelight/binding/video/Hdr10PlusOutputObserver.kt (1)
84-114: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMove the MediaCodec query out of the critical section.
observeOutputholdslockwhile it callscodec.getOutputFormat(bufferIndex).snapshot()takes the samelockand runs on the video input thread once per statistics window (MediaCodecDecoderRenderer.ktline 1990) and on the diagnostics path (line 2229). The input thread therefore can block on a JNI codec call made by the output thread.Decide whether to probe under the lock, then perform the codec query outside it, then record the result under the lock.
♻️ Proposed restructure
- synchronized(lock) { - if (!configured || !queryEnabled) return - - outputFramesObserved++ - if (queryBackoffFrames > 0) { - queryBackoffFrames-- - return - } - - val shouldProbe = if (metadataObserved) { - outputFramesObserved % SAMPLE_INTERVAL_FRAMES == 0L - } else { - outputFramesObserved <= INITIAL_PROBE_FRAMES || - outputFramesObserved % SAMPLE_INTERVAL_FRAMES == 0L - } - if (!shouldProbe) return - - try { - val metadata = codec.getOutputFormat(bufferIndex) - .getByteBuffer(MediaFormat.KEY_HDR10_PLUS_INFO) - recordMetadataLocked(metadata, presentationTimeUs) - } catch (e: RuntimeException) { - queryBackoffFrames = QUERY_FAILURE_BACKOFF_FRAMES - if (metadataTracker.recordQueryFailure()) { - LimeLog.warning( - "Failed to query per-frame HDR10+ metadata: " + - "${e.javaClass.simpleName}: ${e.message}" - ) - } - } - } + synchronized(lock) { + if (!configured || !queryEnabled) return + + outputFramesObserved++ + if (queryBackoffFrames > 0) { + queryBackoffFrames-- + return + } + + val shouldProbe = if (metadataObserved) { + outputFramesObserved % SAMPLE_INTERVAL_FRAMES == 0L + } else { + outputFramesObserved <= INITIAL_PROBE_FRAMES || + outputFramesObserved % SAMPLE_INTERVAL_FRAMES == 0L + } + if (!shouldProbe) return + } + + val metadata = try { + codec.getOutputFormat(bufferIndex) + .getByteBuffer(MediaFormat.KEY_HDR10_PLUS_INFO) + } catch (e: RuntimeException) { + synchronized(lock) { + queryBackoffFrames = QUERY_FAILURE_BACKOFF_FRAMES + if (metadataTracker.recordQueryFailure()) { + LimeLog.warning( + "Failed to query per-frame HDR10+ metadata: " + + "${e.javaClass.simpleName}: ${e.message}" + ) + } + } + return + } + + synchronized(lock) { + if (!configured || !queryEnabled) return + recordMetadataLocked(metadata, presentationTimeUs) + }🤖 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 `@app/src/main/java/com/limelight/binding/video/Hdr10PlusOutputObserver.kt` around lines 84 - 114, Restructure observeOutput so the synchronized(lock) block only validates state, updates probe counters, and decides whether to query; release the lock before calling codec.getOutputFormat(bufferIndex). After the query completes, reacquire lock to invoke recordMetadataLocked with the result, while preserving queryBackoffFrames updates and metadataTracker.recordQueryFailure() behavior for failures.app/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the sampling cadence and the query backoff.
The tests cover the stream-state machine only.
Hdr10PlusOutputObserver.observeOutputalso implements theINITIAL_PROBE_FRAMES/SAMPLE_INTERVAL_FRAMEScadence and theQUERY_FAILURE_BACKOFF_FRAMESrecovery, which the PR objectives list as new behavior. Those paths need aMediaCodecinstance, so no unit test can reach them today.Extract the probe decision into a pure function in
Hdr10PlusOutputObserver.ktand test it directly.♻️ Proposed seam and test
Add the seam in
app/src/main/java/com/limelight/binding/video/Hdr10PlusOutputObserver.kt:internal object Hdr10PlusProbeSchedule { const val INITIAL_PROBE_FRAMES = 120L const val SAMPLE_INTERVAL_FRAMES = 60L fun shouldProbe(frameIndex: Long, metadataObserved: Boolean): Boolean = if (metadataObserved) { frameIndex % SAMPLE_INTERVAL_FRAMES == 0L } else { frameIndex <= INITIAL_PROBE_FRAMES || frameIndex % SAMPLE_INTERVAL_FRAMES == 0L } }Then add tests here:
`@Test` fun initialProbeWindowSamplesEveryFrame() { assertTrue(Hdr10PlusProbeSchedule.shouldProbe(1, false)) assertTrue(Hdr10PlusProbeSchedule.shouldProbe(120, false)) assertFalse(Hdr10PlusProbeSchedule.shouldProbe(121, false)) } `@Test` fun observedMetadataSamplesAtInterval() { assertFalse(Hdr10PlusProbeSchedule.shouldProbe(59, true)) assertTrue(Hdr10PlusProbeSchedule.shouldProbe(60, true)) }🤖 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 `@app/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt` at line 9, Add the pure internal Hdr10PlusProbeSchedule.shouldProbe seam in Hdr10PlusOutputObserver.kt, preserving the initial probe-window and recurring sample-interval behavior; update observeOutput to use it. Extend Hdr10PlusOutputObserverTest with boundary tests for frames 1, 120, 121, 59, and 60, covering both metadata-unobserved and metadata-observed states.
🤖 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 `@app/src/main/java/com/limelight/binding/video/MediaCodecDecoderRenderer.kt`:
- Around line 104-109: Keep the pre-setup HDR10+ eligibility result in
hdrProfileSelector for getCapabilities(), but recompute it in
initializeDecoder() using the negotiated decoder width, height, frame rate
(including chosenFrameRate), and full-range values that setup() applies. Ensure
decoder configuration uses this active-stream eligibility instead of the
preference-based result.
---
Nitpick comments:
In `@app/src/main/java/com/limelight/binding/video/Hdr10PlusOutputObserver.kt`:
- Around line 84-114: Restructure observeOutput so the synchronized(lock) block
only validates state, updates probe counters, and decides whether to query;
release the lock before calling codec.getOutputFormat(bufferIndex). After the
query completes, reacquire lock to invoke recordMetadataLocked with the result,
while preserving queryBackoffFrames updates and
metadataTracker.recordQueryFailure() behavior for failures.
In
`@app/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt`:
- Line 9: Add the pure internal Hdr10PlusProbeSchedule.shouldProbe seam in
Hdr10PlusOutputObserver.kt, preserving the initial probe-window and recurring
sample-interval behavior; update observeOutput to use it. Extend
Hdr10PlusOutputObserverTest with boundary tests for frames 1, 120, 121, 59, and
60, covering both metadata-unobserved and metadata-observed states.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ea729c2e-a377-45ed-b09f-9087cecef2a8
📒 Files selected for processing (5)
app/src/main/java/com/limelight/binding/video/Hdr10PlusOutputObserver.ktapp/src/main/java/com/limelight/binding/video/HdrDecoderProfileSelector.ktapp/src/main/java/com/limelight/binding/video/MediaCodecDecoderRenderer.ktapp/src/main/java/com/limelight/binding/video/StableOutputFormatTracker.ktapp/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt (1)
43-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the
observeOutput()path in the gate tests.These tests assert observer state and use direct
recordMetadata()injection. They never callobserveOutput(). A regression in the runtime query gate,MediaCodec.getOutputFormat()call, or observation-epoch check could therefore pass. Add a codec test double and verify that disabled and fallback states perform no query. Also verify that a result from an old epoch is discarded after a host toggle or codec restart.Also applies to: 58-77, 79-93
🤖 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 `@app/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt` around lines 43 - 56, Extend the Hdr10PlusOutputObserver gate tests around explicit host disable and fallback states to drive the observeOutput() path with a MediaCodec test double, asserting disabled/fallback states make no getOutputFormat() query. Add coverage that results produced before a host toggle or codec restart are discarded after the observation epoch changes, while preserving the existing state assertions.
🤖 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
`@app/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt`:
- Around line 43-56: Extend the Hdr10PlusOutputObserver gate tests around
explicit host disable and fallback states to drive the observeOutput() path with
a MediaCodec test double, asserting disabled/fallback states make no
getOutputFormat() query. Add coverage that results produced before a host toggle
or codec restart are discarded after the observation epoch changes, while
preserving the existing state assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b740b6e-e223-4488-b7e1-77e424814429
📒 Files selected for processing (8)
app/src/main/java/com/limelight/ConnectionCallbackHandler.ktapp/src/main/java/com/limelight/DisplayModeManager.ktapp/src/main/java/com/limelight/Game.ktapp/src/main/java/com/limelight/binding/video/Hdr10PlusMetadataTracker.ktapp/src/main/java/com/limelight/binding/video/Hdr10PlusOutputObserver.ktapp/src/main/java/com/limelight/binding/video/HdrDecoderProfileSelector.ktapp/src/main/java/com/limelight/binding/video/MediaCodecDecoderRenderer.ktapp/src/test/java/com/limelight/binding/video/Hdr10PlusOutputObserverTest.kt
🚧 Files skipped from review as they are similar to previous changes (7)
- app/src/main/java/com/limelight/ConnectionCallbackHandler.kt
- app/src/main/java/com/limelight/DisplayModeManager.kt
- app/src/main/java/com/limelight/binding/video/Hdr10PlusMetadataTracker.kt
- app/src/main/java/com/limelight/binding/video/Hdr10PlusOutputObserver.kt
- app/src/main/java/com/limelight/binding/video/HdrDecoderProfileSelector.kt
- app/src/main/java/com/limelight/Game.kt
- app/src/main/java/com/limelight/binding/video/MediaCodecDecoderRenderer.kt
改了啥呀
HDR10+串流模式,并在设置页提示需要在 Sunshine 开启 Dynamic Metadata Analysis;普通 HDR10 不再偷偷请求 HDR10+。3集中映射为主机协议的 HDR10/PQ 值1,避免把客户端私有值泄漏给 common-c、JNI 或 framegen。qti-ext-dec-picture-order,规避其与 output fence 组合后丢失逐帧 HDR10+ metadata;静态 HDR10/HLG 保留原低延迟策略。HDR10+ (metadata observed),不让解码器看到 metadata 这只小杂鱼冒充显示链已经接受。代码边界
HdrModePolicy:客户端 HDR 模式分类、HDR10+ 请求条件和协议映射。QualcommLowLatencyPolicy:HDR10+ 模式下的 picture-order 策略。HdrDecoderProfileSelector:Profile 映射、能力查询和实际格式预检。Hdr10PlusOutputObserver:codec epoch、逐帧 metadata 采样和一致快照。HdrCapabilityHelper:当前模式与显示器全局 HDR 能力的统一解析。MediaCodecDecoderRenderer只保留 MediaCodec 生命周期、Profile fallback、output-buffer 时序和 common-c capability 接线。为啥要改
之前 HDR10 会在设备支持时自动尝试 HDR10+,设置语义不够明确;本地模式值也没有独立于主机协议。真机 A/B 还确认了 SM8750 的 picture-order 与 output fence 组合会让
KEY_HDR10_PLUS_INFO消失,即使解码器明确公布 HDR10+ Profile。这次把“用户明确选择”“设备能力”“Profile 配置成功”“解码器输出观察到 metadata”四层状态拆开。能力支持不等于已激活,观察到 metadata 也不等于厂商显示合成器接受,覆盖层和诊断现在会如实说话啦。
平台边界
c2.mtk.*与当前激进低延迟参数的组合仍需要 MTK 真机 CTS A/B,本 PR 不加入未经验证的私有参数。验证
.\gradlew.bat :app:testNonRootDebugUnitTest :app:testRootDebugUnitTest :app:assembleNonRootDebug --console=plain.\gradlew.bat :app:lintNonRootDebug --console=plaingit diff --check以上全部通过。lint 仅输出项目既有的 AGP/Gradle 弃用警告。
Summary by CodeRabbit
New Features
Bug Fixes
Localization