Fabio/Auxio
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

playback: re-add queue sanitization

Browse source 
Add library-change sanitization to the queue.

It is hard to describe how unbeliveably difficult this was. It's so
hard to wrap your head around this system and I really would have never
used it if it was not for ExoPlayer's insistence on it's busted
ShuffleOrder code.

Re-enabling state persistence should be easier following this.
This commit is contained in:
Alexander Capehart 2023-01-07 15:45:55 -07:00
parent bef4dca0ce
commit 82a9c08666
No known key found for this signature in database
GPG key ID: 37DBE3621FE9AD47
2 changed files with 132 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

80
app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt
View file

@ -494,50 +494,42 @@ class PlaybackStateManager private constructor() {
*/ */
@Synchronized @Synchronized
fun sanitize(newLibrary: Library) { fun sanitize(newLibrary: Library) {
// if (!isInitialized) { if (!isInitialized) {
// // Nothing playing, nothing to do. // Nothing playing, nothing to do.
// logD("Not initialized, no need to sanitize") logD("Not initialized, no need to sanitize")
// return return
// } }
//
// val internalPlayer = internalPlayer ?: return val internalPlayer = internalPlayer ?: return
//
// logD("Sanitizing state") logD("Sanitizing state")
//
// // While we could just save and reload the state, we instead sanitize the state // While we could just save and reload the state, we instead sanitize the state
// // at runtime for better performance (and to sidestep a co-routine on behalf of // at runtime for better performance (and to sidestep a co-routine on behalf of the caller).
// the caller).
// // Sanitize parent
// // Sanitize parent parent =
// parent = parent?.let {
// parent?.let { when (it) {
// when (it) { is Album -> newLibrary.sanitize(it)
// is Album -> newLibrary.sanitize(it) is Artist -> newLibrary.sanitize(it)
// is Artist -> newLibrary.sanitize(it) is Genre -> newLibrary.sanitize(it)
// is Genre -> newLibrary.sanitize(it) }
// } }
// }
// // Sanitize the queue.
// // Sanitize queue. Make sure we re-align the index to point to the previously queue.remap { it.map(newLibrary::sanitize) }
// playing
// // Song in the queue queue. notifyNewPlayback()
// val oldSongUid = song?.uid
// _queue = _queue.mapNotNullTo(mutableListOf()) { newLibrary.sanitize(it) } val oldPosition = playerState.calculateElapsedPositionMs()
// while (song?.uid != oldSongUid && index > -1) { // Continuing playback while also possibly doing drastic state updates is
// index-- // a bad idea, so pause.
// } internalPlayer.loadSong(queue.currentSong, false)
// if (queue.currentSong != null) {
// notifyNewPlayback() // Internal player may have reloaded the media item, re-seek to the previous position
// seekTo(oldPosition)
// val oldPosition = playerState.calculateElapsedPositionMs() }
// // Continuing playback while also possibly doing drastic state updates is
// // a bad idea, so pause.
// internalPlayer.loadSong(song, false)
// if (index > -1) {
// // Internal player may have reloaded the media item, re-seek to the previous
// position
// seekTo(oldPosition)
// }
} }
// --- CALLBACKS --- // --- CALLBACKS ---

128
app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt
View file

@ -29,8 +29,9 @@ import org.oxycblt.auxio.music.Song
* implementation is instead based around an unorganized "heap" of [Song] instances, that are then * implementation is instead based around an unorganized "heap" of [Song] instances, that are then
* interpreted into different queues depending on the current playback configuration. * interpreted into different queues depending on the current playback configuration.
* *
* In general, the implementation details don't need ot be known for this data structure to be used. * In general, the implementation details don't need to be known for this data structure to be used,
* The functions exposed should be familiar for any typical play queue. * except in special circumstances like [remap]. The functions exposed should be familiar for any
* typical play queue.
* *
* @author OxygenCobalt * @author OxygenCobalt
*/ */
@ -40,11 +41,15 @@ class Queue {
@Volatile private var shuffledMapping = mutableListOf<Int>() @Volatile private var shuffledMapping = mutableListOf<Int>()
/** The index of the currently playing [Song] in the current mapping. */ /** The index of the currently playing [Song] in the current mapping. */
@Volatile @Volatile
var index = 0 var index = -1
private set private set
/** The currently playing [Song]. */ /** The currently playing [Song]. */
val currentSong: Song? val currentSong: Song?
get() = shuffledMapping.ifEmpty { orderedMapping.ifEmpty { null } }?.let { heap[it[index]] } get() =
shuffledMapping
.ifEmpty { orderedMapping.ifEmpty { null } }
?.getOrNull(index)
?.let(heap::get)
/** Whether this queue is shuffled. */ /** Whether this queue is shuffled. */
val isShuffled: Boolean val isShuffled: Boolean
get() = shuffledMapping.isNotEmpty() get() = shuffledMapping.isNotEmpty()
@ -70,19 +75,20 @@ class Queue {
/** /**
* Start a new queue configuration. * Start a new queue configuration.
* @param song The [Song] to play, or null to start from a random position. * @param play The [Song] to play, or null to start from a random position.
* @param queue The queue of [Song]s to play. Must contain [song]. This list will become the * @param queue The queue of [Song]s to play. Must contain [play]. This list will become the
* heap internally. * heap internally.
* @param shuffled Whether to shuffle the queue or not. This changes the interpretation of * @param shuffled Whether to shuffle the queue or not. This changes the interpretation of
* [queue]. * [queue].
*/ */
fun start(song: Song?, queue: List<Song>, shuffled: Boolean) { fun start(play: Song?, queue: List<Song>, shuffled: Boolean) {
heap = queue.toMutableList() heap = queue.toMutableList()
orderedMapping = MutableList(queue.size) { it } orderedMapping = MutableList(queue.size) { it }
shuffledMapping = mutableListOf() shuffledMapping = mutableListOf()
index = index =
song?.let(queue::indexOf) ?: if (shuffled) Random.Default.nextInt(queue.indices) else 0 play?.let(queue::indexOf) ?: if (shuffled) Random.Default.nextInt(queue.indices) else 0
reorder(shuffled) reorder(shuffled)
check()
} }
/** /**
@ -90,6 +96,11 @@ class Queue {
* @param shuffled Whether the queue should be shuffled or not. * @param shuffled Whether the queue should be shuffled or not.
*/ */
fun reorder(shuffled: Boolean) { fun reorder(shuffled: Boolean) {
if (orderedMapping.isEmpty()) {
// Nothing to do.
return
}
if (shuffled) { if (shuffled) {
val trueIndex = val trueIndex =
if (shuffledMapping.isNotEmpty()) { if (shuffledMapping.isNotEmpty()) {
@ -110,21 +121,47 @@ class Queue {
index = orderedMapping.indexOf(shuffledMapping[index]) index = orderedMapping.indexOf(shuffledMapping[index])
shuffledMapping = mutableListOf() shuffledMapping = mutableListOf()
} }
check()
} }
/** /**
* Reformat the queue's internal representation to align with the given values. This is not * Replace the given heap with a new
* useful in most circumstances. * @param map Code to remap the existing [Song] heap into a new [Song] heap. This **MUST** be
* @param * the same size as the original heap. [Song] instances that could not be converted should be
* replaced with null in the new heap.
* @throws IllegalStateException If the given invariants regarding [map] were violated.
*/ */
fun rework(heap: List<Song?>, orderedMapping: IntArray, shuffledMapping: IntArray) { fun remap(map: (List<Song>) -> List<Song?>) {
// val instructions = mutableListOf<Int?>() val newHeap = map(heap)
// val currentBackshift = 0 val oldSong = currentSong
// for (song in heap) { check(newHeap.size == heap.size) { "New heap must be the same size as original heap" }
// if (song == null) {
// instructions.add(0, ) val adjustments = mutableListOf<Int?>()
// } var currentShift = 0
// } for (song in newHeap) {
if (song != null) {
adjustments.add(currentShift)
} else {
adjustments.add(null)
currentShift -= 1
}
}
heap = newHeap.filterNotNull().toMutableList()
orderedMapping =
orderedMapping.mapNotNullTo(mutableListOf()) { heapIndex ->
adjustments[heapIndex]?.let { heapIndex + it }
}
shuffledMapping =
shuffledMapping.mapNotNullTo(mutableListOf()) { heapIndex ->
adjustments[heapIndex]?.let { heapIndex + it }
}
// Make sure we re-align the index to point to the previously playing song.
while (currentSong != oldSong && index > -1) {
index--
}
check()
} }
/** /**
@ -151,6 +188,7 @@ class Queue {
// Add the new song in front of the current index in the ordered mapping. // Add the new song in front of the current index in the ordered mapping.
orderedMapping.addAll(index + 1, heapIndices) orderedMapping.addAll(index + 1, heapIndices)
} }
check()
return ChangeResult.MAPPING return ChangeResult.MAPPING
} }
@ -173,6 +211,7 @@ class Queue {
if (shuffledMapping.isNotEmpty()) { if (shuffledMapping.isNotEmpty()) {
shuffledMapping.addAll(heapIndices) shuffledMapping.addAll(heapIndices)
} }
check()
return ChangeResult.MAPPING return ChangeResult.MAPPING
} }
@ -201,9 +240,12 @@ class Queue {
in (src + 1)..dst -> index -= 1 in (src + 1)..dst -> index -= 1
// We have moved an song from in front of the playing song to behind, shift forward. // We have moved an song from in front of the playing song to behind, shift forward.
in dst until src -> index += 1 in dst until src -> index += 1
else -> ChangeResult.MAPPING else -> {
check()
ChangeResult.MAPPING
}
} }
check()
return ChangeResult.INDEX return ChangeResult.INDEX
} }
@ -229,17 +271,20 @@ class Queue {
// of the player to be completely invalidated. It's generally easier to not remove the // of the player to be completely invalidated. It's generally easier to not remove the
// song and retain player state consistency. // song and retain player state consistency.
return when { val result =
// We just removed the currently playing song. when {
index == at -> ChangeResult.SONG // We just removed the currently playing song.
// Index was ahead of removed song, shift back to preserve consistency. index == at -> ChangeResult.SONG
index > at -> { // Index was ahead of removed song, shift back to preserve consistency.
index -= 1 index > at -> {
ChangeResult.INDEX index -= 1
ChangeResult.INDEX
}
// Nothing to do
else -> ChangeResult.MAPPING
} }
// Nothing to do check()
else -> ChangeResult.MAPPING return result
}
} }
private fun addSongToHeap(song: Song): Int { private fun addSongToHeap(song: Song): Int {
@ -264,12 +309,31 @@ class Queue {
return orphanCandidates.first() return orphanCandidates.first()
} }
} }
// Nothing to re-use, add this song to the queue // Nothing to re-use, add this song to the queue
heap.add(song) heap.add(song)
return heap.lastIndex return heap.lastIndex
} }
private fun check() {
check(!(heap.isEmpty() && (orderedMapping.isNotEmpty() || shuffledMapping.isNotEmpty()))) {
"Queue inconsistency detected: Empty heap with non-empty mappings" +
"[ordered: ${orderedMapping.size}, shuffled: ${shuffledMapping.size}]"
}
check(shuffledMapping.isEmpty() || orderedMapping.size == shuffledMapping.size) {
"Queue inconsistency detected: Ordered mapping size ${orderedMapping.size} " +
"!= Shuffled mapping size ${shuffledMapping.size}"
}
check(orderedMapping.all { it in heap.indices }) {
"Queue inconsistency detected: Ordered mapping indices out of heap bounds"
}
check(shuffledMapping.all { it in heap.indices }) {
"Queue inconsistency detected: Shuffled mapping indices out of heap bounds"
}
}
/** /**
* Represents the possible changes that can occur during certain queue mutation events. The * Represents the possible changes that can occur during certain queue mutation events. The
* precise meanings of these differ somewhat depending on the type of mutation done. * precise meanings of these differ somewhat depending on the type of mutation done.