music: rework indexer thread safety
Move the switch from IO to Main to within Indexer itself, through withContext. This is much easier to work with than the previous system of a separate "update" coroutine, which isn't really needed anymore given that I no longer need to do IO work when sanitizing the playback state.
This commit is contained in:
OxygenCobalt
parent
29cc680c34
commit
94f2d28936
22 changed files with 102 additions and 73 deletions
|
|
@ -77,7 +77,7 @@
|
|||
extracting metadata, and constructing the music library.
|
||||
-->
|
||||
<service
|
||||
android:name=".music.IndexerService"
|
||||
android:name=".music.system.IndexerService"
|
||||
android:foregroundServiceType="dataSync"
|
||||
android:icon="@mipmap/ic_launcher"
|
||||
android:exported="false"
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ import androidx.appcompat.app.AppCompatDelegate
|
|||
import androidx.core.view.WindowCompat
|
||||
import androidx.core.view.updatePadding
|
||||
import org.oxycblt.auxio.databinding.ActivityMainBinding
|
||||
import org.oxycblt.auxio.music.IndexerService
|
||||
import org.oxycblt.auxio.music.system.IndexerService
|
||||
import org.oxycblt.auxio.playback.PlaybackViewModel
|
||||
import org.oxycblt.auxio.playback.system.PlaybackService
|
||||
import org.oxycblt.auxio.settings.Settings
|
||||
|
|
|
|||
|
|
@ -33,9 +33,9 @@ import org.oxycblt.auxio.detail.recycler.SortHeader
|
|||
import org.oxycblt.auxio.music.Album
|
||||
import org.oxycblt.auxio.music.Artist
|
||||
import org.oxycblt.auxio.music.Genre
|
||||
import org.oxycblt.auxio.music.MimeType
|
||||
import org.oxycblt.auxio.music.MusicStore
|
||||
import org.oxycblt.auxio.music.Song
|
||||
import org.oxycblt.auxio.music.system.MimeType
|
||||
import org.oxycblt.auxio.settings.Settings
|
||||
import org.oxycblt.auxio.ui.Sort
|
||||
import org.oxycblt.auxio.ui.recycler.Header
|
||||
|
|
|
|||
|
|
@ -46,10 +46,10 @@ import org.oxycblt.auxio.home.list.SongListFragment
|
|||
import org.oxycblt.auxio.music.Album
|
||||
import org.oxycblt.auxio.music.Artist
|
||||
import org.oxycblt.auxio.music.Genre
|
||||
import org.oxycblt.auxio.music.Indexer
|
||||
import org.oxycblt.auxio.music.IndexerViewModel
|
||||
import org.oxycblt.auxio.music.Music
|
||||
import org.oxycblt.auxio.music.Song
|
||||
import org.oxycblt.auxio.music.system.Indexer
|
||||
import org.oxycblt.auxio.music.system.IndexerViewModel
|
||||
import org.oxycblt.auxio.playback.PlaybackViewModel
|
||||
import org.oxycblt.auxio.ui.DisplayMode
|
||||
import org.oxycblt.auxio.ui.MainNavigationAction
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ import android.content.Context
|
|||
import android.net.Uri
|
||||
import android.provider.MediaStore
|
||||
import org.oxycblt.auxio.R
|
||||
import org.oxycblt.auxio.music.system.MimeType
|
||||
import org.oxycblt.auxio.music.system.Path
|
||||
import org.oxycblt.auxio.ui.recycler.Item
|
||||
import org.oxycblt.auxio.util.unlikelyToBeNull
|
||||
|
||||
|
|
@ -30,9 +32,6 @@ import org.oxycblt.auxio.util.unlikelyToBeNull
|
|||
|
||||
/** [Item] variant that represents a music item. */
|
||||
sealed class Music : Item() {
|
||||
// TODO: Split ID into an ID derived from all fields and a persistent ID derived from stable
|
||||
// fields
|
||||
|
||||
/** The raw name of this item. Null if unknown. */
|
||||
abstract val rawName: String?
|
||||
|
||||
|
|
|
|||
|
|
@ -23,8 +23,18 @@ import android.provider.OpenableColumns
|
|||
import org.oxycblt.auxio.util.contentResolverSafe
|
||||
|
||||
/**
|
||||
* The main storage for music items. The items themselves are located in a [Library], however this
|
||||
* might not be available at all times.
|
||||
* The main storage for music items.
|
||||
*
|
||||
* Whereas other apps load music from MediaStore as it is shown, Auxio does not do that, as it
|
||||
* cripples any kind of advanced metadata functionality. Instead, Auxio loads all music into a
|
||||
* in-memory relational data-structure called [Library]. This costs more memory-wise, but is also
|
||||
* much more sensible.
|
||||
*
|
||||
* The only other, memory-efficient option is to create our own hybrid database that leverages both
|
||||
* a typical DB and a mem-cache, like Vinyl. But why would we do that when I've encountered no real
|
||||
* issues with the current system.
|
||||
*
|
||||
* [Library] may not be available at all times, so leveraging [Callback] is recommended.
|
||||
*
|
||||
* @author OxygenCobalt
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -24,12 +24,12 @@ import com.google.android.exoplayer2.MetadataRetriever
|
|||
import com.google.android.exoplayer2.metadata.Metadata
|
||||
import com.google.android.exoplayer2.metadata.id3.TextInformationFrame
|
||||
import com.google.android.exoplayer2.metadata.vorbis.VorbisComment
|
||||
import org.oxycblt.auxio.music.Indexer
|
||||
import org.oxycblt.auxio.music.Song
|
||||
import org.oxycblt.auxio.music.audioUri
|
||||
import org.oxycblt.auxio.music.id3GenreName
|
||||
import org.oxycblt.auxio.music.iso8601year
|
||||
import org.oxycblt.auxio.music.plainTrackNo
|
||||
import org.oxycblt.auxio.music.system.Indexer
|
||||
import org.oxycblt.auxio.music.trackDiscNo
|
||||
import org.oxycblt.auxio.music.year
|
||||
import org.oxycblt.auxio.util.logD
|
||||
|
|
|
|||
|
|
@ -27,20 +27,20 @@ import androidx.annotation.RequiresApi
|
|||
import androidx.core.database.getIntOrNull
|
||||
import androidx.core.database.getStringOrNull
|
||||
import java.io.File
|
||||
import org.oxycblt.auxio.music.Directory
|
||||
import org.oxycblt.auxio.music.Indexer
|
||||
import org.oxycblt.auxio.music.MimeType
|
||||
import org.oxycblt.auxio.music.Path
|
||||
import org.oxycblt.auxio.music.Song
|
||||
import org.oxycblt.auxio.music.albumCoverUri
|
||||
import org.oxycblt.auxio.music.audioUri
|
||||
import org.oxycblt.auxio.music.directoryCompat
|
||||
import org.oxycblt.auxio.music.id3GenreName
|
||||
import org.oxycblt.auxio.music.mediaStoreVolumeNameCompat
|
||||
import org.oxycblt.auxio.music.packedDiscNo
|
||||
import org.oxycblt.auxio.music.packedTrackNo
|
||||
import org.oxycblt.auxio.music.queryCursor
|
||||
import org.oxycblt.auxio.music.storageVolumesCompat
|
||||
import org.oxycblt.auxio.music.system.Directory
|
||||
import org.oxycblt.auxio.music.system.Indexer
|
||||
import org.oxycblt.auxio.music.system.MimeType
|
||||
import org.oxycblt.auxio.music.system.Path
|
||||
import org.oxycblt.auxio.music.system.directoryCompat
|
||||
import org.oxycblt.auxio.music.system.mediaStoreVolumeNameCompat
|
||||
import org.oxycblt.auxio.music.system.storageVolumesCompat
|
||||
import org.oxycblt.auxio.music.trackDiscNo
|
||||
import org.oxycblt.auxio.music.useQuery
|
||||
import org.oxycblt.auxio.settings.Settings
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ package org.oxycblt.auxio.music.dirs
|
|||
|
||||
import android.content.Context
|
||||
import org.oxycblt.auxio.databinding.ItemMusicDirBinding
|
||||
import org.oxycblt.auxio.music.Directory
|
||||
import org.oxycblt.auxio.music.system.Directory
|
||||
import org.oxycblt.auxio.ui.recycler.BackingData
|
||||
import org.oxycblt.auxio.ui.recycler.BindingViewHolder
|
||||
import org.oxycblt.auxio.ui.recycler.MonoAdapter
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@
|
|||
|
||||
package org.oxycblt.auxio.music.dirs
|
||||
|
||||
import org.oxycblt.auxio.music.Directory
|
||||
import org.oxycblt.auxio.music.system.Directory
|
||||
|
||||
/** Represents a the configuration for the "Folder Management" setting */
|
||||
data class MusicDirs(val dirs: List<Directory>, val shouldInclude: Boolean)
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ import androidx.core.view.isVisible
|
|||
import org.oxycblt.auxio.BuildConfig
|
||||
import org.oxycblt.auxio.R
|
||||
import org.oxycblt.auxio.databinding.DialogMusicDirsBinding
|
||||
import org.oxycblt.auxio.music.Directory
|
||||
import org.oxycblt.auxio.music.system.Directory
|
||||
import org.oxycblt.auxio.settings.Settings
|
||||
import org.oxycblt.auxio.ui.fragment.ViewBindingDialogFragment
|
||||
import org.oxycblt.auxio.util.context
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@
|
|||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package org.oxycblt.auxio.music
|
||||
package org.oxycblt.auxio.music.system
|
||||
|
||||
import android.Manifest
|
||||
import android.content.Context
|
||||
|
|
@ -24,7 +24,14 @@ import android.database.Cursor
|
|||
import android.os.Build
|
||||
import androidx.core.content.ContextCompat
|
||||
import kotlin.coroutines.cancellation.CancellationException
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.withContext
|
||||
import org.oxycblt.auxio.BuildConfig
|
||||
import org.oxycblt.auxio.music.Album
|
||||
import org.oxycblt.auxio.music.Artist
|
||||
import org.oxycblt.auxio.music.Genre
|
||||
import org.oxycblt.auxio.music.MusicStore
|
||||
import org.oxycblt.auxio.music.Song
|
||||
import org.oxycblt.auxio.music.backend.Api21MediaStoreBackend
|
||||
import org.oxycblt.auxio.music.backend.Api29MediaStoreBackend
|
||||
import org.oxycblt.auxio.music.backend.Api30MediaStoreBackend
|
||||
|
|
@ -34,6 +41,7 @@ import org.oxycblt.auxio.ui.Sort
|
|||
import org.oxycblt.auxio.util.logD
|
||||
import org.oxycblt.auxio.util.logE
|
||||
import org.oxycblt.auxio.util.logW
|
||||
import org.oxycblt.auxio.util.requireBackgroundThread
|
||||
|
||||
/**
|
||||
* Auxio's media indexer.
|
||||
|
|
@ -122,7 +130,9 @@ class Indexer {
|
|||
this.callback = null
|
||||
}
|
||||
|
||||
fun index(context: Context) {
|
||||
suspend fun index(context: Context) {
|
||||
requireBackgroundThread()
|
||||
|
||||
val generation = synchronized(this) { ++currentGeneration }
|
||||
|
||||
val notGranted =
|
||||
|
|
@ -180,11 +190,7 @@ class Indexer {
|
|||
|
||||
@Synchronized
|
||||
private fun emitIndexing(indexing: Indexing?, generation: Long) {
|
||||
if (currentGeneration != generation) {
|
||||
// Not the running task anymore, cancel this co-routine. This allows a yield-like
|
||||
// behavior to be implemented in a far cheaper manner for each backend.
|
||||
throw CancellationException()
|
||||
}
|
||||
checkGenerationImpl(generation)
|
||||
|
||||
if (indexing == indexingState) {
|
||||
// Ignore redundant states used when the backends just want to check for
|
||||
|
|
@ -203,24 +209,34 @@ class Indexer {
|
|||
callback?.onIndexerStateChanged(state)
|
||||
}
|
||||
|
||||
@Synchronized
|
||||
private fun emitCompletion(response: Response, generation: Long) {
|
||||
if (currentGeneration != generation) {
|
||||
// Not the running task anymore, cancel this co-routine. This allows a yield-like
|
||||
// behavior to be implemented in a far cheaper manner for each backend.
|
||||
throw CancellationException()
|
||||
}
|
||||
private suspend fun emitCompletion(response: Response, generation: Long) {
|
||||
synchronized(this) {
|
||||
checkGenerationImpl(generation)
|
||||
|
||||
// Do not check for redundancy here, as we actually need to notify a switch
|
||||
// from Indexing -> Complete and not Indexing -> Indexing or Complete -> Complete.
|
||||
|
||||
lastResponse = response
|
||||
indexingState = null
|
||||
}
|
||||
|
||||
val state = State.Complete(response)
|
||||
|
||||
// Swap to the Main thread so that downstream callbacks don't crash from being on
|
||||
// a background thread. Does not occur in emitIndexing due to efficiency reasons.
|
||||
withContext(Dispatchers.Main) {
|
||||
controller?.onIndexerStateChanged(state)
|
||||
callback?.onIndexerStateChanged(state)
|
||||
}
|
||||
}
|
||||
|
||||
private fun checkGenerationImpl(generation: Long) {
|
||||
if (currentGeneration != generation) {
|
||||
// Not the running task anymore, cancel this co-routine. This allows a yield-like
|
||||
// behavior to be implemented in a far cheaper manner for each backend.
|
||||
throw CancellationException()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Run the proper music loading process. [generation] must be a truthful value of the generation
|
||||
|
|
@ -15,7 +15,7 @@
|
|||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package org.oxycblt.auxio.music
|
||||
package org.oxycblt.auxio.music.system
|
||||
|
||||
import android.app.NotificationChannel
|
||||
import android.app.NotificationManager
|
||||
|
|
@ -15,7 +15,7 @@
|
|||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package org.oxycblt.auxio.music
|
||||
package org.oxycblt.auxio.music.system
|
||||
|
||||
import android.app.Service
|
||||
import android.content.Intent
|
||||
|
|
@ -28,6 +28,7 @@ import kotlinx.coroutines.Job
|
|||
import kotlinx.coroutines.launch
|
||||
import org.oxycblt.auxio.IntegerTable
|
||||
import org.oxycblt.auxio.R
|
||||
import org.oxycblt.auxio.music.MusicStore
|
||||
import org.oxycblt.auxio.playback.state.PlaybackStateManager
|
||||
import org.oxycblt.auxio.settings.Settings
|
||||
import org.oxycblt.auxio.util.logD
|
||||
|
|
@ -51,7 +52,6 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
|
|||
|
||||
private val serviceJob = Job()
|
||||
private val indexScope = CoroutineScope(serviceJob + Dispatchers.IO)
|
||||
private val updateScope = CoroutineScope(serviceJob + Dispatchers.Main)
|
||||
|
||||
private val playbackManager = PlaybackStateManager.getInstance()
|
||||
private lateinit var settings: Settings
|
||||
|
|
@ -108,31 +108,27 @@ class IndexerService : Service(), Indexer.Controller, Settings.Callback {
|
|||
|
||||
val newLibrary = state.response.library
|
||||
|
||||
// Load was completed successfully. However, we still need to do some
|
||||
// extra work to update the app's state.
|
||||
updateScope.launch {
|
||||
if (musicStore.library != null) {
|
||||
// This is a new library to replace a pre-existing one.
|
||||
|
||||
// Wipe possibly-invalidated album covers
|
||||
imageLoader.memoryCache?.clear()
|
||||
|
||||
// Clear invalid models from PlaybackStateManager.
|
||||
// Clear invalid models from PlaybackStateManager. This is not connected
|
||||
// to a callback as it is bad practice for a shared object to attach to
|
||||
// the callback system of another.
|
||||
playbackManager.sanitize(newLibrary)
|
||||
}
|
||||
|
||||
musicStore.updateLibrary(newLibrary)
|
||||
|
||||
stopForegroundSession()
|
||||
}
|
||||
} else {
|
||||
|
||||
// On errors, while we would want to show a notification that displays the
|
||||
// error, in practice that comes into conflict with the upcoming Android 13
|
||||
// notification permission, and there is no point implementing permission
|
||||
// on-boarding for such when it will only be used for this.
|
||||
stopForegroundSession()
|
||||
}
|
||||
}
|
||||
is Indexer.State.Indexing -> {
|
||||
// When loading, we want to enter the foreground state so that android does
|
||||
// not shut off the loading process. Note that while we will always post the
|
||||
|
|
@ -15,7 +15,7 @@
|
|||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package org.oxycblt.auxio.music
|
||||
package org.oxycblt.auxio.music.system
|
||||
|
||||
import androidx.lifecycle.ViewModel
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
|
|
@ -15,7 +15,7 @@
|
|||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package org.oxycblt.auxio.music
|
||||
package org.oxycblt.auxio.music.system
|
||||
|
||||
import android.annotation.SuppressLint
|
||||
import android.content.Context
|
||||
|
|
@ -69,6 +69,8 @@ import org.oxycblt.auxio.widgets.WidgetProvider
|
|||
*
|
||||
* TODO: Android Auto
|
||||
*
|
||||
* TODO: Get MediaSessionConnector (or the media3 equivalent) working or die trying
|
||||
*
|
||||
* @author OxygenCobalt
|
||||
*/
|
||||
class PlaybackService :
|
||||
|
|
|
|||
|
|
@ -25,8 +25,8 @@ import androidx.core.content.edit
|
|||
import androidx.preference.PreferenceManager
|
||||
import org.oxycblt.auxio.R
|
||||
import org.oxycblt.auxio.home.tabs.Tab
|
||||
import org.oxycblt.auxio.music.Directory
|
||||
import org.oxycblt.auxio.music.dirs.MusicDirs
|
||||
import org.oxycblt.auxio.music.system.Directory
|
||||
import org.oxycblt.auxio.playback.replaygain.ReplayGainMode
|
||||
import org.oxycblt.auxio.playback.replaygain.ReplayGainPreAmp
|
||||
import org.oxycblt.auxio.playback.state.PlaybackMode
|
||||
|
|
|
|||
|
|
@ -27,10 +27,10 @@ import android.util.Log
|
|||
import androidx.core.content.edit
|
||||
import java.io.File
|
||||
import org.oxycblt.auxio.R
|
||||
import org.oxycblt.auxio.music.Directory
|
||||
import org.oxycblt.auxio.music.directoryCompat
|
||||
import org.oxycblt.auxio.music.isInternalCompat
|
||||
import org.oxycblt.auxio.music.storageVolumesCompat
|
||||
import org.oxycblt.auxio.music.system.Directory
|
||||
import org.oxycblt.auxio.music.system.directoryCompat
|
||||
import org.oxycblt.auxio.music.system.isInternalCompat
|
||||
import org.oxycblt.auxio.music.system.storageVolumesCompat
|
||||
import org.oxycblt.auxio.ui.accent.Accent
|
||||
import org.oxycblt.auxio.util.logD
|
||||
import org.oxycblt.auxio.util.queryAll
|
||||
|
|
|
|||
|
|
@ -31,8 +31,8 @@ import androidx.recyclerview.widget.RecyclerView
|
|||
import coil.Coil
|
||||
import org.oxycblt.auxio.R
|
||||
import org.oxycblt.auxio.home.tabs.TabCustomizeDialog
|
||||
import org.oxycblt.auxio.music.IndexerViewModel
|
||||
import org.oxycblt.auxio.music.dirs.MusicDirsDialog
|
||||
import org.oxycblt.auxio.music.system.IndexerViewModel
|
||||
import org.oxycblt.auxio.playback.PlaybackViewModel
|
||||
import org.oxycblt.auxio.playback.replaygain.PreAmpCustomizeDialog
|
||||
import org.oxycblt.auxio.settings.ui.IntListPreference
|
||||
|
|
@ -53,6 +53,8 @@ import org.oxycblt.auxio.util.systemBarInsetsCompat
|
|||
* TODO: Add option to not restore state
|
||||
*
|
||||
* TODO: Disable playback state options when music is loading
|
||||
*
|
||||
* TODO: Indicate music loading in the "reload music" option???
|
||||
*/
|
||||
@Suppress("UNUSED")
|
||||
class SettingsListFragment : PreferenceFragmentCompat() {
|
||||
|
|
|
|||
|
|
@ -511,7 +511,7 @@ constructor(context: Context, attrs: AttributeSet? = null, defStyle: Int = 0) :
|
|||
// TODO: Improve accessibility by:
|
||||
// 1. Adding a (non-visible) handle. Material components now technically does have
|
||||
// this, but it relies on the busted BottomSheetBehavior.
|
||||
// 2. Adding the controls that BottomSheetBehavior defines in-app.
|
||||
// 2. Adding the controls that BottomSheetBehavior defines
|
||||
sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -205,8 +205,12 @@ This package also contains the two UI components used for all covers in Auxio:
|
|||
This package contains all `Music` implementations, the music loading implementation, and the music folder system. This is the second
|
||||
most complicated package in the app, as loading music in a sane way is horribly difficult.
|
||||
|
||||
Unlike other apps, Auxio does not load music from `MediaStore` as it is shown in the UI. That is dumb and stupid and prevents
|
||||
any advanced features like Album Artists. Instead, we have a single loading process that constructs an entire in-memory music
|
||||
library, which does increase memory usage, but allows for very high-quality metadata.
|
||||
|
||||
The major classes are:
|
||||
- `MusicStore`, which is the container for a `Library` instance. Any code wanting to access the library should use this
|
||||
- `MusicStore`, which is the container for a `Library` instance. Any code wanting to access the library should use this.
|
||||
- `Indexer`, which manages how music is loaded. This is only used by code that must reflect the music loading state.
|
||||
|
||||
Internally, there are several other major systems:
|
||||
|
|
|
|||
Loading…
Reference in a new issue