Add explicit download management for offline playback - #43
Open
chamika wants to merge 12 commits into
Open
Conversation
Adds deliberate, user-controlled offline downloads on top of the existing implicit LRU cache + prefetch, closing #38. - Pinned downloads live in a dedicated SimpleCache with a NoOpCacheEvictor, driven by its own DownloadManager (custom download-index table so it coexists with the prefetch manager on the shared database provider). Playback nests CacheDataSources: read-only pinned cache first, then the transient LRU cache, then network — so pinned tracks play offline and are never evicted or duplicated into the LRU cache. - "Download for offline" per-item browse action on albums, playlists and audiobooks via Media3 media-item command buttons; dispatched in onCustomCommand, expanding the container through the existing resolver. - New "Downloads" browse category (toggleable in Browse categories) listing pinned containers, each with a "Remove download" action. - Pinned state persisted in a new Room table (pinned_downloads), separate from the sync-wiped media cache; DB v1 -> v2 with a non-destructive migration that preserves the existing offline library cache. - "Cache favourites" now routes through the pinned system, so favourites survive eviction and appear under Downloads. - Settings shows storage used by pinned downloads; logout clears pinned state and cache. Tests: new PinnedDownloadDao and migration tests, Downloads-node repository tests, downloads browse-category test, and pinned-cleanup settings tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012BSRADdi4UoKRejWDKeZDC
Resolves conflicts with the Artists/Albums/Genres browse categories (#42), which touched the same category-registration points. Both feature sets are kept: - MediaItemFactory: DOWNLOADS constant alongside ARTISTS/ALBUMS/GENRES; downloads()/downloadedContainer()/artworkUriFor() kept next to artists()/albums()/genres()/letterBucket(), using main's albumCategory signature. - JellyfinMediaTree: "downloads" appended last in canonicalOrder so neither the pre-existing tabs nor the newly added ones shift; DOWNLOADS added to the getItem dispatch. - MediaRepository: DOWNLOADS added to main's restructured staticIds; the local-only Downloads children path and the sync skip are preserved alongside shallowSyncSections. - Resources: both sets of browse category entries and strings retained. - Tests: kept main's artists/albums/genres ordering tests and updated the downloads ordering test to assert it stays last. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012BSRADdi4UoKRejWDKeZDC
chamika
force-pushed
the
claude/unaddressed-issues-review-q094to
branch
from
August 26, 2026 16:17
97a2545 to
8d3b1df
Compare
…sues-review-q094to
Cut the sections a session can reconstruct from the repo itself, so they stop costing context on every turn: - Project Structure tree (ls/find shows it) - User-Configurable Settings table (res/xml/preferences.xml, arrays.xml) - Manifest Components (AndroidManifest.xml) - Tech Stack (gradle/libs.versions.toml) - Build & Run (standard ./gradlew invocations) - Min/Target/Compile SDK, JVM, Kotlin, Gradle (automotive/build.gradle.kts) Replace the Releasing section with a pointer to the existing `release` skill, which already covers the version bump and commit format; the detail now loads only when cutting a release. Kept what the code cannot explain: the Architecture subsections (service flow, auth, caching, audiobook support, state persistence) and the Jellyfin API index. 149 -> 53 lines, ~1.7k -> ~0.7k est. resident tokens per session. Claude-Session: https://claude.ai/code/session_01Vunc8oUg8qTF9xZAkTQ8zv
Selecting DashTune in the AAOS media host killed the host process, which
reads as "the app crashes on launch" — the whole media UI goes down while
our service is still healthy:
FATAL EXCEPTION: main Process: com.android.car.media
java.lang.NullPointerException: uriString
at android.net.Uri.parse(Uri.java:467)
at MediaItemsRepository.parseBrowseActions(MediaItemsRepository.java:427)
The download/remove buttons passed to setCommandButtonsForMediaItems set
only setCustomIconResId, which is Media3-internal. LegacyConversions
.convertToBundle writes the browse action's icon-uri key only when
iconUri != null, so the key was absent entirely and the host called
Uri.parse(null) on connect.
Set an android.resource:// icon uri the host can resolve across process
boundaries, keeping the res id for Media3-native hosts, and record the
trap in a comment so the next button added here does not repeat it.
Regression from 1d0a5d4; main is unaffected as it declares no media item
command buttons.
Verified on the AAOS emulator: media host survives, browse tree loads,
crash buffer empty. testDebugUnitTest green.
Claude-Session: https://claude.ai/code/session_01Vunc8oUg8qTF9xZAkTQ8zv
…sues-review-q094to # Conflicts: # CLAUDE.md
The "Download for offline" and "Remove download" browse actions were published correctly but never rendered. Two independent causes: - The AAOS media host only draws per-item browse actions in its list layouts: media_browse_list_item.xml carries a browse_item_actions_container and neither grid layout has one. Every node carrying DOWNLOAD_COMMAND asked for a grid, so the action had nowhere to be drawn. - CommandButtons.resourceUri built the numeric resource form (android.resource://<pkg>/2131230896), which the host cannot resolve — it logged "Resources$NotFoundException: Resource ID #0x0" and drew nothing. Category artwork already used the named type/entry form. These stack: fixing only the content style leaves an invisible but tappable button, which is worse than no button at all. Category content style now follows the Downloads browse category. With it ticked, the categories that offer downloads (Playlists, plus Latest, Random, Books and Albums via albumCategory) switch to list items so the actions can render; with it unticked they keep the artwork grid. The Downloads node itself is always a list — it is only reachable when the category is on, and it hosts "Remove download". Favourites and Genres keep their existing explicit list styles. The browse_categories key and its defaults move to MediaItemFactory constants so JellyfinMediaTree and the content-style check cannot drift. Also adds the missing @after unmockkAll to MediaItemFactoryExtendedTest. It mocked AlbumArtContentProvider.Companion and never unmocked it, so the stub leaked into AlbumArtContentProviderTest and failed it depending on class order. Verified on the AAOS emulator in both states: with Downloads enabled the action renders on every row and a tap delivers COMMAND.DOWNLOAD with the item id; with it disabled the grid is unchanged. Claude-Session: https://claude.ai/code/session_011vcmudU96aatWWarM6mdtJ
Selecting Downloads switches the browse categories from an artwork grid
to list items, the only layout the AAOS host draws per-item browse
actions in. The host resolves a node's presentation once, when it builds
the browse view: re-delivering children — even after invalidating the
media tree and notifying every category — does not revise it, so the tab
strip updates immediately but the grid/list style does not. Only a
reconnect applies it.
Rather than leave that looking broken, changing the Downloads selection
now asks whether to restart. "Restart now" reuses the existing Force
exit path; "Later" just dismisses, and since the preference is saved
either way the layout applies whenever DashTune next starts. Toggling
any other category is unaffected and prompts nothing.
The hint deliberately is not a toast — the host drops them under a quota
("above allowed toast quota" in logcat) — nor a permanent summary line,
which would caveat every category change for a limitation that only
concerns Downloads.
Verified on the AAOS emulator: restart-now reopens straight into the
list layout with the download buttons; later saves the preference and
defers the layout to the next start; other categories prompt nothing.
Claude-Session: https://claude.ai/code/session_011vcmudU96aatWWarM6mdtJ
"Shuffle all" is a synthetic playable playlist standing in for "play this folder shuffled". MediaItemFactory.shuffleItem builds it with no supported commands, but MediaRepository.toMediaItem re-advertises the download action for any cached row that is playable and typed ALBUM or PLAYLIST — which the shuffle row is (MEDIA_TYPE_PLAYLIST, isPlayable). Shuffle rows are persisted like any other child, so once a folder or genre had been browsed its shuffle row came back from the cache carrying a download button for a container that has nothing to pin. Exclude shuffle ids there, via the existing MediaItemFactory.isShuffleId. Verified on the AAOS emulator against a folder whose shuffle row was in the Room cache: the row no longer shows the action, while album and playlist rows keep theirs. Claude-Session: https://claude.ai/code/session_011vcmudU96aatWWarM6mdtJ
The grid/list gate only reached the category nodes, so three places still disagreed with it. Artists: forArtist hardcoded a grid for an artist's children, so the albums three levels into Artists had nowhere to draw the download action. It now follows the same gate as the categories. Favourites: the node set only the playable content style. Favourites mixes playable rows (tracks, albums, audiobooks) with browsable ones (artists), and the host renders a node by one style, falling back to its grid default for the half that was left unset. Both are now set. Cached rows: toMediaItem replays the extras captured when the row was first stored, so a container kept whatever style it had then regardless of the current setting — an artist cached as a grid stayed a grid even after the fix above. With Downloads on, the list style is now stamped over the stored value; with it off the stored per-type styles are the intended ones and are left alone. Also gates the download action itself on the setting. Folders, Genres, letter buckets and audiobooks are list-styled whatever the setting, so their album and playlist rows were offering a download button while the feature was switched off — visible only in those always-list corners, which is why it looked arbitrary. Verified on the AAOS emulator with Downloads on: Favourites is a list, an artist's albums are a list carrying the action, and Folders shows the action only on rows that can take it. With Downloads off every category returns to its previous look and no row offers the action. Claude-Session: https://claude.ai/code/session_011vcmudU96aatWWarM6mdtJ
Twenty-two end-to-end tests driving a real MediaBrowser, split by the setting the behaviour hinges on. With Downloads enabled, every category that can hold a pinnable container is checked: Latest, Random, Playlists, Books at both levels, Favourites, the Albums and Artists letter buckets, an artist's albums, a genre's albums and a folder's albums. Rows that are not containers — tracks, chapters, artists, genres, shuffle rows, letter buckets and the category nodes themselves — are asserted not to offer it. With Downloads disabled nothing may offer it. That half matters most in the corners that are list-styled whatever the setting (Folders, Genres, letter buckets, audiobooks), since those are the only places an ungated action had somewhere to draw itself. The content style is asserted alongside the action wherever it decides whether the action can be seen: the host draws browse actions only in its list layouts, so a command advertised inside a grid is invisible but still tappable. Both the fresh and the Room-cached paths are covered, the cached one being where a stored grid style used to win. Verified by mutation: reverting the command gate fails five of the disabled tests, and re-hardcoding forArtist's grid fails the artist one. Also clears pinned_downloads in DashTuneE2eRule. It is deliberately not part of the library cache and survives a sync, so it outlived the rule's clean-up and carried real device state into the suite — three tests failed on a developer machine that had pinned something by hand, and passed in isolation. Claude-Session: https://claude.ai/code/session_011vcmudU96aatWWarM6mdtJ
Download albums, playlists and audiobooks for offline listening, with faster and more reliable browsing Full release notes: - New: download albums, playlists and audiobooks for offline listening — pick "Download for offline" on any album, playlist or book row - New: a Downloads browse category listing everything you have saved, with a "Remove download" action on each entry. Turn it on under Settings → Browse categories - Downloaded music is kept separately from the automatic cache, so it is never evicted to make room for recently played tracks - "Cache favourites" now saves through the same system, so favourites survive and appear under Downloads - Settings shows how much storage your downloads are using, and signing out clears them - Fix the download button not appearing at all on the media screen - Fix the download button missing from albums under an artist, and from Favourites - Fix a download button appearing on "Shuffle all", which has nothing to save - Fix download buttons showing in Folders and Genres even when downloads were switched off - Changing the Downloads category now offers to restart the app so the new browse layout applies straight away - Fix browse pages beyond the first failing to load - Fix missing icons on media item browse actions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #38.
Adds deliberate, user-controlled offline downloads on top of the existing implicit LRU cache + next-track prefetch, so a driver can intentionally pin content for a trip and see what's actually available offline.
What's new
Design notes
Two caches, nested for playback. The transient
exoplayer_cache(LRU, sized bycache_size) keeps serving playback buffering and next-track prefetch unchanged. A newpinned_downloadscache uses aNoOpCacheEvictorand is driven by its ownDownloadManager(a customDefaultDownloadIndextable name so it coexists with the prefetch manager on the sharedStandaloneDatabaseProvider). Playback nests theCacheDataSources — read-only pinned cache → transient LRU cache → network — so a pinned track plays fully offline and is never re-copied into the LRU cache, while non-pinned playback is byte-for-byte unchanged.Idiomatic per-item actions. Uses Media3's media-item command buttons (
setCommandButtonsForMediaItems+MediaMetadata.setSupportedCommands); taps arrive inonCustomCommandwith the item id inMediaConstants.EXTRA_KEY_MEDIA_ID, and the container is expanded to its tracks through the existingMediaItemResolver. Cached container rows re-advertise the action inMediaRepository.toMediaItemso it survives a sync.Persistent, separate pinned state. A new
pinned_downloadsRoom table (one row per pinned container) is kept apart fromcached_media_items, which is wiped on every sync/clear-cache/logout — so pinned state and its downloaded audio persist. DB version 1 → 2 with a non-destructiveMIGRATION_1_2that only creates the new table, preserving users' existing offline library cache. The Downloads node is served straight from this table (local-only, works offline, reflects removals).Verification
./gradlew :automotive:assembleDebug— passes./gradlew :automotive:testDebugUnitTest— passesNew tests:
PinnedDownloadDaoTest(CRUD/ordering),DashTuneMigrationTest(v1→v2 preserves the cache and adds the table), Downloads-node repository tests, a Downloads browse-category ordering test, and settings tests asserting logout clears pinned state while clear-cache keeps it.Not yet verified on device
No emulator/live Jellyfin server was attached. Worth watching on first run:
streamingUri(...)URL used for playback; a token or bitrate change between download and playback would miss the URL-derived cache key (same class of limitation the prefetch path already has).🤖 Generated with Claude Code
Generated by Claude Code