playback: make state manager lock

Add synchronized calls to all mutations in PlaybackStateManager.

I mean, it is global mutable state modified on several threads. This is
the safest option to remove a bunch of horrible bugs.
This commit is contained in:
OxygenCobalt 2022-06-19 16:42:54 -06:00
parent b8cc153f07
commit ba48cdad29
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47

View file

@ -47,8 +47,6 @@ import org.oxycblt.auxio.util.logW
* All access should be done with [PlaybackStateManager.getInstance]. * All access should be done with [PlaybackStateManager.getInstance].
* @author OxygenCobalt * @author OxygenCobalt
* *
* TODO: Leverage synchronized here to prevent state issues.
*
* TODO: Bug test app behavior when playback stops * TODO: Bug test app behavior when playback stops
*/ */
class PlaybackStateManager private constructor() { class PlaybackStateManager private constructor() {
@ -98,6 +96,7 @@ class PlaybackStateManager private constructor() {
/** Add a callback to this instance. Make sure to remove it when done. */ /** Add a callback to this instance. Make sure to remove it when done. */
fun addCallback(callback: Callback) { fun addCallback(callback: Callback) {
synchronized(this) {
if (isInitialized) { if (isInitialized) {
callback.onNewPlayback(index, queue, parent) callback.onNewPlayback(index, queue, parent)
callback.onPositionChanged(positionMs) callback.onPositionChanged(positionMs)
@ -108,6 +107,7 @@ class PlaybackStateManager private constructor() {
callbacks.add(callback) callbacks.add(callback)
} }
}
/** Remove a [PlaybackStateManager.Callback] bound to this instance. */ /** Remove a [PlaybackStateManager.Callback] bound to this instance. */
fun removeCallback(callback: Callback) { fun removeCallback(callback: Callback) {
@ -121,6 +121,7 @@ class PlaybackStateManager private constructor() {
return return
} }
synchronized(this) {
if (isInitialized) { if (isInitialized) {
controller.loadSong(song) controller.loadSong(song)
controller.seekTo(positionMs) controller.seekTo(positionMs)
@ -131,6 +132,7 @@ class PlaybackStateManager private constructor() {
this.controller = controller this.controller = controller
} }
}
/** Unregister a [PlaybackStateManager.Controller] with this instance. */ /** Unregister a [PlaybackStateManager.Controller] with this instance. */
fun unregisterController(controller: Controller) { fun unregisterController(controller: Controller) {
@ -151,6 +153,7 @@ class PlaybackStateManager private constructor() {
fun play(song: Song, playbackMode: PlaybackMode) { fun play(song: Song, playbackMode: PlaybackMode) {
val library = musicStore.library ?: return val library = musicStore.library ?: return
synchronized(this) {
parent = parent =
when (playbackMode) { when (playbackMode) {
PlaybackMode.ALL_SONGS -> null PlaybackMode.ALL_SONGS -> null
@ -166,6 +169,7 @@ class PlaybackStateManager private constructor() {
isPlaying = true isPlaying = true
isInitialized = true isInitialized = true
} }
}
/** /**
* Play a [parent], such as an artist or album. * Play a [parent], such as an artist or album.
@ -173,6 +177,8 @@ class PlaybackStateManager private constructor() {
*/ */
fun play(parent: MusicParent, shuffled: Boolean) { fun play(parent: MusicParent, shuffled: Boolean) {
val library = musicStore.library ?: return val library = musicStore.library ?: return
synchronized(this) {
this.parent = parent this.parent = parent
applyNewQueue(library, shuffled, null) applyNewQueue(library, shuffled, null)
seekTo(0) seekTo(0)
@ -181,10 +187,12 @@ class PlaybackStateManager private constructor() {
isPlaying = true isPlaying = true
isInitialized = true isInitialized = true
} }
}
/** Shuffle all songs. */ /** Shuffle all songs. */
fun shuffleAll() { fun shuffleAll() {
val library = musicStore.library ?: return val library = musicStore.library ?: return
synchronized(this) {
parent = null parent = null
applyNewQueue(library, true, null) applyNewQueue(library, true, null)
seekTo(0) seekTo(0)
@ -193,11 +201,13 @@ class PlaybackStateManager private constructor() {
isPlaying = true isPlaying = true
isInitialized = true isInitialized = true
} }
}
// --- QUEUE FUNCTIONS --- // --- QUEUE FUNCTIONS ---
/** Go to the next song, along with doing all the checks that entails. */ /** Go to the next song, along with doing all the checks that entails. */
fun next() { fun next() {
synchronized(this) {
// Increment the index, if it cannot be incremented any further, then // Increment the index, if it cannot be incremented any further, then
// repeat and pause/resume playback depending on the setting // repeat and pause/resume playback depending on the setting
if (index < _queue.lastIndex) { if (index < _queue.lastIndex) {
@ -206,9 +216,11 @@ class PlaybackStateManager private constructor() {
goto(0, repeatMode == RepeatMode.ALL) goto(0, repeatMode == RepeatMode.ALL)
} }
} }
}
/** Go to the previous song, doing any checks that are needed. */ /** Go to the previous song, doing any checks that are needed. */
fun prev() { fun prev() {
synchronized(this) {
// If enabled, rewind before skipping back if the position is past 3 seconds [3000ms] // If enabled, rewind before skipping back if the position is past 3 seconds [3000ms]
if (controller?.shouldPrevRewind() == true) { if (controller?.shouldPrevRewind() == true) {
rewind() rewind()
@ -217,6 +229,7 @@ class PlaybackStateManager private constructor() {
goto(max(index - 1, 0), true) goto(max(index - 1, 0), true)
} }
} }
}
private fun goto(idx: Int, play: Boolean) { private fun goto(idx: Int, play: Boolean) {
index = idx index = idx
@ -227,50 +240,66 @@ class PlaybackStateManager private constructor() {
/** Add a [song] to the top of the queue. */ /** Add a [song] to the top of the queue. */
fun playNext(song: Song) { fun playNext(song: Song) {
synchronized(this) {
_queue.add(index + 1, song) _queue.add(index + 1, song)
notifyQueueChanged() notifyQueueChanged()
} }
}
/** Add a list of [songs] to the top of the queue. */ /** Add a list of [songs] to the top of the queue. */
fun playNext(songs: List<Song>) { fun playNext(songs: List<Song>) {
synchronized(this) {
_queue.addAll(index + 1, songs) _queue.addAll(index + 1, songs)
notifyQueueChanged() notifyQueueChanged()
} }
}
/** Add a [song] to the end of the queue. */ /** Add a [song] to the end of the queue. */
fun addToQueue(song: Song) { fun addToQueue(song: Song) {
synchronized(this) {
_queue.add(song) _queue.add(song)
notifyQueueChanged() notifyQueueChanged()
} }
}
/** Add a list of [songs] to the end of the queue. */ /** Add a list of [songs] to the end of the queue. */
fun addToQueue(songs: List<Song>) { fun addToQueue(songs: List<Song>) {
synchronized(this) {
_queue.addAll(songs) _queue.addAll(songs)
notifyQueueChanged() notifyQueueChanged()
} }
}
/** Move a queue item at [from] to a position at [to]. Will ignore invalid indexes. */ /** Move a queue item at [from] to a position at [to]. Will ignore invalid indexes. */
fun moveQueueItem(from: Int, to: Int) { fun moveQueueItem(from: Int, to: Int) {
logD("Moving item $from to position $to") logD("Moving item $from to position $to")
synchronized(this) {
_queue.add(to, _queue.removeAt(from)) _queue.add(to, _queue.removeAt(from))
notifyQueueChanged() notifyQueueChanged()
} }
}
/** Remove a queue item at [index]. Will ignore invalid indexes. */ /** Remove a queue item at [index]. Will ignore invalid indexes. */
fun removeQueueItem(index: Int) { fun removeQueueItem(index: Int) {
logD("Removing item ${_queue[index].rawName}") logD("Removing item ${_queue[index].rawName}")
synchronized(this) {
_queue.removeAt(index) _queue.removeAt(index)
notifyQueueChanged() notifyQueueChanged()
} }
}
/** Set whether this instance is [shuffled]. Updates the queue accordingly. */ /** Set whether this instance is [shuffled]. Updates the queue accordingly. */
fun reshuffle(shuffled: Boolean) { fun reshuffle(shuffled: Boolean) {
val library = musicStore.library ?: return val library = musicStore.library ?: return
synchronized(this) {
val song = song ?: return val song = song ?: return
applyNewQueue(library, shuffled, song) applyNewQueue(library, shuffled, song)
notifyQueueChanged() notifyQueueChanged()
notifyShuffledChanged() notifyShuffledChanged()
} }
}
private fun applyNewQueue(library: MusicStore.Library, shuffled: Boolean, keep: Song?) { private fun applyNewQueue(library: MusicStore.Library, shuffled: Boolean, keep: Song?) {
val newQueue = (parent?.songs ?: library.songs).toMutableList() val newQueue = (parent?.songs ?: library.songs).toMutableList()
@ -318,6 +347,7 @@ class PlaybackStateManager private constructor() {
return return
} }
synchronized(this) {
// Don't accept any bugged positions that are over the duration of the song. // Don't accept any bugged positions that are over the duration of the song.
val maxDuration = song?.durationMs ?: -1 val maxDuration = song?.durationMs ?: -1
if (positionMs <= maxDuration) { if (positionMs <= maxDuration) {
@ -325,16 +355,19 @@ class PlaybackStateManager private constructor() {
notifyPositionChanged() notifyPositionChanged()
} }
} }
}
/** /**
* **Seek** to a [positionMs]. * **Seek** to a [positionMs].
* @param positionMs The position to seek to in millis. * @param positionMs The position to seek to in millis.
*/ */
fun seekTo(positionMs: Long) { fun seekTo(positionMs: Long) {
synchronized(this) {
this.positionMs = positionMs this.positionMs = positionMs
controller?.seekTo(positionMs) controller?.seekTo(positionMs)
notifyPositionChanged() notifyPositionChanged()
} }
}
/** Rewind to the beginning of a song. */ /** Rewind to the beginning of a song. */
fun rewind() = seekTo(0) fun rewind() = seekTo(0)
@ -343,9 +376,11 @@ class PlaybackStateManager private constructor() {
fun repeat() { fun repeat() {
rewind() rewind()
if (settingsManager.pauseOnRepeat) { if (settingsManager.pauseOnRepeat) {
synchronized(this) {
isPlaying = false isPlaying = false
} }
} }
}
// --- PERSISTENCE FUNCTIONS --- // --- PERSISTENCE FUNCTIONS ---
@ -368,6 +403,7 @@ class PlaybackStateManager private constructor() {
logD("State read completed successfully in ${System.currentTimeMillis() - start}ms") logD("State read completed successfully in ${System.currentTimeMillis() - start}ms")
synchronized(this) {
if (state != null) { if (state != null) {
index = state.index index = state.index
parent = state.parent parent = state.parent
@ -383,6 +419,7 @@ class PlaybackStateManager private constructor() {
isInitialized = true isInitialized = true
} }
}
/** /**
* Save the current state to the database. * Save the current state to the database.
@ -397,19 +434,19 @@ class PlaybackStateManager private constructor() {
val database = PlaybackStateDatabase.getInstance(context) val database = PlaybackStateDatabase.getInstance(context)
database.write( database.write(
synchronized(this) {
PlaybackStateDatabase.SavedState( PlaybackStateDatabase.SavedState(
index = index, index = index,
parent = parent, parent = parent,
queue = _queue, queue = _queue,
positionMs = positionMs, positionMs = positionMs,
isShuffled = isShuffled, isShuffled = isShuffled,
repeatMode = repeatMode)) repeatMode = repeatMode)
})
this@PlaybackStateManager.logD( this@PlaybackStateManager.logD(
"State save completed successfully in ${System.currentTimeMillis() - start}ms") "State save completed successfully in ${System.currentTimeMillis() - start}ms")
} }
isInitialized = true
} }
// --- CALLBACKS --- // --- CALLBACKS ---