Skip to content

Commit

Permalink
playback: save playback state on every change
Browse files Browse the repository at this point in the history 
Prior, I was saving when the service was closed, which is a stupid
decision and caused a lot of unreliability.

Resolves #404.
  • Loading branch information
@OxygenCobalt
OxygenCobalt committed Jan 15, 2024
1 parent b2d9b24 commit 1766283
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 74 deletions.
74 changes: 54 additions & 20 deletions app/src/main/java/org/oxycblt/auxio/playback/system/PlaybackService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import kotlinx.coroutines.yield
import org.oxycblt.auxio.BuildConfig
import org.oxycblt.auxio.list.ListSettings
import org.oxycblt.auxio.music.MusicParent
Expand Down Expand Up @@ -117,6 +119,7 @@ class PlaybackService :
private val serviceJob = Job()
private val restoreScope = CoroutineScope(serviceJob + Dispatchers.IO)
private val saveScope = CoroutineScope(serviceJob + Dispatchers.IO)
private var currentSaveJob: Job? = null

// --- SERVICE OVERRIDES ---

Expand Down Expand Up @@ -228,8 +231,7 @@ class PlaybackService :
player.isPlaying,
// The position value can be below zero or past the expected duration, make
// sure we handle that.
player.currentPosition.coerceAtLeast(0)
.coerceAtMost(it.song.durationMs))
player.currentPosition.coerceAtLeast(0).coerceAtMost(it.song.durationMs))
}
?: Progression.nil()

Expand All @@ -246,11 +248,12 @@ class PlaybackService :

override fun resolveQueue(): RawQueue {
val heap = (0 until player.mediaItemCount).map { player.getMediaItemAt(it).song }
val shuffledMapping = if (player.shuffleModeEnabled) {
player.unscrambleQueueIndices()
} else {
emptyList()
}
val shuffledMapping =
if (player.shuffleModeEnabled) {
player.unscrambleQueueIndices()
} else {
emptyList()
}
return RawQueue(heap, shuffledMapping, player.currentMediaItemIndex)
}

Expand Down Expand Up @@ -279,11 +282,13 @@ class PlaybackService :
player.prepare()
player.play()
playbackManager.ack(this, StateAck.NewPlayback)
deferSave()
}

override fun playing(playing: Boolean) {
player.playWhenReady = playing
// Dispatched later once all of the changes have been accumulated
// Playing state is not persisted, do not need to save
}

override fun repeatMode(repeatMode: RepeatMode) {
Expand All @@ -295,15 +300,19 @@ class PlaybackService :
}
playbackManager.ack(this, StateAck.RepeatModeChanged)
updatePauseOnRepeat()
deferSave()
}

override fun seekTo(positionMs: Long) {
player.seekTo(positionMs)
// Dispatched later once all of the changes have been accumulated
// Deferred save is handled on position discontinuity
}

override fun next() {
player.seekToNext()
playbackManager.ack(this, StateAck.IndexMoved)
// Deferred save is handled on position discontinuity
}

override fun prev() {
Expand All @@ -313,6 +322,7 @@ class PlaybackService :
player.seekToPreviousMediaItem()
}
playbackManager.ack(this, StateAck.IndexMoved)
// Deferred save is handled on position discontinuity
}

override fun goto(index: Int) {
Expand All @@ -324,6 +334,7 @@ class PlaybackService :
val trueIndex = indices[index]
player.seekTo(trueIndex, C.TIME_UNSET)
playbackManager.ack(this, StateAck.IndexMoved)
// Deferred save is handled on position discontinuity
}

override fun shuffled(shuffled: Boolean) {
Expand All @@ -335,16 +346,19 @@ class PlaybackService :
BetterShuffleOrder(player.mediaItemCount, player.currentMediaItemIndex))
}
playbackManager.ack(this, StateAck.QueueReordered)
deferSave()
}

override fun playNext(songs: List<Song>, ack: StateAck.PlayNext) {
player.addMediaItems(player.nextMediaItemIndex, songs.map { it.toMediaItem() })
playbackManager.ack(this, ack)
deferSave()
}

override fun addToQueue(songs: List<Song>, ack: StateAck.AddToQueue) {
player.addMediaItems(songs.map { it.toMediaItem() })
playbackManager.ack(this, ack)
deferSave()
}

override fun move(from: Int, to: Int, ack: StateAck.Move) {
Expand All @@ -367,6 +381,7 @@ class PlaybackService :
}
}
playbackManager.ack(this, ack)
deferSave()
}

override fun remove(at: Int, ack: StateAck.Remove) {
Expand All @@ -378,6 +393,7 @@ class PlaybackService :
val trueIndex = indices[at]
player.removeMediaItem(trueIndex)
playbackManager.ack(this, ack)
deferSave()
}

override fun handleDeferred(action: DeferredPlayback): Boolean {
Expand Down Expand Up @@ -480,6 +496,17 @@ class PlaybackService :
}
}

override fun onPositionDiscontinuity(
oldPosition: Player.PositionInfo,
newPosition: Player.PositionInfo,
reason: Int
) {
super.onPositionDiscontinuity(oldPosition, newPosition, reason)
if (reason == Player.DISCONTINUITY_REASON_SEEK) {
deferSave()
}
}

override fun onEvents(player: Player, events: Player.Events) {
super.onEvents(player, events)

Expand Down Expand Up @@ -580,6 +607,21 @@ class PlaybackService :

// --- OTHER FUNCTIONS ---

private fun deferSave() {
currentSaveJob?.let {
logD("Discarding prior save job")
it.cancel()
}
currentSaveJob =
saveScope.launch {
logD("Waiting for save buffer")
delay(SAVE_BUFFER)
yield()
logD("Committing saved state")
persistenceRepository.saveState(playbackManager.toSavedState())
}
}

private fun broadcastAudioEffectAction(event: String) {
logD("Broadcasting AudioEffect event: $event")
sendBroadcast(
Expand All @@ -589,18 +631,6 @@ class PlaybackService :
.putExtra(AudioEffect.EXTRA_CONTENT_TYPE, AudioEffect.CONTENT_TYPE_MUSIC))
}

private fun stopAndSave() {
// This session has ended, so we need to reset this flag for when the next session starts.
hasPlayed = false
if (foregroundManager.tryStopForeground()) {
// Now that we have ended the foreground state (and thus music playback), we'll need
// to save the current state as it's not long until this service (and likely the whole
// app) is killed.
logD("Saving playback state")
saveScope.launch { persistenceRepository.saveState(playbackManager.toSavedState()) }
}
}

/**
* A [BroadcastReceiver] for receiving playback-specific [Intent]s from the system that require
* an active [IntentFilter] to be registered.
Expand Down Expand Up @@ -657,7 +687,10 @@ class PlaybackService :
ACTION_EXIT -> {
logD("Received exit event")
playbackManager.playing(false)
stopAndSave()
// This session has ended, so we need to reset this flag for when the next
// session starts.
hasPlayed = false
foregroundManager.tryStopForeground()
}
WidgetProvider.ACTION_WIDGET_UPDATE -> {
logD("Received widget update event")
Expand Down Expand Up @@ -687,6 +720,7 @@ class PlaybackService :
}

companion object {
const val SAVE_BUFFER = 5000L
const val ACTION_INC_REPEAT_MODE = BuildConfig.APPLICATION_ID + ".action.LOOP"
const val ACTION_INVERT_SHUFFLE = BuildConfig.APPLICATION_ID + ".action.SHUFFLE"
const val ACTION_SKIP_PREV = BuildConfig.APPLICATION_ID + ".action.PREV"
Expand Down
35 changes: 0 additions & 35 deletions app/src/main/java/org/oxycblt/auxio/settings/RootPreferenceFragment.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,41 +84,6 @@ class RootPreferenceFragment : BasePreferenceFragment(R.xml.preferences_root) {
}
getString(R.string.set_key_reindex) -> musicModel.refresh()
getString(R.string.set_key_rescan) -> musicModel.rescan()
getString(R.string.set_key_save_state) -> {
playbackModel.savePlaybackState { saved ->
// Use the nullable context, as we could try to show a toast when this
// fragment is no longer attached.
logD("Showing saving confirmation")
if (saved) {
context?.showToast(R.string.lbl_state_saved)
} else {
context?.showToast(R.string.err_did_not_save)
}
}
}
getString(R.string.set_key_wipe_state) -> {
playbackModel.wipePlaybackState { wiped ->
logD("Showing wipe confirmation")
if (wiped) {
// Use the nullable context, as we could try to show a toast when this
// fragment is no longer attached.
context?.showToast(R.string.lbl_state_wiped)
} else {
context?.showToast(R.string.err_did_not_wipe)
}
}
}
getString(R.string.set_key_restore_state) ->
playbackModel.tryRestorePlaybackState { restored ->
logD("Showing restore confirmation")
if (restored) {
// Use the nullable context, as we could try to show a toast when this
// fragment is no longer attached.
context?.showToast(R.string.lbl_state_restored)
} else {
context?.showToast(R.string.err_did_not_restore)
}
}
else -> return super.onPreferenceTreeClick(preference)
}

Expand Down
20 changes: 1 addition & 19 deletions app/src/main/res/xml/preferences_root.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,5 @@
app:title="@string/set_rescan" />

</PreferenceCategory>

<PreferenceCategory app:title="@string/set_state">

<Preference
app:key="@string/set_key_save_state"
app:summary="@string/set_save_desc"
app:title="@string/set_save_state" />

<Preference
app:key="@string/set_key_wipe_state"
app:summary="@string/set_wipe_desc"
app:title="@string/set_wipe_state" />

<Preference
app:key="@string/set_key_restore_state"
app:summary="@string/set_restore_desc"
app:title="@string/set_restore_state" />
</PreferenceCategory>


</PreferenceScreen>

0 comments on commit 1766283

Please sign in to comment.