From 07127403ff22f580ee37607978d0e2e3ddfa9984 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Wed, 1 Jun 2022 15:49:11 -0600 Subject: [PATCH] music: fix minor indexer issues Further refine the Indexer and ExoPlayerBackend implementations. These fixes were primarily focused on ensuring stable grouping through stable sorting order, and more graceful handling of edge cases in ExoPlayerBackend. --- CHANGELOG.md | 4 + .../auxio/detail/AlbumDetailFragment.kt | 2 +- .../home/fastscroll/FastScrollRecyclerView.kt | 2 +- .../auxio/music/indexer/ExoPlayerBackend.kt | 113 +++++++++++------- .../oxycblt/auxio/music/indexer/Indexer.kt | 25 ++-- .../org/oxycblt/auxio/ui/BottomSheetLayout.kt | 25 ++-- .../org/oxycblt/auxio/util/FrameworkUtil.kt | 12 +- 7 files changed, 107 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d101a679e..cfc7f9bd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ #### What's Fixed - Fixed crash when seeking to the end of a track as the track changed to a track with a lower duration +- Fixed regression where GadgetBridge media controls would no longer work + +#### Dev/Meta +- Switched from `LiveData` to `StateFlow` ## v2.3.0 diff --git a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt index ae6b85a10..4e44750aa 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt @@ -183,7 +183,7 @@ class AlbumDetailFragment : DetailFragment(), AlbumDetailAdapter.Listener { // If the recyclerview can scroll, its certain that it will have to scroll to // correctly center the playing item, so make sure that the Toolbar is lifted in // that case. - binding.detailAppbar.isLifted = binding.detailRecycler.canScroll() + binding.detailAppbar.isLifted = binding.detailRecycler.canScroll } } } diff --git a/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt b/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt index 2f9c01a04..1effcfb58 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/fastscroll/FastScrollRecyclerView.kt @@ -289,7 +289,7 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr } private fun updateScrollbarState() { - if (!canScroll() || childCount == 0) { + if (!canScroll || childCount == 0) { return } diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt index 02bae60c5..dbb7ff169 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/indexer/ExoPlayerBackend.kt @@ -82,6 +82,7 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { runningTasks[index] = task + break } } @@ -91,8 +92,6 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { // Spin until all tasks are complete } - // TODO: Stabilize sorting order - return songs } @@ -122,25 +121,78 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { } private fun completeAudio(audio: MediaStoreBackend.Audio, metadata: Metadata) { + if (metadata.length() == 0) { + return + } + + // ExoPlayer only exposes ID3v2 and Vorbis metadata, which constitutes the vast majority + // of audio formats. Some formats (like FLAC) can contain both ID3v2 and vorbis tags, but + // this isn't too big of a deal, as we generally let the "source of truth" for metadata + // be the last instance of a particular tag in a file. for (i in 0 until metadata.length()) { - // We only support two formats as it stands: - // - ID3v2 text frames - // - Vorbis comments - // TODO: Formats like flac can have both ID3v2 and OGG tags, so we might want to split - // up this logic. - when (val tag = metadata.get(i)) { - is TextInformationFrame -> - if (tag.value.isNotEmpty()) { - handleId3v2TextFrame(tag.id.sanitize(), tag.value.sanitize(), audio) - } - is VorbisComment -> - if (tag.value.isNotEmpty()) { - handleVorbisComment(tag.key.sanitize(), tag.value.sanitize(), audio) - } + when (val tag = metadata[i]) { + is TextInformationFrame -> populateWithId3v2(audio, tag) + is VorbisComment -> populateWithVorbis(audio, tag) } } } + private fun populateWithId3v2(audio: MediaStoreBackend.Audio, frame: TextInformationFrame) { + val id = frame.id.sanitize() + val value = frame.value.sanitize() + if (value.isEmpty()) { + return + } + + when (id) { + // Title + "TIT2" -> audio.title = value + // Track, as NN/TT + "TRCK" -> value.no?.let { audio.track = it } + // Disc + "TPOS" -> value.no?.let { audio.disc = it } + // ID3v2.3 year, should be digits + "TYER" -> value.toIntOrNull()?.let { audio.year = it } + // ID3v2.4 year, parse as ISO-8601 + "TDRC" -> value.iso8601year?.let { audio.year = it } + // Album + "TALB" -> audio.album = value + // Artist + "TPE1" -> audio.artist = value + // Album artist + "TPE2" -> audio.albumArtist = value + // Genre, with the weird ID3v2 rules + "TCON" -> audio.genre = value + } + } + + private fun populateWithVorbis(audio: MediaStoreBackend.Audio, comment: VorbisComment) { + val key = comment.key.sanitize() + val value = comment.value.sanitize() + if (value.isEmpty()) { + return + } + + when (key) { + // Title + "TITLE" -> audio.title = value + // Track, presumably as NN/TT + "TRACKNUMBER" -> value.no?.let { audio.track = it } + // Disc, presumably as NN/TT + "DISCNUMBER" -> value.no?.let { audio.disc = it } + // Date, presumably as ISO-8601 + "DATE" -> value.iso8601year?.let { audio.year = it } + // Album + "ALBUM" -> audio.album = value + // Artist + "ARTIST" -> audio.artist = value + // Album artist + "ALBUMARTIST" -> audio.albumArtist = value + // Genre, assumed that ID3v2 rules will apply here too. + "GENRE" -> audio.genre = value + } + } + /** * Copies and sanitizes this string under the assumption that it is UTF-8. * @@ -155,35 +207,6 @@ class ExoPlayerBackend(private val inner: MediaStoreBackend) : Indexer.Backend { */ private fun String.sanitize() = String(encodeToByteArray()) - private fun handleId3v2TextFrame(id: String, value: String, audio: MediaStoreBackend.Audio) { - // It's assumed that duplicate frames are eliminated by ExoPlayer's metadata parser. - when (id) { - "TIT2" -> audio.title = value // Title - "TRCK" -> value.no?.let { audio.track = it } // Track, as NN/TT - "TPOS" -> value.no?.let { audio.disc = it } // Disc, as NN/TT - "TYER" -> value.toIntOrNull()?.let { audio.year = it } // ID3v2.3 year, should be digits - "TDRC" -> value.iso8601year?.let { audio.year = it } // ID3v2.4 date, parse year field - "TALB" -> audio.album = value // Album - "TPE1" -> audio.artist = value // Artist - "TPE2" -> audio.albumArtist = value // Album artist - "TCON" -> audio.genre = value // Genre, with the weird ID3v2 rules - } - } - - private fun handleVorbisComment(key: String, value: String, audio: MediaStoreBackend.Audio) { - // It's assumed that duplicate tags are eliminated by ExoPlayer's metadata parser. - when (key) { - "TITLE" -> audio.title = value // Title, presumably as NN/TT - "TRACKNUMBER" -> value.no?.let { audio.track = it } // Track, presumably as NN/TT - "DISCNUMBER" -> value.no?.let { audio.disc = it } // Disc, presumably as NN/TT - "DATE" -> value.iso8601year?.let { audio.year = it } // Date, presumably as ISO-8601 - "ALBUM" -> audio.album = value // Album - "ARTIST" -> audio.artist = value // Artist - "ALBUMARTIST" -> audio.albumArtist = value // Album artist - "GENRE" -> audio.genre = value // Genre, assumed that ID3v2 rules will apply here too. - } - } - companion object { /** The amount of tasks this backend can run efficiently at once. */ private const val TASK_CAPACITY = 8 diff --git a/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt b/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt index 91d9fa85f..8dfb0a4b4 100644 --- a/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt +++ b/app/src/main/java/org/oxycblt/auxio/music/indexer/Indexer.kt @@ -108,16 +108,21 @@ object Indexer { // Deduplicate songs to prevent (most) deformed music clones songs = - songs.distinctBy { - it.rawName to - it._albumName to - it._artistName to - it._albumArtistName to - it._genreName to - it.track to - it.disc to - it.durationMs - } + songs + .distinctBy { + it.rawName to + it._albumName to + it._artistName to + it._albumArtistName to + it._genreName to + it.track to + it.disc to + it.durationMs + } + .toMutableList() + + // Ensure that sorting order is consistent so that grouping is also consistent. + Sort.ByName(true).songsInPlace(songs) logD("Successfully loaded ${songs.size} songs") diff --git a/app/src/main/java/org/oxycblt/auxio/ui/BottomSheetLayout.kt b/app/src/main/java/org/oxycblt/auxio/ui/BottomSheetLayout.kt index 798409e02..4bb7ba45b 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/BottomSheetLayout.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/BottomSheetLayout.kt @@ -151,10 +151,6 @@ constructor(context: Context, attrs: AttributeSet? = null, defStyle: Int = 0) : private var initMotionY = 0f private val tRect = Rect() - /** See [isDragging] */ - private val dragStateField = - ViewDragHelper::class.java.getDeclaredField("mDragState").apply { isAccessible = true } - init { setWillNotDraw(false) } @@ -487,7 +483,7 @@ constructor(context: Context, attrs: AttributeSet? = null, defStyle: Int = 0) : // want to vendor ViewDragHelper so I just do reflection instead. val state = try { - dragStateField.get(this) + DRAG_STATE_FIELD.get(this) } catch (e: Exception) { ViewDragHelper.STATE_IDLE } @@ -540,7 +536,8 @@ constructor(context: Context, attrs: AttributeSet? = null, defStyle: Int = 0) : // desynchronizing [reminder that this view also applies the bottom window inset] // and we can't apply padding to the whole container layout since that would adjust // the size of the panel view. This seems to be the least obtrusive way to do this. - lastInsets?.systemBarInsetsCompat?.let { bars -> + lastInsets?.let { insets -> + val bars = insets.systemBarInsetsCompat val params = layoutParams as MarginLayoutParams val oldTopMargin = params.topMargin @@ -586,10 +583,9 @@ constructor(context: Context, attrs: AttributeSet? = null, defStyle: Int = 0) : get() = panelState != PanelState.HIDDEN && isEnabled private inner class DragHelperCallback : ViewDragHelper.Callback() { - override fun tryCaptureView(child: View, pointerId: Int): Boolean { - // Only capture on a fully expanded panel view - return child === containerView && panelOffset >= 0 - } + // Only capture on a fully expanded panel view + override fun tryCaptureView(child: View, pointerId: Int) = + child === containerView && panelOffset >= 0 override fun onViewDragStateChanged(state: Int) { if (state == ViewDragHelper.STATE_IDLE) { @@ -655,9 +651,7 @@ constructor(context: Context, attrs: AttributeSet? = null, defStyle: Int = 0) : invalidate() } - override fun getViewVerticalDragRange(child: View): Int { - return panelRange - } + override fun getViewVerticalDragRange(child: View) = panelRange override fun clampViewPositionVertical(child: View, top: Int, dy: Int): Int { val collapsedTop = computePanelTopPosition(0f) @@ -668,7 +662,10 @@ constructor(context: Context, attrs: AttributeSet? = null, defStyle: Int = 0) : companion object { private val INIT_PANEL_STATE = PanelState.HIDDEN + private val DRAG_STATE_FIELD = + ViewDragHelper::class.java.getDeclaredField("mDragState").apply { isAccessible = true } + private const val MIN_FLING_VEL = 400 - private const val KEY_PANEL_STATE = BuildConfig.APPLICATION_ID + ".key.panel_state" + private const val KEY_PANEL_STATE = BuildConfig.APPLICATION_ID + ".key.PANEL_STATE" } } diff --git a/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt b/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt index a0a841078..8d4e73051 100644 --- a/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/util/FrameworkUtil.kt @@ -58,10 +58,9 @@ fun View.disableDropShadowCompat() { * Determines if the point given by [x] and [y] falls within this view. * @param minTouchTargetSize The minimum touch size, independent of the view's size (Optional) */ -fun View.isUnder(x: Float, y: Float, minTouchTargetSize: Int = 0): Boolean { - return isUnderImpl(x, left, right, (parent as View).width, minTouchTargetSize) && +fun View.isUnder(x: Float, y: Float, minTouchTargetSize: Int = 0) = + isUnderImpl(x, left, right, (parent as View).width, minTouchTargetSize) && isUnderImpl(y, top, bottom, (parent as View).height, minTouchTargetSize) -} private fun isUnderImpl( position: Float, @@ -143,14 +142,17 @@ fun RecyclerView.applySpans(shouldBeFullWidth: ((Int) -> Boolean)? = null) { } /** Returns whether a recyclerview can scroll. */ -fun RecyclerView.canScroll(): Boolean = computeVerticalScrollRange() > height +val RecyclerView.canScroll: Boolean + get() = computeVerticalScrollRange() > height /** Converts this color to a single-color [ColorStateList]. */ val @receiver:ColorRes Int.stateList get() = ColorStateList.valueOf(this) /** Require the fragment is attached to an activity. */ -fun Fragment.requireAttached() = check(!isDetached) { "Fragment is detached from activity" } +fun Fragment.requireAttached() { + check(!isDetached) { "Fragment is detached from activity" } +} /** * Launches [block] in a lifecycle-aware coroutine once [state] is reached. This is primarily a