Skip to content

fix(native): the media fallback returns to the recovery position and keeps a pause - #11

Merged
Quick104 merged 4 commits into
mainfrom
fix/media-fallback-keeps-position
Sep 29, 2026
Merged

Quick104 merged 4 commits into
mainfrom
fix/media-fallback-keeps-position

Conversation

@Quick104

@Quick104 Quick104 commented Sep 29, 2026 •

Copy link
Copy Markdown

Summary

After #10, a paused title whose player item dies still restarts from where the session was first opened, and still starts playing, when AVPlayer refuses the recovery item's master. Both problems are in fallBackToMediaPlaylist: it reloads at the first mount's start position and calls play() unconditionally. This PR ports the two upstream fixes for that path.

Part of Silo-Server/silo-apple#549 (the "from the start" half).

What changed

Test plan

  • Device / OS: macOS 27 (swift build, swift test). Not run on an Apple TV. There is no hook to force a display rejection, so the fallback call site is covered by the same source-reading test upstream uses.
  • Source media: none. The field case is a Silo diagnostics report from an Apple TV 4K (2nd gen) on tvOS 26.6: HEVC Dolby Vision Profile 8.1 with EAC3 Atmos, loopback HLS, engine a02975e:
    11:26:49  #4 paused, t+1691.46s
    11:31:54  #4 failedToPlayToEndTime AVFoundationErrorDomain/-11868 'Cannot open'
    11:31:57  #93 item death at 1690.28s; reloading item through stage-2 recovery (pause guard bypassed)
    11:31:57  #5 item.status=failed -11868 (CoreMediaErrorDomain/-17223)
    11:31:57  #5 startup .failed is a master rejection; signalling engine for media fallback
    11:31:57  AVPlayer rejected the master; falling back to media playlist (no CC/subtitle renditions) at 0.00s
    11:32:00  #6 timeControlStatus=playing
    
  • Result: swift test passes locally: 3,395 Swift Testing tests, 636 XCTest tests (1 skipped), and the 43 authorization tests run separately, as in CI, on adff22aa. New tests: the placement tests from fix(native): the media fallback comes back where the refused item was placed (#98) superuser404notfound/AetherEngine#621 (a recovery swap moves the recorded placement, a mount with no start position is placed at the head, a live rejoin records none), pure-decision tests for the fallback's resume rule (engine Play, AVKit Play, a pause before the refusal, an engine pause inside the margin, a command after the refusal), a host test that engine Play and Pause drive the verdict, and a call-site check that the verdict is read before the swap.

Known gap: a pause from AVKit, Control Center or PiP less than a second before the refusal cannot be told apart from the refused item's own stop, so that fallback plays. #10 has the same margin. A pause through the engine is handled.

Not addressed: the refusal still sets panelRefusedHDRMaster for the process (superuser404notfound#588), so HDR titles after it play media-direct until the app returns from the background.

Checklist

  • CHANGELOG.md updated
  • Commit messages follow Conventional Commits (feat(...), fix(...), chore(...))
  • The fix lives in the engine, not in a host-side workaround
  • Public API changes are intentional and documented (no public API change)

AI disclosure

🤖 Generated with Claude Code

brandomoore and others added 2 commits September 29, 2026 09:53
… placed (superuser404notfound#98)

The superuser404notfound#98 media fallback replayed the start position of the session's first
mount. The superuser404notfound#93/superuser404notfound#65 stage-2 recovery swaps a fresh item in at the position
playback held, so when that recovery item was refused at startup, the
fallback rewound the session to wherever it had first been loaded.

Field log, Apple TV 4K 3rd gen, tvOS 27.0, HDR10+ HEVC Matroska opened with
a resume at 1844 s: paused at 2099.69 s, the item died behind the
screensaver, the recovery item was refused with -11868, and playback
resumed at 1834.79 s. A title started from its beginning resumes at its
first frame.

NativeAVPlayerHost now records where each mount places its item, in-place
swaps included, and the fallback reads that instead of the engine's
first-mount copy. The first mount records the same value as before, so a
fallback of a first mount is unchanged.

(cherry picked from commit f36fe40)
…ng (superuser404notfound#98)

When a dead item's recovery reload was refused (-11868), fallBackToMediaPlaylist
swapped in the media playlist and called play() unconditionally, so a title
paused behind the tvOS screensaver started itself. The item-death reload now
keeps the viewer's pause (clearing the host's play intent), and the fallback
reads that same intent before it plays.

Ports the fallback half of upstream superuser404notfound#623; the
reload half is covered by this fork's own item-death change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kody-ai

kody-ai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

Access your configuration settings here.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T14:33:26.950994Z adff22a New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 17ed95da-e10e-41c5-a802-719169b90841

📥 Commits

Reviewing files that changed from the base of the PR and between 587d8cf and 5e7cec9.

📒 Files selected for processing (6)
  • CHANGELOG.md
  • Sources/AetherEngine/AetherEngine+Loading.swift
  • Sources/AetherEngine/AetherEngine.swift
  • Sources/AetherEngine/Native/NativeAVPlayerHost.swift
  • Tests/AetherEngineTests/MasterFallbackDecisionTests.swift
  • docs/formats.md
💤 Files with no reviewable changes (1)
  • Sources/AetherEngine/AetherEngine+Loading.swift

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

VOD media fallback now resumes at the rejected item’s mounted position and calls play() only when the host’s transport intent indicates playback. The native host records each item’s mounted start position.

Changes

Master fallback recovery

Layer / File(s) Summary
Track the mounted item position
Sources/AetherEngine/Native/NativeAVPlayerHost.swift, Sources/AetherEngine/AetherEngine+Loading.swift, Sources/AetherEngine/AetherEngine.swift
The native host records the mounted start position on each load. The engine removes the prior saved start-position state.
Apply position and transport intent
Sources/AetherEngine/AetherEngine.swift, Tests/AetherEngineTests/MasterFallbackDecisionTests.swift, docs/formats.md, CHANGELOG.md
VOD fallback uses the rejected item’s mounted position, defaulting to zero, and calls play() only when transport intent is playing. Tests cover position tracking and fallback conditions. The documentation and changelog describe the updated fallback.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: brandomoore

Merge Risk: ⚪ Minimal · up to 5e7ce

The fallback is ready for normal merge checks; device behavior has not been verified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5e7ce

The change is confined to playback recovery. It does not appear to add an externally callable control or expand access to data or privileges. Device behavior and some surrounding code were not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected change affects recovery of the current playback session rather than introducing a new caller authority, credential path, or cross-session data flow. Undocumented external integrations remain unverified.

Trust Boundaries and Controls

  • observed — Fallback retains its rejection-code, master-serving, prior-fallback, and media-URL checks before replacing the item. The new position value does not select the fallback URL or relax those checks.

Resilience and Maintainability Implications

  • inferred — Recording placement on every load and guarding callbacks by session or item identity reduce the opportunity for an earlier item’s state to drive a later recovery. This conclusion is source-based, not device-verified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies both primary changes: recovery-position resume and pause preservation.
Description check ✅ Passed The description directly explains the fallback changes, tests, limitations, and related issues.
Linked Issues check ✅ Passed The description references related issues and upstream pull requests that match the implemented changes.
Out of Scope Changes check ✅ Passed The changes remain within the stated scope of fixing native media fallback position and transport behavior, with related tests and documentation.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e7cec9c1a

ℹ️ 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".

Comment thread Sources/AetherEngine/AetherEngine.swift Outdated
Comment thread Sources/AetherEngine/AetherEngine.swift Outdated
…to resume

The fallback read only the host's play intent, which a Play from AVKit,
Control Center or PiP never sets. A viewer who started an autoplay=false
mount from the transport bar, or pressed Play from AVKit after a paused
item-death reload, got a paused fallback.

The host now decides at the refusal: the item plays on unless the viewer
paused it before the refusal (the item-death one-second margin), and only if
the engine's intent or AVPlayer's rate says it was told to play. When it stays
paused the fallback calls pause() so a latched intent cannot restart the fresh
item on readyToPlay.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kody-ai

kody-ai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

Access your configuration settings here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c4dff5109

ℹ️ 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".

Comment thread Sources/AetherEngine/Native/NativeAVPlayerHost.swift Outdated
Comment thread Sources/AetherEngine/AetherEngine.swift Outdated
Comment thread Sources/AetherEngine/Native/NativeAVPlayerHost.swift Outdated
…at the refusal

Three races in the previous verdict, raised in review:

- An engine pause less than a second before the refusal falls inside the
  failure margin, and the per-load roll flag still said the item had played,
  so the fallback resumed a paused title.
- The fallback runs a task hop after the refusal; a Play or Pause pressed in
  between was overwritten by the verdict taken earlier.
- The verdict was taken in the status KVO hop, which can run before the rate
  KVO hop that records a Play from AVKit.

The host now records when the master was refused and any engine command
since, and the engine asks for the verdict inside fallBackToMediaPlaylist,
before the swap. A roll counts only if AVPlayer reported it after the
engine's last stop, so an engine pause clears it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kody-ai

kody-ai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

Access your configuration settings here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: adff22aaa9

ℹ️ 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".

Comment thread Sources/AetherEngine/Native/NativeAVPlayerHost.swift
Comment thread Sources/AetherEngine/Native/NativeAVPlayerHost.swift
Comment thread Sources/AetherEngine/Native/NativeAVPlayerHost.swift
@Quick104
Quick104 merged commit b1e4879 into main Sep 29, 2026
9 checks passed
Quick104 added a commit to Silo-Server/silo-apple that referenced this pull request Sep 29, 2026
…item dies (#551)

Moves the AetherEngine pin to b1e4879e, which includes
Silo-Server/AetherEngine#10 (the item-death reload keeps a viewer's pause) and
Silo-Server/AetherEngine#11 (the media fallback returns to the recovery
position and plays only for a viewer who was playing).

Refs #549

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants