Skip to content

Commit 91f3ef0

Browse files
mmarca-techclaude
andcommitted
all: trim comments
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 588dde0 commit 91f3ef0

9 files changed

Lines changed: 34 additions & 115 deletions

File tree

app/build.gradle

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ android {
4343
buildTypes {
4444
debug {
4545
applicationIdSuffix ".debug"
46-
// Suffix with the abbreviated commit so any crash report self-identifies which
47-
// build produced it -- line numbers in stack traces are useless without that.
46+
// Include the commit so crash reports self-identify their build.
4847
versionNameSuffix "-DEBUG-" + ("git rev-parse --short HEAD".execute([], rootDir).text.trim() ?: "unknown")
4948
}
5049

app/src/main/java/org/oxycblt/auxio/image/coil/CoilModule.kt

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,21 +61,14 @@ class CoilModule {
6161
.transitionFactory(ErrorCrossfadeTransitionFactory())
6262
// Not downloading anything, so no disk-caching
6363
.diskCachePolicy(CachePolicy.DISABLED)
64-
// Coil defaults both of these to Dispatchers.IO, which is 64 threads wide. Scrolling
65-
// a large library queues a cover request per row as it flies past, so that default
66-
// lets dozens of decodes run at once and take every core with them -- the UI thread
67-
// then misses its frame deadline by hundreds of milliseconds even though no
68-
// individual decode is slow. Bound the whole pipeline instead. Both stages share one
69-
// limiter so this caps total concurrent cover work, not each stage separately.
64+
// Coil defaults these to Dispatchers.IO (64 threads), which lets fast scrolling
65+
// saturate every core with cover decodes and starve the UI thread. One shared
66+
// limiter caps total concurrent cover work.
7067
.fetcherCoroutineContext(coverDispatcher)
7168
.decoderCoroutineContext(coverDispatcher)
7269
.build()
7370

7471
private companion object {
75-
/**
76-
* Leave at least half the cores for everything else, and never go wide enough that cover
77-
* work can crowd out the UI thread on a big device.
78-
*/
7972
val coverDispatcher =
8073
Dispatchers.IO.limitedParallelism(
8174
(Runtime.getRuntime().availableProcessors() / 2).coerceIn(2, 4)

app/src/main/java/org/oxycblt/auxio/image/coil/CoverCompositionFetcher.kt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,8 @@ abstract class CoverCompositionFetcher(
108108
}
109109

110110
/**
111-
* Find the largest power-of-two downscale that keeps the cover at or above [target] pixels on
112-
* its shorter edge. A composition never draws a source cover larger than the composition
113-
* itself, so decoding beyond that is wasted heap -- and covers can be several thousand pixels
114-
* wide, which is enough to exhaust the heap once four of them are held at once.
111+
* The largest power-of-two downscale keeping the cover at or above [target] pixels. Four
112+
* full-resolution covers held at once is enough to exhaust the heap.
115113
*/
116114
private fun calculateInSampleSize(width: Int, height: Int, target: Int): Int {
117115
if (width <= 0 || height <= 0) return 1

app/src/main/java/org/oxycblt/auxio/list/recycler/FastScrollRecyclerView.kt

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,8 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
285285

286286
var listener: Listener? = null
287287

288-
// onPreDraw runs from an ItemDecoration, so it fires on every draw pass of the list --
289-
// scrolling or not. Deriving the popup text is not free (it re-reads the current sort out of
290-
// SharedPreferences, and depending on the sort allocates a Calendar or an ICU MeasureFormat),
291-
// so memoize it against the item it was derived from. The text only changes when the item
292-
// under the thumb does.
288+
// Deriving popup text is not free and onPreDraw runs on every draw pass, so memoize it
289+
// against the position it was derived from.
293290
private var popupDataPos = NO_POSITION
294291
private var popupDataText: String? = null
295292

@@ -457,11 +454,6 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
457454
invalidatePopupData()
458455
}
459456

460-
/**
461-
* Drop the memoized popup text so that it is re-derived on the next draw. Must be called
462-
* whenever the item at a given position may have changed, since the memo is keyed on position
463-
* alone.
464-
*/
465457
private fun invalidatePopupData() {
466458
popupDataPos = NO_POSITION
467459
popupDataText = null
@@ -578,24 +570,17 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
578570
return
579571
}
580572
val dy = newOffsetY - previousOffsetY
581-
// scrollBy makes the layout manager walk the requested distance one item at a time,
582-
// binding and laying out every row it passes over. That is fine for the small deltas a
583-
// slow drag produces, but a pixel of thumb travel maps to hundreds of pixels of content
584-
// on a library-sized list, so a single touch event can end up laying out thousands of
585-
// rows before the frame is even allowed to start. Past a couple of screenfuls, jump
586-
// straight to the target row instead of walking there.
573+
// scrollBy lays out every row it travels past, which on a large list can be thousands of
574+
// rows in one touch event. Past a couple of screenfuls, jump instead of walking.
587575
if (abs(dy) > height * MAX_SCROLL_BY_SCREENS && jumpToOffset(newOffsetY)) {
588576
return
589577
}
590578
scrollBy(0, max(dy.roundToInt(), -computeVerticalScrollOffset()))
591579
}
592580

593581
/**
594-
* Jump directly to the row nearest [offsetY], skipping everything in between.
595-
*
596-
* This is an estimate off the average laid-out row height, in the same spirit as the scroll
597-
* range the thumb position itself is derived from. Returns false if there is nothing laid out
598-
* to estimate from, in which case the caller should fall back to an incremental scroll.
582+
* Jump directly to the row nearest [offsetY], estimated off the average laid-out row height.
583+
* Returns false if there is nothing laid out to estimate from.
599584
*/
600585
private fun jumpToOffset(offsetY: Float): Boolean {
601586
val layoutManager = layoutManager as? LinearLayoutManager ?: return false
@@ -616,7 +601,6 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr
616601
return true
617602
}
618603

619-
/** The average height of the currently laid-out children, or 0 if there are none. */
620604
private fun averageRowHeight(): Int {
621605
val childCount = childCount
622606
if (childCount == 0) {

app/src/main/java/org/oxycblt/auxio/playback/PlaybackUtil.kt

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,7 @@ fun Long.formatDurationMs(isElapsed: Boolean) = msToSecs().formatDurationSecs(is
6363
private var popupDurationLocale: Locale? = null
6464
private var popupDurationFormat: MeasureFormat? = null
6565

66-
/**
67-
* The [MeasureFormat] used by [formatDurationMsPopup], memoized against the current locale.
68-
*
69-
* Building one is expensive enough to matter here, since fast-scrolling asks for a new popup string
70-
* every time the item under the thumb changes.
71-
*/
66+
/** The [MeasureFormat] used by [formatDurationMsPopup], memoized against the current locale. */
7267
private fun popupDurationFormat(): MeasureFormat {
7368
val locale = Locale.getDefault()
7469
var format = popupDurationFormat

app/src/main/java/org/oxycblt/auxio/playback/service/ExoPlaybackStateHolder.kt

Lines changed: 10 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -88,30 +88,15 @@ class ExoPlaybackStateHolder(
8888
private var currentSaveJob: Job? = null
8989
private var openAudioEffectSession = false
9090

91-
// --- QUEUE STATE ---
92-
// The queue is owned HERE, not by ExoPlayer. Handing the player a MediaItem per queue entry
93-
// does not scale: every item costs a MediaItem plus several player-internal holder objects,
94-
// which at library scale (a 40k-song "play from all" queue) measures out to ~77MB of Java
95-
// heap and transiently doubles on every reshuffle -- enough to exhaust the standard 256MB
96-
// heap once the library model itself is accounted for. Instead the player only ever holds a
97-
// small window of items around the current song (see below), and these fields are the source
98-
// of truth that resolveQueue() snapshots for the rest of the app.
99-
//
100-
// The representation mirrors RawQueue: heap is the queue in unshuffled order, mapping remaps
101-
// resolved positions to heap indices when shuffled (empty otherwise), and heapIndex is the
102-
// heap position of the current song.
91+
// The queue is owned here rather than by ExoPlayer, since a MediaItem per queue entry does
92+
// not scale to library-sized queues. The representation mirrors RawQueue.
10393
private val heap = mutableListOf<Song>()
10494
private val mapping = mutableListOf<Int>()
10595
private var heapIndex = -1
106-
107-
// The player no longer sees the full queue, so its repeat mode can't represent ALL on a
108-
// windowed queue and the app's repeat mode must be held here (see syncPlayerRepeatMode).
10996
private var repeatModeState = RepeatMode.NONE
11097

111-
// The player's current playlist, as resolved queue positions: the player's item i is the
112-
// queue's window[i]-th song in playback order. Kept contiguous (circularly contiguous when
113-
// wrapping under RepeatMode.ALL) and always containing the current song with margin on both
114-
// sides, so gapless preloading of the next song keeps working.
98+
// The player's playlist as resolved queue positions, a small contiguous run around the
99+
// current song. Wraps circularly under RepeatMode.ALL.
115100
private val window = ArrayDeque<Int>()
116101

117102
var sessionOngoing = false
@@ -244,8 +229,7 @@ class ExoPlaybackStateHolder(
244229
repeatModeState = repeatMode
245230
syncPlayerRepeatMode()
246231
updatePauseOnRepeat()
247-
// The repeat mode decides whether the window wraps around the queue edges, so it may
248-
// need to be reshaped.
232+
// The repeat mode decides whether the window wraps, so it may need to be reshaped.
249233
slideWindow()
250234
playbackManager.ack(this, StateAck.RepeatModeChanged)
251235
deferSave()
@@ -284,7 +268,6 @@ class ExoPlaybackStateHolder(
284268
}
285269
mapping.clear()
286270
if (shuffled) {
287-
// Anchor the current song to the front of the new shuffled order.
288271
mapping.addAll(shuffledMapping(anchor = heapIndex))
289272
}
290273
syncPlayerRepeatMode()
@@ -350,13 +333,11 @@ class ExoPlaybackStateHolder(
350333
val insertAt = heapIndex + 1
351334
heap.addAll(insertAt, songs)
352335
if (isShuffled) {
353-
// Inserting into the heap shifted every heap index at or above the insertion point.
354336
for (i in mapping.indices) {
355337
if (mapping[i] >= insertAt) {
356338
mapping[i] += songs.size
357339
}
358340
}
359-
// Then place the new songs directly after the current position in play order.
360341
mapping.addAll(resolvedIndex() + 1, List(songs.size) { insertAt + it })
361342
}
362343
refreshWindow()
@@ -383,7 +364,6 @@ class ExoPlaybackStateHolder(
383364
return
384365
}
385366
if (isShuffled) {
386-
// Play order is defined by the mapping; the heap (and thus heapIndex) is untouched.
387367
mapping.add(to, mapping.removeAt(from))
388368
} else {
389369
heap.add(to, heap.removeAt(from))
@@ -425,8 +405,7 @@ class ExoPlaybackStateHolder(
425405
player.clearMediaItems()
426406
}
427407
songWillChange -> {
428-
// Match ExoPlayer's own removal behavior: playback moves to the song that now
429-
// occupies the removed song's position (or the last song if it was the tail).
408+
// Playback moves to the song now occupying the removed song's position.
430409
heapIndex = heapIndexAt(at.coerceAtMost(heap.size - 1))
431410
hardResetWindow()
432411
}
@@ -544,7 +523,6 @@ class ExoPlaybackStateHolder(
544523
return (0 until size).toList()
545524
}
546525
return if (repeatModeState == RepeatMode.ALL) {
547-
// Positions stay unique because the window is strictly smaller than the queue.
548526
(center - WINDOW_RADIUS..center + WINDOW_RADIUS).map { ((it % size) + size) % size }
549527
} else {
550528
val start = (center - WINDOW_RADIUS).coerceAtLeast(0)
@@ -572,7 +550,7 @@ class ExoPlaybackStateHolder(
572550
}
573551
}
574552

575-
/** Replace the playlist outright, (re)starting playback of the current song. */
553+
/** Replace the playlist outright, (re)starting the current song. */
576554
private fun hardResetWindow() {
577555
val desired = computeWindowPositions(resolvedIndex())
578556
window.clear()
@@ -585,11 +563,7 @@ class ExoPlaybackStateHolder(
585563
player.seekTo(desired.indexOf(resolvedIndex()), C.TIME_UNSET)
586564
}
587565

588-
/**
589-
* Rebuild the playlist around the currently-playing item after a queue mutation, without
590-
* interrupting it. The songs behind the window's positions may have changed, so everything
591-
* except the current item is replaced.
592-
*/
566+
/** Rebuild the playlist around the currently-playing item without interrupting it. */
593567
private fun refreshWindow() {
594568
val current = resolvedIndex()
595569
val desired = computeWindowPositions(current)
@@ -600,7 +574,6 @@ class ExoPlaybackStateHolder(
600574
}
601575
val playing = player.currentMediaItem?.song
602576
if (playing == null || playing != heap.getOrNull(heapIndex)) {
603-
// Player is empty or out of sync with our state; start over.
604577
hardResetWindow()
605578
return
606579
}
@@ -639,8 +612,7 @@ class ExoPlaybackStateHolder(
639612
return
640613
}
641614
val desiredSet = desired.toHashSet()
642-
// Trim stale edges. This can never remove the current item, since the window is always
643-
// computed around it.
615+
// Never removes the current item, since the window is always computed around it.
644616
while (window.isNotEmpty() && window.first() !in desiredSet) {
645617
player.removeMediaItem(0)
646618
window.removeFirst()
@@ -671,7 +643,7 @@ class ExoPlaybackStateHolder(
671643
}
672644
}
673645

674-
/** Adopt the player's current item as the current song after the player moved on its own. */
646+
/** Adopt the player's current item after the player moved on its own. */
675647
private fun syncIndexFromPlayer() {
676648
val resolved = window.getOrNull(player.currentMediaItemIndex) ?: return
677649
heapIndex = heapIndexAt(resolved)

app/src/main/java/org/oxycblt/auxio/playback/service/MediaSessionHolder.kt

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,7 @@ private constructor(
101101
val notification: ForegroundServiceNotification
102102
get() = _notification
103103

104-
// The bounds of the queue window currently published to the media session, as absolute
105-
// indices into the full queue.
104+
// Bounds of the queue window currently published, as absolute queue indices.
106105
private var queueWindowStart = 0
107106
private var queueWindowEnd = 0
108107

@@ -137,9 +136,7 @@ private constructor(
137136

138137
override fun onIndexMoved(index: Int) {
139138
updateMediaMetadata(playbackManager.currentSong, playbackManager.parent)
140-
// Only a window of the queue is published, so it has to follow playback as the index
141-
// walks out of it. If the whole queue already fits, the window never changes and this
142-
// does nothing.
139+
// The published queue window has to follow playback as the index walks out of it.
143140
val queue = playbackManager.queue
144141
if (
145142
max(0, index - QUEUE_WINDOW_RADIUS) != queueWindowStart ||
@@ -327,12 +324,9 @@ private constructor(
327324
* @param queue The current queue to upload.
328325
*/
329326
private fun updateQueue(queue: List<Song>, index: Int) {
330-
// setQueue marshals the entire list into a Parcel and hands it to the system, and each
331-
// item first has to have its description resolved (several strings, a Uri and a Bundle
332-
// apiece). Doing that for a library-sized queue blocks the main thread for seconds and
333-
// risks TransactionTooLargeException. Consumers of this queue (Android Auto, bluetooth
334-
// head units) only ever show a handful of surrounding items, so publish a window around
335-
// the current song instead of the whole thing.
327+
// setQueue parcels the whole list to the system, which for a library-sized queue blocks
328+
// the main thread for seconds and risks TransactionTooLargeException. Publish a window
329+
// around the current song instead.
336330
val start = max(0, index - QUEUE_WINDOW_RADIUS)
337331
val end = min(queue.size, index + QUEUE_WINDOW_RADIUS + 1)
338332
val queueItems =
@@ -342,9 +336,7 @@ private constructor(
342336
context,
343337
{ putInt(MediaSessionInterface.KEY_QUEUE_POS, i) },
344338
)
345-
// Store the item index so we can then use the analogous index in the
346-
// playback state. This stays absolute so that skip-to-item and queue item
347-
// removal still address the right song.
339+
// Ids stay absolute so skip-to-item still addresses the right song.
348340
MediaSessionCompat.QueueItem(description, i.toLong())
349341
}
350342
queueWindowStart = start

app/src/main/java/org/oxycblt/auxio/widgets/WidgetComponent.kt

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,9 @@ private constructor(
119119
}
120120
}
121121

122-
// Never ORIGINAL here. This bitmap ends up in a RemoteViews that has to be
123-
// marshalled to the launcher, and the widget draws it at a couple of hundred
124-
// pixels at most, so decoding native-resolution art wastes a large amount of
125-
// memory and CPU per update -- and update() runs on nearly every playback
126-
// event, including progression changes. The corner radius applied below is an
127-
// absolute pixel value sized for the widget, so bounding the bitmap also
128-
// makes that radius land at roughly the intended proportion.
122+
// Bounded, not ORIGINAL: this bitmap is marshalled into a RemoteViews and
123+
// drawn at a couple hundred pixels, and update() runs on nearly every
124+
// playback event.
129125
return builder.size(COVER_MAX_PX, COVER_MAX_PX).transformations(transformations)
130126
}
131127

@@ -195,11 +191,6 @@ private constructor(
195191
)
196192

197193
private companion object {
198-
/**
199-
* Upper bound for the cover bitmap handed to the widget. Comfortably above any widget cover
200-
* a phone will actually draw, and 1MB as ARGB_8888 rather than the tens of MB a
201-
* native-resolution decode can reach.
202-
*/
203194
const val COVER_MAX_PX = 512
204195
}
205196
}

musikr/src/main/java/org/oxycblt/musikr/cache/db/DBCache.kt

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,8 @@ class DBCache private constructor(private val readDao: CacheReadDao) : Cache {
4141
private val mappingLock = Mutex()
4242

4343
/**
44-
* Drop the in-memory copy of the cache table.
45-
*
46-
* [read] memoizes the entire table so that a load doesn't issue a query per file, but that copy
47-
* holds every tag of every song and is only useful while a load is in progress. Holding it for
48-
* the lifetime of the process is a second, complete copy of the library's metadata sitting
49-
* alongside the library itself, which is enough to exhaust the heap on large libraries.
44+
* Drop the in-memory copy of the cache table. It is only useful while a load is in progress,
45+
* and holding it longer duplicates the entire library's metadata in memory.
5046
*/
5147
suspend fun release() {
5248
mappingLock.withLock { mapping = null }
@@ -166,7 +162,6 @@ private constructor(private val inner: DBCache, private val writeDao: CacheWrite
166162

167163
override suspend fun cleanup(excluding: List<CachedFile>) {
168164
writeDao.deleteExcludingUris(excluding.mapTo(mutableSetOf()) { it.file.uri.toString() })
169-
// Cleanup terminates a load, so the read mapping is dead weight from here on.
170165
inner.release()
171166
}
172167

0 commit comments

Comments
 (0)