Move the star and more buttons from the meta line onto the first section
header so they sit below the hanging shuffle button again, and remove the
per-section counts now that the meta line carries the total.
Distinguish the album-credit rule (any downloaded song on the album) from the
track-credit rule (the credited track itself must be downloaded) in the
repository test, so a swap of those clauses fails.
Assert the combined meta line while loading, cover the downloaded partition
and the plural album/song credit branches, share the album-credit id query
with its plan test, and put the meta line through a translatable string.
Put the artist type and total album count on the single meta row beside the
star and more buttons, then list albums where the artist is an album artist
separately from albums where they only have a track credit. The hero shuffle
button and page-level actions stay where they were.
The split comes from an albumCredit flag on the shared credit rows, so the
online and downloaded album lists are partitioned by the same rule and the
two sections still sum to the stored album count.
Put the download-visibility comment on the constant it describes, keep the
no-scan guards beside the seek assertions, and give the identical-sync test a
positive control for both the artist aggregate and the album play recompute.
Share the artist page SQL with its plan test and assert the artist seeks, so
the test fails if the predicate push-down regresses. Add a query-log test that
an identical sync skips the aggregate, stop the downloaded album badge from
counting a song whose album row is missing, and correct comments that
overclaimed the remaining hand-written credit paths.
Replace the per-credit invalidation with one signal from the sync's write
transactions: if artists, albums or songs changed, the source is marked stale
and a single aggregate pass refreshes counts, plays and the credited flag in
one query. Identical syncs no longer run any of those scans, and a new credit
path cannot forget to invalidate the cache.
A changed sync still reads the credit unions once instead of separately for
counts and plays, and migration 33->34 remains a frozen snapshot rather than
sharing the runtime query text.
The artist page, shuffle, download attribution and artist badge each spelled
out the four credit paths. Reuse the shared credited song and album rows, and
keep only the download-aware album queries hand-written, where the per-credit
download visibility genuinely differs.
The shared song and album rows use UNION ALL so the artist list filter can push
its artist predicate into every branch instead of materialising a deduplicated
union; callers that need a set use DISTINCT or COUNT(DISTINCT).
The credit recompute was keyed on song-to-album moves and relation-table
writes, so a server that changed songs.artistId or albums.artistId while the
plural refs stayed the same left the counts attributed to the old lead. Fold
the lead ids into the change check for both songs and albums.
The artist badge now counts albums from album lead ids and album_artists even
when an album has no songs, and only counts song-credited albums that still
exist, matching what albumsForArtist lists. Preserve the derived albumCount
when the server re-upserts an artist row so an identical sync does not rewrite
it twice, relax the migration guard to either relation table, and share one
album-count order between the plain and downloaded artist lists.
Artist badges and play rankings only counted album credits, while the artist
page, shuffle queue and download attribution already included track credits.
Derive albumCount and the artist plays from the same four-way credit union,
count lead ids that the server omits from the plural refs, and bump the song's
credited artists on a local scrobble even when the track has no album.
Sort the downloaded artist list by the downloaded album count it displays
instead of the synced total, and backfill the artist totals on upgrade so the
fix applies before the next sync.
The preamp prompt promised extra level unconditionally, but a track whose gain has no peak is capped at unity. Say so in the supporting text so the setting does not overpromise for Opus/R128 libraries.
A peak is the only headroom check the gain code has. When the server sends a gain without one (Navidrome does this for Opus R128 gains, which carry no peak tags by design), a positive gain was applied unchecked and the 16-bit sink saturated.
Cap a peakless gain at unity instead: attenuation still applies, but a boost needs a peak to prove it fits. The cap is unchanged when a peak is present.
Update the tests that expected a peakless boost, and cover the positive preamp that can no longer lift a peakless track and the attenuation that still applies.
The adjacent artwork effect keys on the item and queue layout, but the predictions now also depend on the repeat mode. Toggling repeat left the cached neighbours stale, so the edge swipe still resisted until the track changed. Key the effect on the repeat mode too, with a route test that toggles repeat and expects the wrapped previous art.
Lift the wrap arithmetic out of next(), previous() and previousTrack() into nextPosition()/previousPosition() so upcomingItem()/previousItem() cannot drift from the actions again. A one-track queue no longer predicts itself as its own neighbour, online or offline: the wrap would land on the current position, and the actions are a no-op there, so the strip offered a dead swipe.
The now playing strip took its neighbours from previousItem() and upcomingItem(), which returned null at the queue edges online even when repeat would wrap. Swiping there resisted and only a hard fling advanced the track, leaving no artwork to slide in. Mirror next() and previousTrack() so the online path wraps with repeat All, and with repeat One for the next slot.
Use right/down chevrons in the download manager and order downloaded
tracks by disc and track number, matching the album screen. Move sync
concurrency to the end of the network settings, let the artist hero name
wrap to six lines, and leave the album hero title unbounded.
Restore the both-tables-empty fallback in the credit recompute, skip
redundant marker writes, append albumless tracks after album tracks, and make
the paged artist query observe sources. Add tests for an interrupted sync and
the partial-relation migration guard.
Left-join albums in the artist queue queries so albumless credited songs can
be played, downloaded and deleted. Add sortName to the starred index, persist
a per-source stale-credit marker so an interrupted sync cannot strand artist
counts or hide artists, and recompute artist play data in one grouped pass.
Add artists.credited, updated at the end of a sync that changed artists or
credits and backfilled on upgrade when relation data exists. Filter the
artist lists and the downloaded artist list on it, which keeps partially
synced libraries visible and hides display-only rows once credits are known.
Filter the paged, search and home artist lists to artists referenced by the
plural credits, falling back to the full list while the relation tables are
still empty. Display names keep using the singular tag and navigation keeps
using the plural refs.
Collect every credited artist id when resolving album and song sort keys so
the singular id and plural refs can both be looked up, and add regression
tests for the compilation and stale-sort-name cases.
Drop the FTS tables before the backfill so the old triggers do not index rows
that are about to be discarded, and rely on createSearchIndex's rebuild
instead of rebuilding a second time. Treat a whitespace-only artist sortName
as absent, and use the sort keys in the downloads tree too.
Parse the OpenSubsonic sortName attributes, store sort keys on artists,
albums and songs, and sort on them instead of the display names. Resolve
album and track artist sort names from the artists table, and include the
keys in the FTS content so romaji sort names are searchable as well.
Schema 31 rebuilds the sort indexes and search tables, and backfills
existing rows from their display names.
Use songCount as the total for both artist status queries and stop excluding
albumless tracks from download attribution, so the Artists badge and the
artist page agree. Backfill song counts in the 29->30 migration for offline
upgrades, and recompute only when a song moves albums or its credits change
rather than on every play-metadata sync.
Add artists.songCount, recomputed after syncs that change albums or songs,
and derive the Artists tab status map from the download rows plus that count
instead of walking every song. The badge query is now proportional to
downloads rather than library size.
Rewrite artist queue membership as a UNION of indexed song-id branches
instead of an OR that scanned every song, and recompute artist play data from
an explicit credited-album set instead of scanning all albums per artist.
Run PRAGMA optimize after syncing songs and before play aggregation, and
derive downloaded artist lists and filters from downloads rather than all
songs. Count downloaded albums per artist with artist-first branches.
Extend the query timing harness with the artist playback, download and status
queries plus a partial-download fixture.
Keep the server's combined legacy artist/albumArtist string when it is
present, prefer the OpenSubsonic displayArtist when it is not, and otherwise
join the plural artist refs with Navidrome's separator. The normalized value
is stored in the display columns, so song rows, album cards, now playing,
menu subtitles, downloads, search and artist ordering all get it.
Rewrite the artist album lookup as indexed UNION branches instead of ORs
that scanned the song table, and let artistName take the first indexed row.
Keep the type label and album count blank until the album flow has emitted,
so the page no longer flashes "Artist" and "0 albums" before showing the
real values.
A multi-artist album or song now adds one row per artist, so the sheet can
exceed the screen and clip the remaining actions. Cap it at 560dp and scroll
the content inside.
Credit all album artists in play aggregation and recorded plays, include the
relation rows in the downloaded artist list, and fall back to the album artist
display name in download groups.
Also keep the chooser scrollable, close it when the track changes, keep the
singular id in the album-artist label, stop a nested artists container from
swallowing later artist elements, and make the refs encoding length-prefixed
so tag values cannot corrupt it.
OpenSubsonic servers send plural artists/albumArtists refs and can omit the
singular artistId when an album or song has more than one artist. Parse and
store the ordered refs on the item and in indexed album_artists/song_artists
tables, so artist pages, shuffles, downloads and the artist filters include
every credited artist.
Album and now playing artist taps open a chooser when several artists are
credited; context menus get one artist row per artist.
The toggle's behaviour was untested: deleting the Modifier.keepScreenOn(),
hard-coding the row's request or emptying the Switch handler all left the
suite green, so none of the three new code paths were actually pinned.
- NowPlayingKeepScreenOnTest renders Now Playing and asserts a view in the
activity tree holds the screen on while the flag is set and releases it
once the flag clears.
- SettingsScreenTest now covers off->on and on->off on the row, and the
Switch itself, instead of only the off->on row click.
- Remember the keep-screen-on flow so NowPlayingRoute stops tearing down
and rebuilding its DataStore collector on every position tick.
A screen timeout mid-album is a common complaint for a music player: the
phone lies on a table while art or lyrics are on screen and the display
sleeps. Add a Settings toggle under Playback, off by default, that holds
the screen awake while now playing is open. The flag rides on the now
playing overlay through Compose's keepScreenOn modifier, and since the
overlay is only composed while expanded, collapsing it releases the flag
without any window-flag lifecycle handling.
Closesaustinried/subtracks#201
Review follow-ups.
- The snap clamped nothing, so an out-of-range stored value would show a
decibel label the slider cannot reach: clamps now live in the snap
rather than duplicated in the step.
- The screenshot test only rendered the dialog without asserting it
opened, so deleting the row's click would have left it passing while
the capture showed the plain list. It now requires the dialog's Reset.
- "Adjust level on top of the ReplayGain" was not grammatical, and the
mode prompt claimed the server normalises when it only supplies the
tags.
- Drop a test assertion that restated the production formula.
The in-repo comment rule allows a comment only when it names an external
system, version or issue, so both of these now name Compose Material 3,
which is what they are actually about.
Fifty-nine steps across a phone-width track is a tick every few dps,
which reads as a smudge on the bar. The steps are what make the slider
snap in 0.5 dB increments, so they stay; only the tick drawing is
suppressed, through a SliderDefaults.Track that skips drawTick.
- Call it ReplayGain, which is the name of the standard, in the row, the
prompt and the preamp prompt.
- Give the slider room under the prompt instead of sitting against it.
- Put Reset in the button row, where it lines up with Cancel and Done,
rather than above them, and centre the decibel value that is left in
the content.
- Add a screenshot test that scrolls to the section and opens the dialog,
which also composes the new rows; the existing screenshot only captures
the top of the list, so it never rendered them.
The slider alone is awkward for the two things people actually do with a
preamp: zero it, and nudge it by a known amount.
- Minus and plus buttons flank the slider, stepping one grid notch (0.5
dB) and disabling at the ends.
- A Reset button sits under it, beside the current value, disabled when
the value is already zero.
- Both go through the same snapping the slider does, so a button cannot
land off the grid.
Review follow-ups.
- Remove the second "Playback" section header the rows were added
under; the section already had one, so the screen rendered it twice.
- Snap the preamp to the 0.5 dB grid in the dialog. Material's slider
snaps in pixel space and converts back, so the value it reports is near
the configured grid but not on it: -1.0 dB arrives as -0.99999905, was
displayed verbatim and was stored that way until the next drag. The
step count is now derived from the range rather than hardcoded.
- Format the decibel value with the resource locale so translated
layouts get their own decimal separator.
- The controls move to `ReplayGainControls.kt`, since the file now holds
the grid as well as the labels, and the snap is covered by a test that
reproduces the off-grid value without it.
Final slice: the screen that drives the mode and preamp preferences the
playback side already reads. Until now `setReplayGainMode` and
`setReplayGainPreampDb` had no production caller, so Off and the preamp
were unreachable.
- `SettingsViewModel` exposes both preferences and writes them back.
- The Playback section gains a "Replay gain" row opening an Off / Track /
Album choice, and a "Preamp" row opening a slider across the +-15 dB
range in 0.5 dB steps.
- `ReplayGainMode` is ordered Off, Track, Album so the choice list reads
that way; the stored value is the name, so the order is not persisted
and the default is still Track.
- The labels live in their own file so the signed decibel formatting can
be tested, as `formatTrackTime` does.
Float output is the one way the audio processor chain is skipped on this
build, and it is a flag the renderers factory is handed. Nothing sets it,
so nothing happens today; if it is ever turned on, this says so once at
sink construction rather than leaving replay gain silently inapplicable
to high-resolution tracks.
Offload has no equivalent check: it is not a flag this code sets, and
AudioSink.setOffloadMode is only reachable by building the audio renderer
by hand. The factory comment names it instead.
Review follow-ups for the apply slice.
- A negative-infinite gain produced 0 dB, not unity: 10^(inf/20) is 0.0,
which is finite, so the result guard never fired and the track played
silent — the opposite of what the guard is for. A large-enough finite
negative gain underflows the same way, and an infinite gain paired with
a peak survived as the cap. The gain is now rejected before the pow and
an underflowed result falls back to unity.
- A peak carrying no data shadowed a usable peak in the other slot, so
trackPeak=0 with albumPeak=0.5 skipped the cap entirely — the gonic
shape on the default mode. Both peaks are filtered before one is chosen.
- Drop the claim that a period-less flush comes from a sink reset; it does
not, and the guard stands on its own.
- Trim two comments back to naming the Media3 types.
- Revert the observeStar rename, which was for a path that no longer
applies the gain.
The in-repo comment rule now allows a comment only when it names an
external system, version or issue, so the ones carrying rationale move
to this message.
- why the gain rides on the media item: PlaybackService can then apply it
the moment the item becomes current instead of after a library round
trip
- that ReplayGainController is written from the playback thread by
onFlush and from the app thread by the preference collector
- that an empty passthrough input is the pipeline's own shared buffer,
which cannot be put into itself
Review follow-ups for the apply slice.
- Drop the media item transition application. It never supplied a gain
the flush had not already supplied, and as a second writer it raced the
playback thread: it applied the incoming item's gain while the outgoing
item's buffers were still queued, scaling the previous track's tail by
the next track's gain (reproduced at +12 dB). The flush is the only
writer now.
This reverses the earlier decision to keep it as a fallback. That was
made on the premise that it was insurance against a flush that could
not be resolved. The flush covers every transition, and a period-less
flush only happens while the sink is being reset, so the premise does
not hold.
- Ignore a flush that carries no period rather than reading it as "this
item has no gain". Resetting to unity there would have been wrong if a
bare flush ever arrived mid-playback; the test that pinned that
behaviour now pins the opposite.
- Guard the pass-through branch against an empty input, which is the
pipeline's own shared buffer and cannot be put into itself.
- The controller is written from the playback thread now, so its state
updates and the recompute that follows them are synchronized rather
than two independent volatile writes.
- Add ReplayGainSinkTest, which drives the real DefaultAudioSink and
asserts it hands the processor the item being configured. That is the
media3 behaviour this whole feature rests on, and it was previously
asserted only by comments.
The app-thread transition callback was only ever a stopgap: media3 posts
it after the playback thread has already rendered, so the first buffers
of a new item could be scaled by the previous item's gain.
media3 forwards the playback timeline and the current period to
AudioProcessor.flush, and DefaultAudioSink flushes the processors on
every sink configuration, which happens at each media item transition
even when the format is unchanged. That is early enough to set the gain
before any of the new item's buffers are processed, on the playback
thread. See androidx/media#418.
The processor resolves the period back to its MediaItem and reports it,
so the gain maths and the preferences stay in ReplayGainController. That
controller is now written from two threads, so its state is volatile.
The transition callback stays as the fallback for a flush that carries no
period: without it a stale gain would persist for a whole track rather
than for a few milliseconds.
Review follow-ups.
- `ReplayGainController.applyItem` owns reading the gain off a
`MediaItem`, so the transition, the controller and the extras read are
exercised together rather than only in isolation.
- drop the leftover `setTrack(null)` on a missing active source: now that
the transition sets the gain from the item, that line only raced it and
could reset a playing item to unity.
- tests for a zero gain surviving the extras round trip (zero is a real
gain, not an absent one), for the latest item's gain being the one left
applied, and for a missing item applying unity.
The gain was resolved by querying the library from the media item
transition callback, so the buffers the player had already begun
queueing for the new track could be scaled by the previous track's gain.
A quiet track followed by a hot one could clip at the boundary for as
long as that query took.
Carry the gain on `QueueItem` so `queueMediaItem` puts it on the
`MediaItem` extras, and apply it from the item that just became current,
with no library round trip.
That narrows the window rather than closing it: media3 delivers
`onMediaItemTransition` on the app thread after the playback thread has
already rendered for that iteration, so the very first buffers after a
transition can still carry the previous item's gain. The gap is now one
handler hop instead of a database query.
The trade-off is staleness: if a sync changes a track's gain while that
track is queued, the item keeps the old value until it is rebuilt.
Applying a new gain mid-track is of marginal value anyway.
austinried
chiangww