fix(playback): restore Safari Dolby Vision Profile 8 remux - #613
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe change adds structured Dolby Vision profile-level capabilities, preserves Dolby Vision levels from media probing through playback planning, refreshes web sessions when HDR output changes, and enables alternate-file fallback for HDR incompatibility terminals. ChangesHDR playback support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MediaScanner
participant PlaybackPlanner
participant WebClient
participant PlaybackSession
MediaScanner->>PlaybackPlanner: Provide source Dolby Vision level
WebClient->>PlaybackPlanner: Provide HDR profile-level capabilities
PlaybackPlanner->>PlaybackPlanner: Validate source and client HDR limits
PlaybackPlanner-->>PlaybackSession: Return plan or HDR terminal
PlaybackSession->>PlaybackPlanner: Replan after HDR output change
PlaybackPlanner-->>PlaybackSession: Return updated plan or refusal
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8702bdfe5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
🤖 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 `@docs/design/schemas/playback-v3/v3/decision-response.schema.json`:
- Around line 764-766: Constrain the dolby_vision_level property in the response
schema to integers from 1 through 13 by adding minimum and maximum bounds, and
ensure the field is omitted when no valid level is available.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f2acdd00-fb5c-4913-8947-98eaaae0dffd
📒 Files selected for processing (20)
docs/design/schemas/playback-v3/v3/decision-response.schema.jsondocs/design/schemas/playback-v3/v3/replan-request.schema.jsondocs/design/schemas/playback-v3/v3/start-request.schema.jsoninternal/catalogseed/service.gointernal/catalogseed/service_test.gointernal/catalogseed/types.gointernal/models/media.gointernal/playback/capabilities_v3.gointernal/playback/plan_v3.gointernal/playback/protocol_v3.gointernal/playback/protocol_v3_test.gointernal/scanner/probe.gointernal/scanner/probe_video_range_test.gointernal/scanner/scanner.gointernal/scanner/types.goweb/src/player/client-context-v3.test.tsweb/src/player/client-context-v3.tsweb/src/player/hooks/useCodecDetection.test.tsweb/src/player/hooks/useCodecDetection.tsweb/src/player/protocol-v3.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/player/client-context-v3.test.ts
- web/src/player/hooks/useCodecDetection.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dae1c3c280
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bf8a9ac1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ecac75f85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fea09a0537
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea6fdf92bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@web/src/player/components/VideoPlayer.tsx`:
- Around line 1400-1403: Update the shouldAutoPlay-disabled branch in
VideoPlayer to mark hlsStartupGuardRef as complete before returning, preventing
paused HLS playback from triggering startup timeout recovery. Add a regression
test covering paused HLS with autoplay disabled and verifying no unnecessary
recovery replan occurs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cc1a4b3c-6778-4b6d-bef8-7f32f5761393
📒 Files selected for processing (20)
docs/design/schemas/playback-v3/v3/decision-response.schema.jsondocs/design/schemas/playback-v3/v3/replan-request.schema.jsondocs/design/schemas/playback-v3/v3/start-request.schema.jsoninternal/api/handlers/playback_v3.gointernal/api/handlers/playback_v3_test.gointernal/playback/capabilities_v3.gointernal/playback/plan_v3.gointernal/playback/protocol_v3.gointernal/playback/protocol_v3_test.goweb/src/player/client-context-v3.test.tsweb/src/player/client-context-v3.tsweb/src/player/components/VideoPlayer.test.tsxweb/src/player/components/VideoPlayer.tsxweb/src/player/components/WatchPage.test.tsweb/src/player/components/WatchPage.tsxweb/src/player/hooks/useCodecDetection.test.tsweb/src/player/hooks/useCodecDetection.tsweb/src/player/hooks/usePlaybackSession.test.tsweb/src/player/hooks/usePlaybackSession.tsweb/src/player/protocol-v3.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- internal/playback/capabilities_v3.go
- internal/playback/protocol_v3_test.go
- docs/design/schemas/playback-v3/v3/decision-response.schema.json
- web/src/player/hooks/useCodecDetection.test.ts
- docs/design/schemas/playback-v3/v3/start-request.schema.json
- web/src/player/client-context-v3.test.ts
- web/src/player/protocol-v3.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ce5a490b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bd376370c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0249e3776
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/playback/protocol_v3.go`:
- Around line 831-832: Reject non-zero FailureV3 values for intent-only
operations track_change, quality_change, and output_change in
ReplanRequestV3.Validate, while preserving the documented seek_reanchor
exception. Update docs/design/schemas/playback-v3/v3/replan-request.schema.json
to forbid failure for those operations, and extend
internal/playback/contract/contract_test.go with negative wire bodies for each
operation asserting both schema and runtime rejection.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7800a972-6ea4-4712-bc43-75cc1422550f
📒 Files selected for processing (18)
cmd/playbackfixtures/main.godocs/architecture/playback-protocol-v3.mddocs/design/schemas/playback-v3/v3/replan-request.schema.jsoninternal/api/handlers/playback_v3.gointernal/api/handlers/playback_v3_test.gointernal/playback/contract/contract_test.gointernal/playback/protocol_v3.gointernal/playback/protocol_v3_test.gointernal/playback/testdata/protocol_v3/conformance_matrix.jsonweb/src/player/client-context-v3.test.tsweb/src/player/client-context-v3.tsweb/src/player/components/VideoPlayer.test.tsxweb/src/player/components/VideoPlayer.tsxweb/src/player/hooks/useCodecDetection.test.tsweb/src/player/hooks/useCodecDetection.tsweb/src/player/hooks/usePlaybackSession.test.tsweb/src/player/hooks/usePlaybackSession.tsweb/src/player/protocol-v3.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- web/src/player/hooks/usePlaybackSession.test.ts
- internal/api/handlers/playback_v3_test.go
- internal/api/handlers/playback_v3.go
- web/src/player/components/VideoPlayer.tsx
- web/src/player/client-context-v3.ts
- web/src/player/client-context-v3.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 530ff695c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36632b2bb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5694c1f214
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dolby_vision_profiles: dolbyVisionProfiles, | ||
| dolby_vision_profile_levels: dolbyVisionProfiles.map((profile) => { |
There was a problem hiding this comment.
When a newer cached web bundle reaches a pre-change backend during a rolling deployment, Go ignores the unknown dolby_vision_profile_levels field but still honors this legacy dolby_vision_profiles list; the base planner matches only the profile number. The exact Profile 8.1/Level 6 probe therefore becomes an unrestricted Profile 8 claim, allowing that backend to remux unverified Profile 8.2/8.4 or Level >6 sources. Fresh evidence beyond the resolved per-profile bounds is this cross-version interpretation; advertise planner support through a distinct capability before sending the legacy profile claim, or make the payload backward-safe.
AGENTS.md reference: AGENTS.md:L112-L119
Useful? React with 👍 / 👎.
| if (!serverFeaturesRef.current.includes(FEATURE_OUTPUT_CHANGE_V3)) { | ||
| return; |
There was a problem hiding this comment.
When a current web bundle talks to a backend that predates output_change_v1, this return occurs after the refreshed capability key was marked active, so moving from HDR to SDR neither replans nor retries. Such a backend already understands the legacy Dolby Vision profile claim and can have selected a Dolby Vision route during the initial HDR start, leaving that incompatible route active indefinitely after the output changes. Use a compatible replacement-start fallback that preserves playback state instead of consuming the refresh without applying it.
AGENTS.md reference: AGENTS.md:L12-L14
Useful? React with 👍 / 👎.
| pendingReplanRef.current = { | ||
| options, | ||
| loadSequence: loadSequenceRef.current, | ||
| retireSessionOnRefusal, | ||
| resolve, |
There was a problem hiding this comment.
With an ordinary replan in flight, an HDR-to-SDR change can first queue an output_change with retireSessionOnRefusal=true; if a track, quality, or failure request arrives before dispatch, this assignment replaces that entry using the new call's default false. The first request may adopt a plan using the old output evidence, while the replacement request uses fresh SDR evidence and returns terminal without retiring the now-incompatible plan. Fresh evidence after the prior queued-retirement fix is this reverse ordering; preserve the existing retirement requirement when replacing the pending entry.
AGENTS.md reference: AGENTS.md:L12-L14
Useful? React with 👍 / 👎.
…C in remuxes (#617) * fix(playback): claim Safari DV/HDR10 decode evidence and preserve dvvC Follow-up to #613 for #609: Safari 26 reports dynamic-range: standard even on an XDR display, and answers canPlayType "probably" only for dvh1/hvc1 sample entries, never dvhe/hev1. The web probe gated every structured HDR claim on that media query and probed only dvhe, so the planner still saw empty hdr_details and terminated with hdr_transcode_unsupported. - Run the Dolby Vision and HDR10 shape probes unconditionally; the dynamic-range query survives only as the best-effort hdr output boolean. - Probe dvh1+dvhe per DV profile and hvc1+hev1 for the exact HDR10 Media Capabilities shape; either definitive answer earns the claim. - Tag preserved-DV remuxes dvh1 with -strict unofficial: FFmpeg omits the dvvC configuration record under either tag without it, so the previous dvhe output carried no DV signaling at all. - Scale the screen-derived max_resolution by devicePixelRatio so 2x panels stop advertising 1080p. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): align DV/HDR10 claims with the sample entries the remux emits Review follow-ups on #617 (Codex P1s): - Probe only dvh1: a browser answering "probably" solely for dvhe has given no evidence for the dvh1-tagged file the preserve remux delivers, so that answer no longer earns a Dolby Vision claim; such browsers keep the validated HDR10 fallback. - Label the explicit v3 HDR10 strip output hvc1 so the file matches the hvc1 evidence the web probe accepts; legacy/auto strips keep hev1. - Cover preserve tagging for profiles 5 and 8, and exercise the mode-to-argument mapping through StartRemuxWithDVMode instead of passing the tag flag directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): name the subtitle when burn-in alone forces a refused transcode Selecting a PGS/VOBSUB track on a client without a bitmap renderer forces a burn-in transcode. When that transcode cannot run, the terminal blamed the HDR pipeline (hdr_transcode_unsupported) or the 4K policy (no_alternate_version) — problems that were not blocking playback, since deselecting the subtitle restores the previous route. - When the burn requirement is the sole trigger of the adaptation, the HDR and 4K-policy terminals emit subtitle_conversion_unsupported with a message naming the burn requirement and the actual blocker. A range the client genuinely cannot take keeps the HDR terminal. - describePlanTerminal passes the server's subtitle message through instead of flattening every subtitle_* reason to one generic sentence. - The web player toasts the refusal when it rolls the subtitle selection back; previously the only surface was the quality menu, which a user who just picked a subtitle has no reason to open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playback): require decode evidence for the exact hvc1 entry and gate the disabled-transcode subtitle terminal Review follow-ups on #617: - The HDR10 probe accepts only hvc1.2.4.L153.B0: the explicit v3 strip remux labels its output hvc1, so an hev1-only decodingInfo answer is evidence for bytes Silo never sends and earns no claim (Codex P1). - The disabled-transcode branch blames the subtitle only when the burn requirement was the sole adaptation trigger; other causes keep transcoding_disabled. In practice a bitmap selection without transcode terminals in the subtitle policy before this branch, but the guard keeps the sole-cause invariant if that ordering ever changes (CodeRabbit / Codex P2). - Protocol doc scopes the hvc1 labeling to the explicit v3 strip path; legacy/auto strips keep hev1 (CodeRabbit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ality Narrow proxy selection by capability *before* selection rather than rejecting a single round-robin pick afterwards. Abandoning the pool on one mismatch meant a capable proxy with free capacity sat unused while playback either ran ffmpeg on the API node or, with playback.local_transcode_fallback disabled, was refused outright — the exact api/proxy split this branch targets, during exactly the rolling ffmpeg upgrade the capability check exists for. PlanSessionWith now applies its eligibility predicate to the proxy on proxy-only plans (the proxy is the executor there), mirroring how HLS filters transcode nodes, and the planner grows ProxyNodeURLs to match TranscodeNodeURLs. Direct play still skips the probe: it copies bytes and needs no recipe. Every committed route now records transport locality, not just the identity-proxy one. A session replanned from a proxy onto the integrated transcoder previously kept a stale remote-transport mark, and the widened idle grace it grants would hold that session's stream and transcode slots for five minutes after the local stream disconnected without an explicit stop. The remote HLS route sets it too — it also hands the client an absolute proxy URL that never reaches this server. The proxy's CORS config exposed no response headers, so cross-origin JavaScript could send the If-Range/Range request headers it already allows but never read the ETag, Accept-Ranges or Content-Range needed to build them. direct_stream_resume_v1 silently degraded to a full restart whenever the proxy was on a different origin than the web app, which is the normal deployment. Also regenerates internal/playback/testdata/protocol_v3 and the schema fixtures, which were stale for output_change_v1 since #613/#617 and failed CI on every branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(playback): route v3 direct play and remux through proxy nodes
Protocol v3 consulted the node planner only for the HLS deliveries, so
`original_http` and `server_remux_progressive` sessions returned an
API-local `/stream/{session_id}` URL and the API node served the bytes —
ServeDirectPlay for direct play, a locally spawned ffmpeg for the remux.
An operator running dedicated proxy nodes still saw all of that egress on
the API node.
The capability already existed: the proxy implements /stream/direct and
/stream/remux, and the Jellyfin-compat transport already plans a proxy for
exactly these two methods. Native v3 was the only surface skipping it, so
Jellyfin clients routed correctly on a deployment where Silo's own clients
did not. This wires the same shape into the v3 identity transport rather
than inventing a second selection path.
The proxy serves from the stream token alone, so the token now carries the
media path, the file's Dolby Vision profile (a P7 remux must strip the
dangling RPU) and the audio-only flag (which picks audio/mp4 over
video/mp4, the MIME the plan promised). RecipeCard models none of the
three; a missing claim would not fail loudly, it would serve a subtly
different stream than the plan promised.
Two related fixes:
- Proxy direct play served via http.ServeFile, which sets no strong ETag.
direct_stream_resume_v1 depends on the ETag ServeDirectPlay sets before
ServeContent, so routing direct play to a proxy without this would have
silently broken resumable direct streams: If-Range never validates and a
resumed range restarts at 200. The proxy now uses the same serve path.
- playback.local_transcode_fallback was only checked in the HLS branch, so
a progressive remux that converts audio still spawned ffmpeg locally on
an API-only node with the setting disabled. Identity deliveries now
honor the gate too — direct play still falls back locally, since moving
bytes is not transcode work and single-node deployments must keep
working.
Falling back to the API-local path when no proxy is eligible preserves
single-node behavior, and a planner reservation is released whenever the
session does not actually reach a proxy.
Closes #619
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(playback): validate proxy recipes and keep proxy sessions alive
Addresses three P1 findings on the proxy-transport change.
Proxies do run ffmpeg — /stream/remux converts audio and strips Dolby
Vision RPUs — but they exposed no capability endpoint, so unlike the HLS
offload path nothing checked that the selected proxy could execute the
transformations a plan froze. A pool whose proxies carry a different
ffmpeg build (rolling upgrade, custom image) would fail at stream time: a
missing aac encoder 500s, a missing dovi_rpu filter is refused outright by
the remux itself. Proxies now serve /hw-capabilities in the same shape and
at the same path as a transcode node, and identity planning validates the
frozen recipe against the selected proxy, falling back to a node that can
do the work. A proxy that does not answer is treated as incapable rather
than assumed good: an older proxy predating the endpoint is exactly the
mismatched build the check exists to catch. Direct play copies bytes and
needs no recipe, so it skips the probe entirely.
meteredResponseWriter implemented neither Unwrap nor SetWriteDeadline, so
RollingDeadlineWriter could not install its stall deadline on any proxy
stream. With the standalone proxy running WriteTimeout 0 there was no
server-level guard behind it, so a client that stopped reading without
closing its connection would block a write forever, holding the session,
the file, the goroutine and the connection.
A proxy-served session never produces a transport request on the API node,
so activeTransportCount — what protects a local stream from the idle
reaper — stays zero and a heartbeat gap longer than the active grace would
reap a healthy stream, after which progress, stop and replan all fail with
session-not-found while bytes still flow. Sessions are now marked as
remotely transported, which widens their idle windows rather than granting
immunity: this manager has no absolute session lifetime, so unconditional
immunity would leak a session forever when a client disappears without
stopping. The mark is always set on commit, so a re-plan that moves a
session back onto the API clears a stale one.
Also adopts the exported transformation constants in the tests and covers
the effective-recipe bitrate branch, per review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(playback): pick capable sibling proxies and refresh transport locality
Narrow proxy selection by capability *before* selection rather than
rejecting a single round-robin pick afterwards. Abandoning the pool on one
mismatch meant a capable proxy with free capacity sat unused while
playback either ran ffmpeg on the API node or, with
playback.local_transcode_fallback disabled, was refused outright — the
exact api/proxy split this branch targets, during exactly the rolling
ffmpeg upgrade the capability check exists for. PlanSessionWith now
applies its eligibility predicate to the proxy on proxy-only plans (the
proxy is the executor there), mirroring how HLS filters transcode nodes,
and the planner grows ProxyNodeURLs to match TranscodeNodeURLs. Direct
play still skips the probe: it copies bytes and needs no recipe.
Every committed route now records transport locality, not just the
identity-proxy one. A session replanned from a proxy onto the integrated
transcoder previously kept a stale remote-transport mark, and the widened
idle grace it grants would hold that session's stream and transcode slots
for five minutes after the local stream disconnected without an explicit
stop. The remote HLS route sets it too — it also hands the client an
absolute proxy URL that never reaches this server.
The proxy's CORS config exposed no response headers, so cross-origin
JavaScript could send the If-Range/Range request headers it already allows
but never read the ETag, Accept-Ranges or Content-Range needed to build
them. direct_stream_resume_v1 silently degraded to a full restart whenever
the proxy was on a different origin than the web app, which is the normal
deployment.
Also regenerates internal/playback/testdata/protocol_v3 and the schema
fixtures, which were stale for output_change_v1 since #613/#617 and failed
CI on every branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(playback): restore the alternate-version fallback for burn-in refusals
#617 renamed the terminal a burn-in-forced adaptation reports: when the
subtitle burn requirement is the sole trigger, an HDR source that cannot
be re-encoded now returns subtitle_conversion_unsupported instead of
hdr_transcode_unsupported, so the refusal names the thing the viewer can
actually act on.
terminalAllowsAlternateFileV3 was not updated to match, and it gates the
alternate-version retry on the old reason strings. That silently retired
the fallback for exactly the case its own comment describes — a bitmap
subtitle needing burn-in that an HDR source cannot support while an SDR
alternate can. Playback was refused outright instead of switching to the
version that can serve it.
Adds the new reason to the gate and covers it directly, so a future
rename of a refusal reason fails on the gate rather than only on the
end-to-end replan test.
Also drops debug instrumentation that was committed by mistake in
TestHandleReplanPlaybackV3BitmapSubtitleFallsBackFromHDRToSDRVersion; the
assertion is back to its original form and now passes on the merits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Upstream Silo-Server#613 renamed the HDR burn-in terminal to subtitle_conversion_unsupported but never added it to the alternate-version fallback allowlist, so a bitmap subtitle selected against an HDR source could no longer fall back to an SDR alternate version. Add the reason to terminalAllowsAlternateFileV3 and pin the behavior with handler tests. Also keep the branch's stale route-event no-op while preserving the 403 for terminal-start claims on unknown attempts, and add web regression tests that a manually selected SRT stays selected once the server adopts the replan.
1 participant
Problem
The web capability probe only sent the legacy
hdrboolean. Playback V3 validates HDR and Dolby Vision against structuredhdr_details, so Safari could decode the issue's Dolby Vision Profile 8.1 source but the server rejected it as requiring video adaptation. With 4K transcoding disabled, that surfaced as the misleading terminal reasonno_alternate_version.Fixes #609.
Approach
dvhe.05.06anddvhe.08.06) throughHTMLMediaElement.canPlayType.hdr_transcode_unsupportedover the 4K policy terminal when an HDR-incompatible client truly requires video encoding.The probe deliberately does not use MediaSource support: this route plays the server's progressive MP4 remux directly through a media element.
Runtime evidence
Validated against the shared development deployment with a real single-version 2160p HEVC Main10 Dolby Vision Profile 8 / BL compatibility ID 1 source:
adaptation_unavailable / no_alternate_version.playable / server_remux_progressive / container_normalization.dolby_vision_profiles: [8]plus max Level 6, kept HDR10/HLG false, and cleared the DV claim from HLS. The real source returned201 playable / server_remux_progressiveand reachedHTMLMediaElement.readyState = 4with no media error. On exact PR headb0249e377630f60e4ccb8c6880053cabb3943a16, a fresh request with HEVC and Profile 8.1 advertised only for progressive delivery again returnedplayable / server_remux_progressivewith native Dolby Vision preserved. A liveoutput_changereplan with the same capabilities kept the same progressive route and plan-attempt key. A second live Profile 7 source that initially selected its 1080p SDR alternate stayed on requested file236941once this exact HDR10 evidence was present, usingserver_remux_progressivewithserver_dv7_to_hdr10.hdr_transcode_unsupported.Validation
go test ./internal/playback ./internal/scanner ./internal/catalogseedgo test ./internal/api/handlerspnpm --dir web exec vitest run src/player/client-context-v3.test.ts src/player/hooks/useCodecDetection.test.ts— 18 tests passedpnpm --dir web exec vitest run src/player/hooks/usePlaybackSession.test.ts src/player/hooks/useCodecDetection.test.ts src/player/client-context-v3.test.ts src/player/components/WatchPage.test.ts src/player/components/VideoPlayer.test.tsx— 63 tests passedPATH="$DEV_CACHE_ROOT/bin:$PATH" golangci-lint run --new-from-merge-base="origin/main" ./...— 0 issuespnpm --dir web run lint— 0 errors (151 inherited warnings)pnpm --dir web run format:checkpnpm --dir web run buildmake verify-local-pathsFull local gates also exposed existing baseline failures outside this change:
make lint: 300 inherited full-tree findings; the CI-equivalent changed-lines lint above is clean.make test: two unchanged macOS-only Jellycompat process-lock tests fail because the current process identity token is empty. The focused playback and API handler suites pass; CI runs the Go suite on Linux.Risk
The capability claim is conservative: exact Dolby Vision profiles are only published when both the media element and active HDR output attest support, are bounded to the probed level and Profile 8.1 base-layer shape, and are scoped away from original/HLS delivery. Browsers that do not recognize the sample entry continue to omit Dolby Vision and follow the existing adaptation path.
AI Disclosure
Summary by CodeRabbit
New Features
Bug Fixes
Tests