Repository navigation
feat(tv): keep several Remote Control controllers connected to one TV - #653
Conversation
Emulators sit behind NAT that mDNS can't cross, so a phone emulator can't discover a TV emulator or an Apple TV simulator, and there was no way to see what a TV advertises. Debug builds get two more PlaybackDebugReceiver actions: - CAST_STATUS (TV): the live Remote Control port, name and TXT record. - CAST_TARGET (phone): adds or replaces a TV target from a host, port, name and TXT string resolved on the host; `--ez clear true` removes them. Injected targets merge with real discovery, survive a restart of it, and win over an mDNS record with the same device id. The browser's TXT parsing moves into SiloCastNsdBrowser.targetFromRecord so the hook and discovery read records the same way, and the advertiser keeps the record it registered so the hook reports exactly that. Behavior of release builds is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Apple TVs now keep several Remote Control controllers connected and say why they close a session; Android phones treated every close as "someone took over", forgot the TV, and replaced other profiles' titles without asking. This brings the phone and the shared wire format level with silo-apple's SiloControl contract. Every addition is optional and feature-gated, so older peers see today's behavior. Wire (shared): - hello: serverIdentity, features, passive, clientKind; `presence` message; optional `close` reason, decoded leniently as Apple does. - handoff_offer: serverIdentity, resumeToken, replaceConfirmed, replacingContentId. handoff_ready: resumeToken. handoff_cancel: nowPlaying. - Feature, close-reason, cancel-reason and client-kind names, matching Apple's strings. - TVs advertising serverIdentity match an active server on the same deployment at another address. Phone: - Lists close_reason, resume_token and replace_confirmation; a silent auto-resume connects passive and sends presence once the user engages (remote opened, a command, or a title sent). - Close policy: an older TV's close still forgets it. A multi-controller TV's timeout reconnects; tv_disconnected forgets the TV; other reasons end the session but keep the target, and the remote keeps the TV's error. - Sends its profile name, its verified server identity, and the resume token the TV issued it; the token lives in memory per TV, server and profile. - "Replace what's playing?" on the remote, for a TV's replace_confirmation_required and before sending a different title to a TV that is playing one. The consent names the title the user saw. - Play and pause buttons send explicit play/pause for what they show, never play_pause. A rate_limited reply no longer fails a launch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An Android TV kept one controller: every new TCP connection, even a port probe, closed the phone that was using it, so two people on a couch, or a phone and a Home Assistant integration, kept knocking each other off. Apple TVs now keep several controllers; this brings the Android TV receiver to the same contract (silo-apple SiloControlReceiverPolicy), with the pure rules in TvSiloCastReceiverPolicy. - Up to 8 controllers, each with its own heartbeat, authorization and command budget (20/s, burst 40, `rate_limited`). At the cap the TV closes, in order, a connection that never said hello, an idle cross-server controller, then the oldest passive one, never one that is handing off or in use; otherwise it refuses the newcomer with `too_many_controllers`. - A new connection from the same device replaces its older sockets. - One state tick goes to every authorized controller, through a latest-only slot per controller so a slow peer holds up no one; errors go only to the controller that caused them. - One handoff at a time: a newer offer from another controller cancels the pending one with `superseded`. A launch plays under a borrowed profile only for the controller whose handoff produced it, and reusing that profile needs the same connection or the resume token issued to it (device IDs are self-reported). Keeping it alive while its phone reconnects still follows that phone. - A handoff that would replace a title another profile is watching asks first (`replace_confirmation_required` with what is playing), when the controller can ask, and checks again just before it stops playback. - Passive controllers never put up standby; standby names the newest waiting controller and counts the rest. Disconnect on the TV closes the non-passive ones with `tv_disconnected`. - After the borrowed profile changes, every controller is re-checked; one that loses authorization is told `server_mismatch`, unless its own profile simply ended, and cross-server ones stay connected to hand off again. - A borrowed profile whose session the server stops refreshing now ends: every authorized controller gets `temporary_session_expired`, playback under it stops, and the TV falls back to its own sign-in. The token manager reports this through a new `temporarySessionExpired` flow. - Close reasons go to controllers that list `close_reason`; the TV advertises `serverIdentity` in hello and Bonjour and matches controllers on it as well as the server ID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 10
✨ Finishing Touches
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 |
|
Temporary borrowed-session expiry is a visible flow without an end-to-end recording: playback stops and the TV exits the player, while the Android Remote Control shows an expiry or server-mismatch error. The description says this flow was not run because it requires a refused refresh. Please add a recording covering Android TV and the Android phone layouts (phone, tablet, and foldable folded and open); the private evidence page should include those affected devices. Automated check: Macroscope check run agent (gpt-6-luna). Evidence was not reviewed for correctness. Posted via Macroscope — Visible change evidence |
…shed registration Review follow-ups on the debug hooks: - A resolved mDNS record could pass the "no injected target with this id" check outside the browser's lock and then overwrite an injected target set in between. Every write to the target list now holds the lock. - Clearing an injected target dropped the discovered record it had replaced until discovery found it again. The browser keeps the hidden record and restores it. - CAST_STATUS reported the requested advertisement before NsdManager confirmed it, and under the requested name. It now reports only a registered advertisement, under the name NsdManager registered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-multi-controller-tv
|
Two visible paths are not fully evidenced:
Automated check: Macroscope check run agent (gpt-6-luna). Evidence was not reviewed for correctness. Posted via Macroscope — Visible change evidence |
A server signed in before the identity contract has no verified identity on the phone. The picker then can't list a TV that advertises another address of the same deployment, and hello and the handoff offer can't name the deployment. silo-apple learns it on the server's next activation; the phone now learns it once when it starts looking for TVs (the picker or a foreground auto-resume), through ServerIdentityRepository. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-multi-controller-tv
|
Evidence updated (same link):
|
…ontroller-tv # Conflicts: # android-shared/src/androidMain/kotlin/org/siloserver/silo/common/cast/SiloCastNsdAdvertiser.kt # androidApp/src/androidMain/kotlin/org/siloserver/silo/android/cast/SiloCastController.kt
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
Review follow-ups in the multi-controller receiver: - broadcastState chose its recipients before reading the player state, so a handoff that changed the TV's profile during the read could send the new profile's state to a controller it had just de-authorized. It now chooses recipients after the read, and each controller's writer checks authorization again before sending a queued state. - A failed handoff_ready rolled back the session's current readiness. A newer offer from the same phone could own it by then, so the older attempt's failure ended the newer profile. Each attempt now rolls back only the generation it granted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
… expiry wait on writes Review follow-ups in the multi-controller receiver: - completeHandoff granted readiness for whichever profile was active. It now checks, under the receiver lock, that the active profile is the one this offer prepared, and fails the handoff otherwise. - When the server refused a borrowed profile's refresh, the TV notified every controller before stopping playback and ending the profile, so a controller that stopped reading could stall both. It now tears down first and sends the notices in the background. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
A queued state was checked against the controller's authorization before the writer waited for the connection's write lock, and wasn't tied to the profile it was read under. Each state now carries the borrowed-profile generation it was read under, and the writer checks authorization and that generation after taking the write lock, just before writing. A state read under another profile, or queued before the controller lost access, is dropped; the next tick follows within half a second. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
The remote's Subtitles menu listed tracks, eleven delay offsets and three positions. It now lists Off and the subtitle tracks only, the same as silo-apple. Delay and position stay in the TV's own player menu, which is where they are adjusted while watching. The dropdown no longer needs section headers. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
Problem
Related issue: N/A
Validation tasks: changes #331 C1–C3 (not yet run)
An Android TV keeps one Remote Control controller. Every new connection closes the phone that was using it, even a port probe. Two people on the couch, or a phone and a Home Assistant integration, keep knocking each other off. A phone on one profile can also replace a title another profile is watching without anyone being asked.
This lets several devices control one Android TV, under the same rules as the Apple TV in Silo-Server/silo-apple#856. A handoff that would replace another profile's title asks first.
Approach
TvSiloCastReceivernow holds a set of controllers instead of one session. Each controller has its own heartbeat, authorization and command budget (20 per second, burst 40, thenrate_limited). The player, the borrowed profile, the one in-flight handoff and the standby screen stay shared. The pure rules live inTvSiloCastReceiverPolicy, ported from silo-apple'sSiloControlReceiverPolicywith the same tests:too_many_controllersinstead.superseded.replace_confirmation_requiredwith what is playing, when the controller can ask. The check runs again just before playback stops.tv_disconnected.server_mismatch, unless its own profile simply ended.temporary_session_expired, playback under it stops, and the TV returns to its own sign-in.TokenManagerreports this through a newtemporarySessionExpiredflow, which emits nothing by default.serverIdentityin hello and in its mDNS record, and matches controllers on it as well as on the server ID.Stacked on #652 (phone and wire), which is stacked on #651 (debug hooks).
The TV keeps its current timing for borrowed profiles: kept while the lending phone is connected, a 2-second hand-back once it leaves while idle, and a 10-minute idle limit. The lender is now the connection that completed the handoff, or a holder of its token, not a device ID.
Validation
testDebugUnitTest: 6,120 tests pass (last run on 12ff751), including 14 new receiver-policy tests and one new identity-reuse test.lintDebugpasses for all three modules,lintVitalReleasefor both apps, and the supply-chain checks pass.replaced.too_many_controllers, while a passive controller makes room for a newcomer.rate_limited.mainwith test(cast): add adb hooks to inspect and inject Remote Control targets #651's hooks: the iPhone joining knocks the Android phone off, and standby names only the iPhone.temporary_session_expiredend to end, which needs the server to refuse a refresh. Unit tests cover it.Evidence
Evidence: https://evidence.siloserver.org/r/silo-android/pr-653/
Surface and build: Android TV emulator (1080p) on this stack, and
mainplus #651's hooks for the before captures. The phone captures show the TV's prompt and the other controller's view. Their tablet and foldable layouts are in #652's evidence.Risks
TvSiloCastReceiveraround the singleactiveSessionthat this PR replaces. Whichever lands second needs a port: with several controllers, "return control to the TV" should close the non-passive controllers withtv_disconnected, as Disconnect does here.Follow-ups:
player_not_ready; the Apple TV cancels the load.remoteStopRequestshas no replay, so a Stop right after registration can be lost.Checklist
AI Disclosure
mainand silo-apple's receiver, without running it. It reported eight findings, all in the receiver:handoff_readyleft other controllers de-authorized and could end another controller's profile.supersedednotice was sent from the new controller's read loop.🤖 Generated with Claude Code
Note
Support multiple simultaneously connected Remote Control controllers on one TV
TvSiloCastReceiverwith a controller map that admits up to 8 concurrent connections and evicts idle controllers (oldest silent, then unauthorized, then passive) when full. Adds pure decision helpers in TvSiloCastReceiverPolicy.ktRemotePlaybackIdentityManager.preparenow takes anallowReuseflag because device IDs are self-reportedtemporarySessionExpiredflow toTokenManagerso borrowed-profile session expiry stops playback and notifies authorized controllershandleOfferreuse rules andpreparecallers updated for the newallowReuseparameterMacroscope summarized ac5195a.