Skip to content

Enhance library performance, genre separation, and better TTML support#2082

Merged
theovilardo merged 15 commits into
PixelPlayerHQ:masterfrom
Amonoman:master
Jun 9, 2026
Merged

Enhance library performance, genre separation, and better TTML support#2082
theovilardo merged 15 commits into
PixelPlayerHQ:masterfrom
Amonoman:master

Conversation

@Amonoman

@Amonoman Amonoman commented May 23, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Amonoman Amonoman force-pushed the master branch 20 times, most recently from 2722a8e to 2332d1d Compare May 24, 2026 18:09
@lostf1sh

lostf1sh commented May 24, 2026

Copy link
Copy Markdown
Collaborator

we'd really appreciate using semantic commits

@Amonoman Amonoman force-pushed the master branch 4 times, most recently from fb6887a to 9e06a35 Compare May 24, 2026 19:15
@Amonoman Amonoman changed the title Enhance library performance, scrollbar UX, and better TTML support Enhance library performance, genre separation, and better TTML support May 24, 2026
@Amonoman Amonoman force-pushed the master branch 4 times, most recently from 76cba5d to 084ee98 Compare May 27, 2026 17:18
@Amonoman Amonoman force-pushed the master branch 4 times, most recently from 17a694e to a84d138 Compare May 29, 2026 02:20
@theovilardo

theovilardo commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the work here, but I can’t merge this PR yet.

I validated it against current master. The merge applies cleanly and the targeted tests compile/pass, but there are a few blockers in the actual behavior:

  1. Genre matching has a regression in MusicDao.kt.
    The old query used LIKE :genreName, but the new multi-genre query uses genre = :genreName for the exact match case. That makes single-genre rows case-sensitive, while the repository/API/tests expect case-insensitive genre matching. Example: querying rock should still match Rock.

    Please fix the query so exact and comma-separated matches are consistently case-insensitive, and add a real DAO/Room test for:

    • Rock matching query rock
    • Rock,Pop matching pop
    • Rock, Pop matching pop

  2. The new load control path in DualPlayerEngine.kt drops setPrioritizeTimeOverSizeThresholds(true).
    The previous code explicitly used time-based buffering so high-bitrate/lossless files and compressed files start based on buffered duration, not buffered bytes. The PR replaces that builder and no longer sets this flag, which can regress FLAC/hi-res startup behavior.

    Also, loadControlBufferProfileFor() and its tests still exist, but production no longer uses that function. So the tests are green while no longer validating the real playback path.

    Please either restore the previous time-based buffering behavior or explain with evidence why removing it is safe, and update/add tests so they validate the actual load control used by buildPlayer().

  3. TransitionController.kt schedules a delayed transition after player swap using a captured MediaItem.
    After the 1s delay it only checks that the observed player is the same. If the user changes tracks during that delay, it can schedule a transition for a stale item.

    Please re-check that newPlayer.currentMediaItem still matches the captured item before calling scheduleTransitionFor.

Once these are fixed and covered by tests, I’m happy to re-review.

Amonoman and others added 13 commits June 4, 2026 20:32
@Amonoman
perf(library): suppress redundant genre recomposition and add file_pa…
2340a03 
…th index migration

LibraryStateHolder: add distinctUntilChanged() after the theme-color map
in the genres flow. The existing one skips recomputation when genre seeds
are unchanged; the new one suppresses downstream recomposition when the
fully-resolved Genre list is structurally identical — preventing UI
invalidation on every unrelated Room table write.

Add Migration41To42 to create index_songs_file_path on existing installs.
SongEntity already declares the index for fresh installs; this covers
devices upgrading from schema v41. getSongByPath() was doing a full table
scan — 10–50ms per call at 10 000+ songs on mid-range hardware.

Bump PixelPlayDatabase version to 42 and register MIGRATION_41_42.
@Amonoman
feat(lyrics): fix three TTML support gaps
ce954bb 
LyricsImportSecurity: add MAX_TTML_FILE_BYTES (1 MB) separate from the
256 KB LRC limit — Apple Music TTML files routinely exceed 256 KB and
were silently rejected.

LyricsUtils: extend looksLikeTtmlDocument to match <smil> root elements
(Apple Music / broadcast TTML variants) and <tt:tt> namespaced roots in
addition to bare <tt>.

LyricsRepositoryImpl: check SYNCEDLYRICS and TTML tag keys in addition
to LYRICS and UNSYNCEDLYRICS when reading embedded lyrics from file
metadata — covers files tagged by mp3tag, Kid3, and broadcast tools.
@Amonoman
feat(genre): support multi-genre tags in genre grid and song browsing
c77d32d 
- Add MusicDao.getSongsByGenreContaining with four LIKE patterns covering
  all comma-separated positions (first, last, middle, exact)
- Fix getGenres() to flatMap raw genre strings on "," before building
  Genre objects, so "Rock, Pop" appears as two separate genre cards
- Fix getMusicByGenre() to use getSongsByGenreContaining instead of
  the exact-match getSongsByGenre so multi-genre songs are included
  when browsing any of their genres
@lostf1sh @Amonoman
fix: address lyrics and genre review issues
4e07355
@Amonoman
feat: honour system 12h/24h preference in all stats timeline labels
e4b85c5 
Pass Context into formatTimelineLabelForRange and convert am/pm hour
labels to 24h format (e.g. "09:00") when the device is set to 24-hour
time, leaving 12h format unchanged for 12-hour devices.

Covers both the full stats screen (ListeningTimelineSection via
TimelineBarChart) and the overview card (MiniListeningTimeline in
StatsOverviewCard), which was previously rendering raw entry.label
with no formatting.
@Amonoman
fix: honour system 24h setting in RecentlyPlayed hour bucket labels
2b812be 
resolveTimestampBucket was hardcoding "h a" pattern, always producing
12h labels (e.g. "2 PM") regardless of system setting. Now checks
AndroidDateFormat.is24HourFormat(context) and uses "HH:mm" pattern
(e.g. "14:00") when the device is set to 24-hour time.
@Amonoman
fix(player): CBR seeking, adaptive buffers, CLOUD_PROXY_SCHEMES, dead…
561b253 
…-code removal

DualPlayerEngine.kt
- fix: FLAG_ENABLE_CONSTANT_BITRATE_SEEKING_ALWAYS → FLAG_ENABLE_CONSTANT_BITRATE_SEEKING
  VBR MP3s with Xing/VBRI headers now seek via their own table instead of the CBR
  approximation, eliminating the ±30 s position jumps on variable-bitrate files
- feat: replace inline DefaultLoadControl with buildAdaptiveLoadControl()
  Low-RAM devices (ActivityManager.isLowRamDevice) get 15 s/30 s buffers; normal
  devices keep 30 s/60 s; both tiers drop bufferForPlaybackMs 5 000 → 2 500 ms
  (ExoPlayer's documented default) cutting first-audio latency with no regression
- refactor: add CLOUD_PROXY_SCHEMES = REMOTE_MEDIA_SCHEMES minus {http, https}
  Scopes resolveDataSpec cache lookup and resolveMediaItem guard to proxy schemes only;
  plain HTTP/HTTPS URIs previously entered the LruCache path needlessly on every
  DataSpec resolution and on every setMediaItem call
- refactor: pre-resolution loop — remove mutableListOf<Uri> + for-each
  Replace with two direct ?.takeIf { scheme in CLOUD_PROXY_SCHEMES }?.let { } chains,
  eliminating one list allocation and one iterator per track transition
- refactor: rebuildPlayersPreservingMasterState — .map → pre-sized ArrayList + for loop
  Removes the IntRange object and avoids the extra copy that .map produces
- refactor: ensureQueueSnapshot — merge two consecutive refresh guards into one
  isEmpty() short-circuits the size check so refreshQueueSnapshotFromMaster() is
  called at most once per invocation instead of potentially twice
- remove: awaitPlayerPlaying — private, never called anywhere
- fix: disable redundant ID3 parsing for FLAC

MusicService.kt
- fix: remove unreachable `return true` after `return hasWearHints` in
  shouldRejectWearController (dead code, flagged by the compiler as unreachable)
- perf: ByteArrayOutputStream in readBytesCapped pre-sized to 4 × DEFAULT_STREAM_BUFFER_SIZE
  (32 KB) to cut reallocation churn on typical 50–300 KB album-art payloads
- chore: collapse triple-duplicate KDoc block above the `future` extension function
  into a single comment
@Amonoman
refactor(preferences): clean up UserPreferencesRepository internals
a10100c
@Amonoman
refactor(preferences): extract EqualizerViewMode to top-level enum
6a4c839 
Promote EqualizerViewMode out of its nested scope into a standalone file,
update EqualizerPreferencesRepository and all import sites to use it, and
add a missing catch block in DualPlayerEngine.
@Amonoman
fix: remove FLAG_WORKAROUND_IGNORE_EDIT_LISTS to fix Opus seek skip o…
f3ee681 
…n startup

Opus files store encoder pre-skip samples in an edit list. Setting
FLAG_WORKAROUND_IGNORE_EDIT_LISTS on the Mp4Extractor caused ExoPlayer
to discard that edit list, landing playback ~44-52s into the track on
first startup.

The skip only happened on the first 1-2 tracks because the audio offload
stall fallback (4s timer) would trigger rebuildPlayersPreservingMasterState,
which snapshotted the bad position and restored it on the rebuilt player.
Once offload was disabled after the second rebuild, it never fired again.

Fix 1: Remove FLAG_WORKAROUND_IGNORE_EDIT_LISTS. That flag exists for
broken MP4 files with malformed edit lists and actively harms Opus.

Fix 2: Guard the position snapshot in rebuildPlayersPreservingMasterState
to fall back to 0 if the saved position is under 5s, preventing any
future early-startup rebuild from restoring a spurious offset.

Also removes the now-unused Mp4Extractor import.
@Amonoman
fix(player): delay post-swap transition scheduling to prevent immedia…
f565b41 
…te re-crossfade

After a crossfade swap, swapListener was calling scheduleTransitionFor
immediately on the incoming track. The duration wait loop would block on
C.TIME_UNSET, but by the time duration resolved, currentPosition had
already advanced past transitionPoint — triggering an instant second
crossfade and jumping the track ~47s into playback.

Add a 1s delay before scheduling so the incoming track's duration and
position stabilize first. A currentObservedPlayer identity check ensures
the delayed schedule is a no-op if another swap occurs in the interim.
@Amonoman
Fix audio skipping by disabling offload for Pixel on SDK >= 37
c34c0fd 
The root cause of premature song termination and massive position skips
(jumping forward ~49s at a time) is a HAL-level bug in the audio offload
implementation on the Pixel (akita_beta) running Android SDK 37 beta.
ExoPlayer is merely reporting the corrupt hardware position counters.

Fix this by adding Google/Pixel to `shouldDisableAudioOffloadByDefaultForDevice`
for SDK >= 37, mirroring the existing Xiaomi workaround for SDK >= 36.

Note: Retain the previous TransitionController 1s delay fix as it provides
a genuine improvement. This workaround should be revisited once stable
SDK 37 releases to check if the HAL bug was resolved upstream.
@Amonoman
Refactor: Improve genre matching, stabilize FLAC playback, and refine…
16738d3 
… transition logic

​MusicDao.kt: Updated genre filtering to use LIKE for case-insensitive matching, ensuring consistent results for all genre list formats.
​DualPlayerEngine.kt: Enabled setPrioritizeTimeOverSizeThresholds(true) to utilize duration-based buffering, preventing premature rebuffering during high-bitrate FLAC playback.
​TransitionController.kt: Added a mediaId check post-delay to prevent stale transitions if the user switches tracks while the timer is running.
Amonoman added 2 commits June 7, 2026 15:43
@Amonoman
Merge branch 'master' into master
5e6a240
@Amonoman
fix(ci): remove rebase duplicate declarations and fix CodeQL Kotlin 2…
69b8054 
….4.0 incompatibility

Rebase introduced multiple duplicate declaration blocks in
UserPreferencesRepository and duplicate init block in SettingsViewModel,
causing overload resolution ambiguity errors across SettingsViewModel,
SetupViewModel, FileExplorerStateHolder, LibraryStateHolder,
MainViewModel, PlayerViewModel, and DailyMixStateHolder.

Fixes:
- Remove duplicate libraryTabsOrderFlow declaration (lines 1329 vs 1046)
- Remove two old-style dataStore.data.map blocks duplicating blockedDirectoriesFlow,
  initialSetupDoneFlow, keepPlayingInBackgroundFlow, favoriteSongIdsFlow,
  fullPlayerLoadingTweaksFlow, showPlayerFileInfoFlow, minSongDurationFlow,
  minTracksPerAlbumFlow, isGenreGridViewFlow, isAlbumsListViewFlow, and more
- Remove duplicate dailyMixSongIdsFlow, yourMixSongIdsFlow,
  lastDailyMixUpdateFlow and their save functions
- Merge duplicate init block in SettingsViewModel
- Remove duplicate KDoc comment in SetupViewModel
- Pin Kotlin to 2.3.20 in CodeQL workflow via gradle.properties at CI
  runtime; CodeQL's extractor does not yet support Kotlin 2.4.0

TODO: remove the Kotlin version pin once CodeQL adds Kotlin 2.4.0 support.
@theovilardo theovilardo merged commit 3b1cfae into PixelPlayerHQ:master Jun 9, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Amonoman @lostf1sh @theovilardo