diff --git a/AGENTS.md b/AGENTS.md index 4aa0b2a..4e2c7a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,3 +53,9 @@ grep -rnEio '\b([a-z]{3,}iz(e|es|ed|ing|ation)|behaviors?|colors?|favorites?|hon Expect hits — most are the exemptions above. Read each one and ask whether a viewer or an operator ever sees it. + +## Version bumps + +Every version bump is a release change: update the version and changelog together, then +commit and push the complete release to GitHub. Do not leave a bumped version only in the +local working tree. diff --git a/CHANGELOG.md b/CHANGELOG.md index 64ffbfb..ba6bd21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,24 @@ +## 0.2.33 — 2026-08-08 +- General bug fixes and improvements. + +## 0.2.32 — 2026-08-08 +- General bug fixes and improvements. +- Backend improvements. + +## 0.2.31 — 2026-08-08 +- General bug fixes and improvements. + +## 0.2.30 — 2026-08-08 +- General bug fixes and improvements. + +## 0.2.29 — 2026-08-07 +- Fixed: A small bug with new seek previews. +- Fixed: Skip Intro and Outro Credits were not appearing in some cases. + +## 0.2.28 — 2026-08-07 +- Added: Skip outro credits +- Added: Themes for Halloween, Christmas and Easter. + ## 0.2.27 — 2026-08-06 - Added: Skip Intro button - Fixed: Player improvements & bug fixes diff --git a/CLAUDE.md b/CLAUDE.md index e18dd15..b0cb38c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,6 +116,10 @@ keystore the build still succeeds but emits an unsigned APK and logs a warning. matters more than the code: Android identifies an app by applicationId **plus** signing key, so a changed key forces every user to uninstall and reinstall. +Every version bump is also a publish operation: update the version and changelog together, +commit the complete release, and push it to GitHub. Never leave a bumped version only in the +local working tree. + For direct TV deployment without publishing a release, `deploy-tv.ps1` builds and verifies the signed release, connects over wireless ADB, installs it with `-r`, and launches the Leanback activity. It reads the same signing settings from the current user's persistent @@ -427,10 +431,94 @@ title matches above the backend's own relevance order — it never re-sorts alph and it keeps weak matches rather than showing an empty pane. A small access-ordered map caches results per query for the session, so backspacing is instant. +**Every search the tab performs is recorded, and the gateway is what records it.** It used +to depend entirely on the television posting to `/v1/search/history` after a result landed, +which meant a query answered from the client's own cache, one whose post failed, or one +from a build that predates the call was never written down at all — and the table feeding +the recent-searches row and future per-user ranking was a partial record of what the +household looks for. `handleSearch` now calls `recordSearchQuery` itself, before the Redis +lookup, so a cached answer counts the same as one that reached Emby. Things to preserve: + +- **The write is detached from the request context.** Instant search cancels the in-flight + request on every keystroke, so a write hung off `r.Context()` would be abandoned for + exactly the searches somebody typed fastest. It is also fire-and-forget: a search whose + record failed still returns results, and the failure is DEBUG for the same reason the + search line is. +- **`store.SearchDedupeWindow` is what makes two writers safe.** The handler and the + client's post both describe one search, and the POST route is still there because an + older APK is the only thing that records at all. An identical query inside the window is + the same search; a minute later it is its own row. +- **`searchQueryRecordable` is the one rule both routes apply**, so a query `/v1/search` + records is exactly one `/v1/search/history` would have accepted. Length is counted in + runes, or a title in Japanese is rejected at a third of an English one's length. + +**And the console can read it back** — `/admin/searches`, in the Insights group beside Row +engagement, over `internal/api/admin_searches.go` and the store queries in +`internal/store/searches.go` (where the writer moved to, so one table's rules sit in one +file). It is **two tables of the same rows on purpose**: the summary groups by +`lower(query)` and answers "what does this house look for", which is what a library is +organised against; the log is uncollapsed and newest-first and answers "what happened just +now", which is the one to read when somebody reports that search is not finding something, +because it shows the query as it was typed, by whom, and when. Things to preserve: + +- **The widest window is the retention period.** `searchWindowDays` is derived from + `store.SearchRetention` rather than written down, because `RecordSearch` prunes to it — + a page offering 90 days would draw a flat line for two thirds of it. A tile states the + retention for the same reason: a quiet week and a window that has aged out look identical. +- **`SearchTotals` is its own query, not a sum of the table above it.** The summary is + capped at `searchTermLimit`, so adding it up would report the top twenty-five's total as + the household's, wrong by however long the tail is. +- **Names are resolved in Go from `KnownUsers`, not joined per row**, and an id with no + session left keeps its row wearing the id — the query is what the page is for, and a + viewer whose sessions have expired is still one searcher rather than nobody. +- **Nothing on it is editable**, the stance every insights page takes. It also cannot + delete: the 30-day prune is the only thing that removes a row. + +**A genre is browsed, not searched.** The chips on the empty state ran their own label +through `/v1/search`, which is a text query: "Drama" matched a film *called* Drama, anything +with the word in its overview, and — relevance being a score rather than a rule — a +scattering of titles not in the genre at all, while missing most of the ones that were. A +chip now opens `GET /v1/genres/{genre}/items` (`server/internal/api/genres.go`, +`EmbyRepository.browseGenre`), which filters on Emby's `Genres` parameter and answers a page +at a time. Things to preserve: + +- **It is a mode, not a query.** `SearchUiState.genre` sits beside the query rather than + pretending to be one, which is what lets the pane head itself "Comedy" instead of "Search + results for “Comedy”" and lets Back step out of the shelf without clearing something + nobody typed. Typing supersedes it; `runSearch` returns early while a genre is open, or + the empty-query transition that opening one causes would wipe the shelf it just filled. +- **Paging must not repeat or skip a card**, which is why both paths sort on + `PremiereDate,SortName` rather than a date alone — two titles sharing a premiere could + otherwise swap places between requests, and the scroll would show one twice and the other + never. Newest first, because the alphabet is not an answer to "show me Comedy". +- **Two rules end the scroll and both are needed** (`hasMoreGenreItems`, pure and tested): + reaching the total is the ordinary end, and a page shorter than the one asked for is the + other — a backend that would not count says nothing useful with its total. `genreTotal` on + the server is the same judgement from the other side, for an Emby that did not count. +- **A page is appended by its own offset**, never by order of arrival: a response for an + offset already scrolled past, or for a genre the viewer has left, is dropped rather than + pasted into the middle of the grid. A page that fails part way down keeps what is on + screen and simply stops paging. +- **The scroll trigger is a `snapshotFlow`, not a composable read.** The last visible index + changes on every frame of a scroll, and reading it in the body would recompose the grid + the whole way down a genre — the same rule as the animation one above. It asks + `LOAD_MORE_ROWS_AHEAD` rows early, since a request that starts when the viewer arrives at + the end is one they watch. +- **Something has to take the focus the chip was holding**, because opening a shelf unmounts + the whole discovery pane. The grid claims it when the first page lands, and an empty or + failed genre hands it back to the keyboard rather than leaving a television with nothing + focused. +- **Episodes are excluded on both paths.** An episode inherits its series' genres, so + including them fills a page with twenty entries of one comedy and buries the rest. +- **The gateway path degrades to a keyword search on the *first* page only**, so a set on a + new build talking to a gateway that predates the route still shows something. A later page + does not: a gateway that answered page one and failed on page two is having trouble, not + missing the route, and search results pasted onto the end of a genre would be nonsense. + Focus is the hard part and is explicit: the leftmost keyboard column goes to the rail, the rightmost goes to the results grid, the grid's first column goes back to the *last key used* (a `FocusRequester` attached to whichever key that is), and the grid has a -`focusRestorer`. Back moves results → keyboard → clear query → leave, one step per press. +`focusRestorer`. Back moves results → keyboard → genre → leave, one step per press. Physical keyboards and phone-remote apps feed the same state through one `onPreviewKeyEvent` that consumes only printable characters and backspace — D-pad and Back must fall through. The voice button needs the `android.speech.RecognitionService` entry in @@ -574,6 +662,38 @@ fields to a home query is a startup-cost regression — extend the detail call i Emby time values are 100-ns ticks; convert at the boundary (`millisecondsToTicks`, `resumePositionMs`). +**A detail page is warmed while its card is focused, in two waves.** By the time somebody +presses a card, the item record, its "why you might enjoy it" and its playable URL have all +been fetched — and now so have its episode list and its trailer, which were the two things +the page still opened cold. That mattered most on Continue Watching, where every card is an +episode and pressing one opened a page with no season scroller and no episode list until the +network answered. Things to preserve: + +- **The two waves have deliberately different delays.** The metadata warm follows the D-pad + closely at `FOCUS_METADATA_DEBOUNCE_MS` (140 ms) because it decides what the panel beside + the row says; `warmDetailPage` waits `DETAIL_PREFETCH_DELAY_MS` (450 ms) because an + episode list is the largest request this client makes — a long-running show is a thousand + records — and warming one per card as somebody scans a shelf would spend more than it + saves. The job is cancelled when focus moves, so a viewer travelling along a row never + reaches it and one who has stopped, which is what precedes a press, does. +- **Everything warmed on focus must be single-flighted on the repository's own scope**, for + the reason `getRelated` already was: the warming job dies with the D-pad, and a request + cancelled at the socket is one the gateway logs as a failure and one nobody keeps the + answer of. `getSeriesEpisodes` and `getLocalTrailer` now take the same shape. A prefetch + added without it makes navigation *slower*, because the press that follows re-asks. +- **`getLocalTrailer` caches its negative answer** (`CachedTrailer`, the `CachedTrickplay` + precedent). Most of a library has no local trailer, so before this every detail page opened + with a request the gateway answered 404 to, repeated on walking Back and again for every + step of the "More like this" trail. +- **An episode is keyed on its series**, not on itself — that is what its own page will ask + for, and it is what makes one warm serve a whole row of Continue Watching. + +**A detail overlay seeds its settings from `repository.currentSettings`**, never +`Settings.EMPTY`. Collecting a flow with an empty initial value draws the first frame under +default preferences and then recomposes the entire page — and restarts the effects keyed on +those preferences, which is a second ratings request — one frame later, at exactly the moment +the page is trying to appear. + **Synced settings.** A viewer's settings live on the *server* and follow the person to whichever television they sign into; an operator can also read and push them per user from the admin console's accounts page. Three pieces: @@ -651,7 +771,7 @@ What syncs is a person's choices; what does not is anything identifying a *telev device name, update source and token, the screensaver's rotation and ring colour. `showTitleLogo` / `autoPlayNextEpisode` / `showTenMinuteReminder` moved from device-wide to per-profile as part of this, because a device-wide value would push whoever signed in last -into everyone else's account. `SettingsStore.applyRemotePreferences` writes all eighteen +into everyone else's account. `SettingsStore.applyRemotePreferences` writes all nineteen keys and the revision in **one** edit — DataStore rewrites the whole file per edit, and the revision landing apart from the values it describes would leave a TV permanently believing it was up to date while holding something else. @@ -764,13 +884,23 @@ is intercepted in `dispatchKeyEvent` and routed via the `ScreensaverActions` hol Playback from the dream `finish()`es first and starts `PlayerActivity` on a delayed main- thread post to avoid the "activity behind the dream" race. -**In-app updates.** `UpdateChecker` polls a user-configured **Gitea** release -(`/api/v1/repos/{owner}/{repo}/releases/latest`, token auth for private repos), downloads and -verifies the APK, then commits it to a **`PackageInstaller` session**. Because replacing the -APK kills a running Dream and leaves a black surface, `UpdateRecoveryReceiver` catches -`MY_PACKAGE_REPLACED` and relaunches `MainActivity` with +**In-app updates.** `UpdateChecker` polls a **Gitea** release +(`/api/v1/repos/{owner}/{repo}/releases/latest`, token auth for private repos) or a static +manifest, downloads and verifies the APK, then commits it to a **`PackageInstaller` +session**. Because replacing the APK kills a running Dream and leaves a black surface, +`UpdateRecoveryReceiver` catches `MY_PACKAGE_REPLACED` and relaunches `MainActivity` with `EXTRA_LAUNCH_UPDATED_SLIDESHOW`. +**There is no "check for updates" on the television.** Settings had an Updates page with that +button on it, and the address, repository and token it needs could not be entered anywhere on +this app — so the only thing it could ever report was a failure, on the one screen a viewer +opens when they already suspect something is wrong. The single answer about updates is the +gateway's (`server/internal/appupdate` → `ui/UpdateScreen.kt`), which arrives on every launch +and carries its own `downloadUrl`; `SettingsPage` has no `UPDATES` entry and the version this +TV is running is stated once, on About. The `updateBaseUrl`/`updateRepo`/`updateToken` keys +remain in `Settings` and nothing writes them — they are what an install predating this still +has in its DataStore. + The session is not an implementation detail — it is why the updater works on a television. The phone idiom, an `ACTION_VIEW` intent at the APK's content URI, fails three ways here: the implicit intent is subject to package visibility on Android 11+, several TV builds expose no @@ -854,31 +984,89 @@ person to every set rather than staying in the room it was made in. Things to pr embedded track, which is why `preferredTextTrack` falls back to running the same rule over the player's real tracks rather than giving up. -**A subtitle the library does not have is fetched through Bazarr**, from the player, in -`server/internal/bazarr` and `internal/api/subtitle_download.go`. The whole feature rests on -one property of Bazarr: it writes the file *beside the media file*. So the gateway stores -nothing, serves nothing and never holds a provider credential — it asks Bazarr to fetch, +**A subtitle the library does not have is fetched from the player**, in +`internal/api/subtitle_download.go` over the provider layer in `subtitle_providers.go`. +**Two backends answer and they are not the same shape**, which is the one thing to hold on +to about this feature. **Bazarr** (`server/internal/bazarr`) writes the file *beside the +media file*, so the gateway stores nothing and serves nothing — it asks Bazarr to fetch, calls `emby.RefreshItem` so Emby notices, waits `embyRefreshSettleDelay`, and re-reads the streams; the new track then arrives down the ordinary `PlaybackInfo` path, which is why `playableSubtitle` needed no new shape and `selectSubtitle` works on it with no special -case. Things to preserve: +case. **OpenSubtitles** (`server/internal/opensubtitles`) hands back *bytes*, and the +gateway has no reach into the media directory, so a file fetched there is stored in +`downloaded_subtitles` and served back as a sidecar from `/v1/subtitles/{file}`. That +difference is the entire reason the gateway now holds a subtitle at all, and it is +contained: `mergeSubtitleTracks` puts what the gateway holds beside Emby's tracks inside +`playbackSubtitles`, so a downloaded subtitle is an ordinary track on every later playback +rather than something that exists only in the response that produced it. Things to +preserve: -- **The hard part is identity, not the download.** Bazarr keys on the *arr's id - (`radarrid`, Sonarr's `episodeid`) and Emby knows neither, so `bazarrMovieFor` / - `bazarrSeriesFor` / `bazarrEpisodeFor` match by title, year and episode number. They are - pure and tested hard because a mismatch writes one film's subtitle next to another. - Episodes match on *numbers*, never titles — the two disagree often enough (translations, - differently named two-parters) to reject correct matches — and season 0 is specials, a - real season, not "no season". -- **The provider row is opaque.** `subtitle` is a provider-specific token that must be - handed back verbatim on the download call; it round-trips through the television - untouched rather than living in a server-side cache, so a viewer reading the list by - remote cannot have their choice expire underneath them. +- **The operator's switches are `store.SubtitlePolicy`, not environment variables.** A + provider is offered when the `subtitle_download` feature is on *and* it is configured + *and* it is switched on — `subtitleSources`, one place. Bazarr's address stays an + environment variable because it is a service the household runs; OpenSubtitles is an + account, so its key and login live in that document and the console's Subtitles page can + enter, replace or remove them without a redeployment. The store refuses to record + OpenSubtitles as on with no key, so the console can never show a switch that does + nothing. Nothing on that page ever returns a credential — only whether one is saved, + the stance the MDBList page takes. +- **A candidate carries its `Source` and the download dispatches on it.** The two tokens + are opaque in different ways and handing one to the other is a mistake nothing + downstream could detect. Empty means Bazarr, because an app built before there was a + second provider sends no source and all its rows came from one place. +- **The two providers make opposite trades on identity, which is the hard part.** Bazarr + keys on the *arr's id (`radarrid`, Sonarr's `episodeid`) and Emby knows neither, so + `bazarrMovieFor` / `bazarrSeriesFor` / `bazarrEpisodeFor` match by title, year and + episode number. They are pure and tested hard because a mismatch writes one film's + subtitle next to another. Episodes match on *numbers*, never titles — the two disagree + often enough (translations, differently named two-parters) to reject correct matches — + and season 0 is specials, a real season, not "no season". OpenSubtitles keys on an imdb + or tmdb id, which Memby already holds because the library import asks Emby for + `ProviderIds` so external ratings can be looked up, so there is **no guessing on that + path at all**. An episode is searched by its series' id plus season and episode number + whenever the episode carries no id of its own, since a show has one far more often than + each of its episodes does. +- **`resolveSubtitleTarget` reads the item once and fails per provider.** A film Bazarr + has never heard of may still have an imdb id, and a title with no provider id may still + be in Bazarr's list; only both failing is a failure. `providerSubtitles` then searches + both at once — a manual search is a live provider query measured in seconds — and a + provider that fails is dropped rather than failing the search. +- **The provider row is opaque.** The token is provider-specific and must be handed back + verbatim on the download call; it round-trips through the television untouched rather + than living in a server-side cache, so a viewer reading the list by remote cannot have + their choice expire underneath them. +- **A machine translation is offered, and says so.** It is a real answer and sometimes the + only one, so `rankMergedCandidates` sinks it below everything a person wrote rather than + hiding it, and `MachineOnly` is on the wire because it is the one property that changes + whether a viewer wants the row at all. The provider is now *named* on a row, where the + single-provider version deliberately did not name it: with two backends the same + language appears twice and "which of these is which" is a question the row has to answer. +- **The exhausted-allowance case keeps its own wording** (`opensubtitles.QuotaError` → + `subtitleFailureMessage` / `subtitleDownloadFailureMessage`). It is the only failure + where pressing the button again is definitely not the answer, and a television has no + log and no support channel — that sentence is the whole diagnosis. +- **A stored subtitle's id is derived, not random** (`storedSubtitleID`), so fetching the + same language for the same title twice replaces the file rather than growing a second + track a viewer has to tell apart by guessing. Its `gw:` prefix is what keeps it out of + Emby's namespace, which is stream indices — plain numbers — because the player matches a + track on the id it was handed. +- **The gateway sends a *path* for a subtitle it serves, never an address.** It does not + reliably know its externally reachable name and the television does, because it is the + thing talking to it. `EmbyRepository.resolveSubtitleUrls` puts the base and the `t=` + token on it, exactly as `imageUrl` does for artwork and for the same reason: media3 + fetches a sidecar as a plain URL with none of Memby's headers attached. A URL that is + already absolute is left alone, so it is safe over every playback response. - **`subtitleDownloadAvailable` rides the playback response**, not `/v1/status`: the drop-up is the only thing that asks and it already holds that response, where the status - poll is made by every open TV every ten seconds. It is `bazarr != nil` **and** the - `subtitle_download` feature, and the client default is **false** — a missing field must - never conjure a row that leads to a request the backend cannot answer. + poll is made by every open TV every ten seconds. The client default is **false** — a + missing field must never conjure a row that leads to a request the backend cannot answer. +- **A title with no subtitles is told so, in the track section** (`SubtitleTracksState`). + A list holding nothing but "Off" is indistinguishable from a menu that failed to load, + and the row that could fix it sits below a rule under a heading the eye has no reason to + travel to — so the notice goes under the SUBTITLES heading and the focus ring opens on + the search row instead of on "Off", which is the state the viewer is already in. Where + no provider can be asked it says that instead, once, rather than leaving somebody + looking for an option that is not there. `SubtitleMenuScreenshotTest` covers both. - **Client-side it is a second screen, not a third section.** The drop-up is 344dp by about a third of a 720p screen; stacking a track list, the size chips and search results squeezed the tracks to one visible row. `SubtitleDownloadState.expanded` swaps @@ -960,6 +1148,17 @@ decoded. Four things exist to hold it down, and each is easy to give back: same answer. `MainActivity` keeps two states for this — `launchingItem` is the gate that stops a second Play press stacking a second player, `resolvingItem` is the loading screen and belongs only to the route that waits. +- **The two launch forms are two places a `Playable` has to be unpacked, and only one of + them is obvious.** `adoptPlayable` handles the request form; on the URL form — the cold + start, and *any* launch where `readyPlayableForLaunch` had a warm prefetch in hand — the + intent is the only thing that will ever carry an answer, and a field with no + `putExtra`/`getExtra` pair silently takes its default. That is how `trickplayAvailable`, + `skipIntroAvailable` and `endCreditsAvailable` were all shipped switched off: each + defaults to **false** on purpose, so the omission produced no error, no log line and no + request — the gateway saw zero `/trickplay` and zero `/intro` calls across seventy-seven + playbacks. A new "is it worth asking the backend" boolean must be added in **four** + places: the `Playable`, the intent's parameter list, its `putExtra`, and `onCreate`'s + `getBooleanExtra`. `subtitleDownloadAvailable` is the worked example of all four. - **`ui/player/PlaybackTrace.kt` says where the time went.** "Playback is slow" is not actionable; `event=first_frame … activity=…(+…) player=… stream=… prepared=… ready=… first_frame=…` is. Marks are cumulative from the Play press, a repeated stage keeps the @@ -1038,10 +1237,13 @@ television. Read the first few kilobytes and every frame's byte range is known, thumbnail costs a ranged request of about seven kilobytes rather than a download nobody would wait through mid-seek. Things to preserve: -- **Emby answers ranges on that route and does not say so.** The response carries - `Accept-Ranges: none` and a `Content-Length` borrowed from the media file. Trust the 206; - both `emby.TrickplayBytes` and `TrickplayClient` cap the read anyway, because being wrong - about that must not turn a press of Right into a five-megabyte download. +- **Trust the 206, not the headers.** Emby 4.10.0.21 answers ranges on that route properly + and says so — a 206 with `Accept-Ranges: bytes` and a `Content-Range` naming the BIF's own + length — but earlier builds were reported to serve the range while advertising + `Accept-Ranges: none` and a `Content-Length` borrowed from the media file. So both + `emby.TrickplayBytes` and `TrickplayClient` cap the read at what was asked for regardless, + because being wrong about that must not turn a press of Right into a five-megabyte + download. - **A zero-frame BIF is an answer, not a fault.** Emby returns a perfectly well-formed 72-byte file for a title whose thumbnails it has not generated, and for a width it does not hold — which is why `trickplayWidth` is not a free parameter and the client's @@ -1070,6 +1272,20 @@ would wait through mid-seek. Things to preserve: - **Cancelling the in-flight frame is load-bearing**, the same property `collectLatest` gives search: presses arrive faster than a fetch completes, and without it a slow response for a frame already skipped past lands on screen after the one being waited for. +- **Scrubbing the transport shows them too**, not only the Left/Right chip. Those are the two + ways to move through a film, and previews that stopped the moment somebody pressed a button + to look at the controls read as the feature having broken rather than as a deliberate line. + With the transport up the presses are the time bar's — `seekControlsActive` stands down — + so `bindScrubPreview` hangs a `TimeBar.OnScrubListener` off `exo_progress` and draws into + `player_scrub_preview`, a strip above the bar in the *controller* layout, which is what + makes it disappear with the controls and need no visibility of its own. Things to preserve: + it is the same `TrickplayPreview` instance as the chip, so one layout is fetched per title + and one cache of frames serves both; it carries no wording, because the transport is already + printing the position a caption would repeat; its horizontal place is computed from the + scrubber in window coordinates and clamped to the bar, since a thumbnail parked mid-screen + while the scrubber is at the far end is a picture of some other moment, and one pushed past + the end is one overscan cuts; and it goes down on `onScrubStop` rather than on a timer, so + it can never outlive the scrub that raised it. - **The cache holds JPEG bytes, not bitmaps**, and is the player's own rather than Coil's. Decoded, forty frames would be most of a megabyte; as bytes they are a couple of hundred kilobytes, and decoding one costs a millisecond off the main thread. Keeping them out of @@ -1137,6 +1353,85 @@ not land somewhere different depending on whether the container is up. Things to `build/screenshots/skip-intro/`. There is no scrim under this button, so a capture over black would prove nothing. +**The closing credits are read out of the same chapter list as the intro.** `markersFor` in +`server/internal/api/intro.go` reads one `Fields=Chapters` response and answers for both +features, and `EmbyRepository.chapterMarkers` caches one reading that `introSegment` and +`creditsStartMs` both read. That is **the whole reason this is affordable** — adding a second +lookup anywhere in that path gives away the only performance claim the feature has. From the +result, `PlayerActivity` scales the picture into the left half, ramps it to 2× and puts what +is on next in the right half. + +**Two sources, and the one it was built on does not exist.** `CreditsStart` is in Emby's +`MarkerType` enumeration, which is what this was originally written against — but a survey of +the 20,000-item library it ships to found `Chapter`, `IntroStart` and `IntroEnd` and **no +`CreditsStart` at all** on Emby 4.10. The enum having a value is not the detector populating +it. What that survey did find was 216 items carrying a chapter *named* like credits, clustered +at 90–98% of runtime and consistent within a show, and that is where every bit of the +feature's coverage comes from today (roughly 5% of items, but effectively all episodes of the +shows that have it). The marker is still read first, so the day a version writes one this +needs no change. Things to preserve: + +- **The position floor is the load-bearing guard** + (`creditsMinimumPositionFraction` / `CREDITS_MINIMUM_POSITION_FRACTION`, 0.75). Chapter + names are not a vocabulary anybody agreed on, and real media carries **"Opening Credits"** — + Belfast at 1% of runtime, Game of Thrones at 0%. A name match without a position test starts + the pane in the *first minute* of a film and runs its opening at double speed, which is the + worst thing this feature could do. Three quarters is deliberately far below the evidence + rather than near it: every genuine roll in that survey began at 90% or later. The name + exclusions beside it are belt-and-braces — a position test catches wordings nobody thought + of, a word list only catches the listed ones. +- **The rule exists twice** — `creditsFromChapters` (Go) and `creditsStartFrom` + (`data/Credits.kt`) — pinned by deliberately parallel tests carrying the real library's + cases, the usual reason: with no gateway there is nobody to ask. +- **The two sources resolve in opposite directions, and that is not an inconsistency.** For a + *marker*, the last wins (as the intro rule takes the first): two starts mean the markers are + untrustworthy, so each rule picks whichever risks least, and the two features are damaged in + opposite directions — an intro skip firing late throws somebody past the story, while the + credits pane firing early runs the last scene past them at double speed. For a *name*, the + earliest qualifying chapter wins: several credits-named chapters are ordinary rather than + suspicious ("The Pitt" carries both "Credits" and "End Credits") and they describe one roll, + which begins at the first of them. +- **A marker below the floor gives way to a name; with no runtime at all, the marker is + honoured and the name is not.** An explicit marker is Emby asserting a position, so it gets + the benefit of the doubt on wording — but not on position, and never enough to skip the one + test that separates an opening sequence from a closing one. +- **`RunTimeTicks` rides the chapter lookup on both paths** for exactly that test. It is a + default field on the response, so it costs nothing; do not drop it to tidy the query. +- **The duration guard is a second, separate refusal.** `creditsWorthShowing` asks whether + enough of the roll is left to be worth moving the picture for (`CREDITS_MINIMUM_TAIL_MS`), + and lives client-side because the player has the decoder's exact duration. +- **2× is a ceiling that falls, never a speed that is defended.** Doubling the speed doubles + the bitrate pulled from Emby over HTTP, and a high-bitrate file on a remote server may not + sustain it. `STATE_BUFFERING` calls `stepDownCreditsSpeed`, `creditsCeilingAfterStall` only + ever goes down, and it is never re-armed inside one roll — a marginal stream that could + climb back would oscillate between stuttering and recovering for the length of the credits. + Reaching 1× leaves the pane up: what is on next is still worth showing, and only the + speeding up failed. +- **It is the next-up banner's replacement, not a second thing beside it.** Both shrink the + same picture and both say what is on next, so `updateNextUpFromPlayhead` returns early + while the pane is up and the countdown moves into it — that countdown only appears inside + the last minute, where it was always the banner's job. The pane also rides the banner's + 250 ms tick and its `nextEpisode` guard, which is exactly the right gate: a film and the + last episode of a season both correctly get nothing. It therefore inherits auto-play's + switch, since nothing resolves a next episode when that is off — deliberate, because this + *is* the auto-advance experience. +- **The transform is a scale, never a reparent.** The pre-roll moves the `PlayerView` between + parents; doing that mid-playback tears the SurfaceView down and flashes black over + somebody's credits. `CREDITS_VIDEO_SCALE`/`CREDITS_VIDEO_SHIFT_X` are public so + `EndCreditsScreenshotTest` can place its stand-in picture at exactly the transform the + activity applies — a capture that guessed the split would prove nothing about whether the + two halves balance, which is the only thing worth looking at. Neither is a round number: + 0.5 and 0.25 put the picture's left edge at exactly x=0, which is the first thing overscan + cuts. +- **`speedUpCredits` is a synced per-profile toggle**, defaulting to **on** — a different + trade from `skipIntroMode`'s, which defaults to the button rather than the automatic seek. + This one is visible, reversible and over in a minute, so somebody who dislikes it turns it + off having watched exactly what it does. `maskMarkers` withholds the half of a reading whose + feature an operator has turned off, on the way *out* rather than on the way in, so the cache + keeps the truth and a feature switched back on takes effect on the next playback. +- Screenshots are `EndCreditsScreenshotTest` → `build/screenshots/end-credits/`, over a + deliberately bright frame: there is no scrim between the credits and the panel. + **Performance instrumentation.** `PerformanceMonitor` (JankStats) is debug-only and logs to tag `EmbyClientPerf`. `benchmark/` is a `com.android.test` macrobenchmark module targeting the release variants the `androidx.baselineprofile` plugin generates, so its numbers are @@ -1252,6 +1547,117 @@ and two secondary greys, which is visible the moment a detail page opens from a colour or radius belongs in the token file, or is a considered exception — not a fifth value. +**Those colours are the server's answer, not constants.** Every token in `DesignTokens.kt` +is now a `get()` over one process-wide `mutableStateOf(MembyPalette)`, and `applyMembyPalette` +is what repaints the app. Two things follow and both are easy to undo: an alias must be a +getter too (`HomeComponents`' `EmbyGreen`, `DetailPageComponents`' `Detail*`, the settings +sheet's own `EmbyGreen`), because a `val` captures whichever theme was loaded when its class +initialised and never changes again; and `MembyTheme` builds its `darkColorScheme` per +composition rather than holding one, which is how a theme change reaches every component +that never names a colour. Shape and punctuation are deliberately *not* themeable — a +palette that could move a corner radius could make a layout wrong from the server, and the +whole safety of this feature is that the worst a bad theme does is look bad. + +**Themes** are `server/internal/api/themes.go`, and there are two kinds. A **selectable** +theme is the viewer's own choice, held as the ordinary synced preference `themeId` and +picked in Settings → Appearance. A **seasonal** theme (Halloween, Christmas, Easter) is not +a choice at all: it is in force for its dates and nothing on the television can decline it, +because a per-person opt-out is a thing somebody turns off in October and never reconsiders, +which is the same as the feature not existing. The only switch is the operator's +`seasonal_themes` feature flag, for the whole house. Things to preserve: + +- **`resolveTheme` is the whole rule and it is pure**: a season outranks the viewer, the + viewer outranks the default, and the operator's per-user allowlist narrows the *choice* but + never a season. There is no argument a television can send that suppresses one, which is + what "cannot be controlled by the user" means in code. The viewer's own pick is still + reported as `chosen` underneath a season, or the picker would show nothing selected for a + fortnight and read as having forgotten it. +- **The two windows resolve their edges deliberately.** Halloween opens on 25 October (a + theme nobody sees until the evening of the 31st is one nobody sees) and Christmas closes on + Boxing Day (the tree is down; red-and-green on the 30th reads as a server nobody maintains). + Easter is the anonymous Gregorian computus in `easterSunday` — computed, because a + hard-coded table is a feature with an expiry date on it — over Good Friday to Easter Monday. + `seasonalThemeFor` takes the time rather than reading the clock, so every edge is tested. +- **The revision is a hash of the resolved theme, not a counter.** There is no write to + attach a counter to: nobody writes anything at midnight on 1 December, the answer simply + becomes different. `/v1/status` carries `theme: {id, revision, locked, seasonal}` — the + `preferencesRevision` precedent — and the TV fetches `/v1/theme` only when it moves. It + rides the poll rather than the sign-in because that *is* the feature: a season has to reach + a set that is already switched on. +- **The allowlist is `user_themes`, and absence is permissive.** No row exists for anybody + until an operator restricts somebody, so an empty list means "unrestricted"; reading it the + other way would empty every picker in the house on the day it shipped. + `normalizeThemeAllowlist` stores "every box ticked" as the empty list for the same reason — + they are the same decision — and drops seasonal ids, which are not grantable per person. + It is a separate table from `user_preferences` because that document is the viewer's own + choices and every television they own writes it; this is policy *about* them and only the + console writes it. +- **The available list is per viewer and comes from the server.** `/v1/theme` sends only the + themes that person may pick, so a withheld scheme is a row the television was never sent + rather than a greyed one — the client has no catalogue of its own to fall back to, and the + picker is simply not drawn when fewer than two arrive. +- **Nothing repaints from the local choice.** `setThemeId` writes the preference and stops; + the palette arrives through `ThemeSync` when the gateway has resolved it. That is what makes + a choice a season covers, or one the operator has since withdrawn, visibly not take effect + rather than take effect and be yanked back a second later. +- **`ThemeSync` paints from the cached palette before any request**, the promise `HomeCache` + makes about the rows, and clears the cache on a profile switch (a different person, a + different scheme). The palette cache is device state; only `themeId` syncs. +- **There is no second copy of the rule on the direct path**, unlike subtitles, intros and + Continue Watching. Those exist twice because the direct path would otherwise *behave* + differently; here it behaves as it always did — the default palette. A television deciding + from its own clock that it is Halloween, while the household's gateway has seasons switched + off, would be the feature failing rather than degrading. `data/Themes.kt` is only hex + parsing, and it refuses anything it cannot read so the app's own token stands in. +**Seasonal decorations** are `ui/seasonal/SeasonalDecorations.kt`: snow, bats or blossom +drifting over the launcher for the few days a season is on. A palette on its own is a thin +idea of Christmas — the colours change and nothing says why — and this is the half that +does. It is also the most expensive thing in the app, the only animation that runs +continuously while somebody is merely browsing, so: + +- **Nothing about it recomposes.** One `Canvas`, one animated `State` never read in a + composable body, every position derived arithmetically inside the draw lambda. A full field + costs zero recompositions and one draw pass. The palette colours *are* read in composition, + deliberately, so a theme change repaints this node. +- **A particle is `index` and `progress` and nothing else** — no array, no per-particle state + to allocate or re-seed. `drawSeasonalField` is therefore a pure function of one number, + which is what lets a screenshot capture an exact frame with no animation clock. +- **Every cycle count is a whole number**, so at the instant the driving value rolls 1 → 0 + the entire field is exactly where it was. Without that it visibly jumps once a minute. + `decoration-snow-0.png` and `decoration-snow-100.png` must be identical; that is what they + are for. +- **Placement is stratified, not hashed.** Each particle owns a slice of the axis and the + hash only jitters within it. Twenty-six samples is far too few for a hash to look evenly + spread — the first version put visible bands and a bare patch through the middle, and the + eye finds a clump in a snowfield instantly. +- **The slug comes from the gateway** (`decoration` on the theme), never derived from the + theme id: an operator turning `seasonal_decorations` off sends an empty one, and a set + holding a cached Christmas palette must stop snowing. It is a *second* switch from + `seasonal_themes` because the palette and the animation have quite different costs on a + weak box. An unknown slug draws nothing. +- **Launcher only.** Not over playback — a film is the one thing nothing may drift across — + and not over the settings sheet. It is a sibling of `HomeScreen` rather than inside it, so + an arriving theme cannot invalidate the rows. +- **The platform's "remove animations" setting is honoured.** A season is deliberately not + the viewer's to decline, but an accessibility choice is not a preference. +- **The alpha was tuned by looking, not reasoned about.** The layer sits over an opaque + surface, so a flake occasionally lands on the Play button; at this value that reads as snow + in front of the screen and a few points higher it reads as a rendering fault. + `decoration-over-content.png` exists for exactly that judgement. + +- **Screenshots are the only real test this feature has.** A unit test can check that a hex + string parses; it cannot check whether Easter's pale accent is legible on its own + near-black or whether Forest still has a hairline. `ThemeScreenshotTest` renders one series + page under every palette → `build/screenshots/themes/`, deliberately one screen across nine + themes rather than nine screens on one, so the images differ in nothing but colour. It + composes once and repaints by assigning the palette, which is both the only thing + `setContent` allows and the more honest picture — that is exactly what a set already + showing the page does when a season begins. The palettes are a *fixture* copied from the + catalogue, not a second copy of the rule: nothing derives a colour from it. It caught the + chip row overflowing at six themes, which is why that row is a `FlowRow` where every other + choice row on the page is a `Row`, and it is why the blossom is a five-petal flower — a + single petal rendered as a grey seed, recognisable as *something falling* and nothing else. + **One button language.** `ui/MembyButtons.kt` — `MembyPlayButton` (focusable), `MembyPlayChip` (the same surface as decoration inside an already-focusable parent, for the home hero) and `MembyChoiceChip`. There were three: this one, a hand-rolled copy in the hero diff --git a/app/build.gradle.kts b/app/build.gradle.kts index c901cd5..dd91c1f 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -42,7 +42,7 @@ val projectNoticeText = // A release workflow can derive the app version from its Git tag without editing the // source tree. Local builds keep using the checked-in default. -val defaultVersionName = "0.2.27" +val defaultVersionName = "0.2.33" val membyVersionName: String = (project.findProperty("memby.versionName") as String?) ?.trim()