Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,34 @@ class QueueRepository @Inject constructor(
)
}

/**
* Ensures [itemId] is the queue's front entry (issue #196): moves it there if already queued,
* inserts it there (never as auto-queued) otherwise. This is how [com.bugzapperlabs.mycasts.playback.PlaybackController]
* marks an episode "now playing" -- the currently-playing episode is a real queue entry like
* any other, always at the front, rather than a special case hidden from the queue entirely,
* so it shows up in Next Up itself (clearly marked) instead of only via the mini/expanded player.
*/
suspend fun moveToFront(itemId: String) {
val position = queueDao.minPosition() - 1
if (queueDao.findItemId(itemId) != null) {
queueDao.setPosition(itemId, position)
} else {
queueDao.insert(QueueEntry(itemId, position = position, addedAt = System.currentTimeMillis()))
}
}

/**
* Moves [itemId] to the back of the queue if it's currently queued; a no-op otherwise (issue
* #196). Used to get a just-failed episode out of the front slot so [PlaybackService] doesn't
* immediately trip over it again advancing to whatever's next, without discarding it outright
* the way a finished episode is -- it never actually played, so it's still worth surfacing in
* Next Up for the user to retry.
*/
suspend fun moveToEnd(itemId: String) {
if (queueDao.findItemId(itemId) == null) return
queueDao.setPosition(itemId, queueDao.maxPosition() + 1)
}

/** Returns the entry that was removed (for issue #284's undo), or null if it wasn't queued. */
suspend fun remove(itemId: String): QueueEntry? {
val entry = queueDao.getEntry(itemId) ?: return null
Expand All @@ -61,12 +89,10 @@ class QueueRepository @Inject constructor(
orderedItemIds.forEachIndexed { index, itemId -> queueDao.setPosition(itemId, index) }
}

/** Removes and returns the item at the front of the queue, or null if the queue is empty. */
suspend fun popNext(): String? {
val next = queueDao.firstItemId() ?: return null
queueDao.remove(next)
return next
}
/** The item at the front of the queue, without removing it (issue #196) -- the currently-playing
* episode is meant to stay queued (as the front entry) for as long as it's playing, so
* advancing to it shouldn't also dequeue it the way the old pop-and-remove semantics did. */
suspend fun peekFront(): String? = queueDao.firstItemId()

/**
* Evicts this feed's oldest *auto-queued* episodes (earliest added, not earliest published)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,16 @@ class PlaybackController @Inject constructor(

/**
* Returns false without starting playback if streaming is disallowed and nothing is
* downloaded. Removes [item] from the Next Up queue unconditionally, even when playback then
* fails to start -- issue #171: it's already shown pinned to the top of the Next Up screen via
* the current-playback player bar, so a leftover queue entry for it would just be a duplicate
* further down the list. Mirrors the dequeue-then-play behavior [QueueRepository] callers used
* to have to do themselves (e.g. the old `QueueViewModel.playNow`), but now for every path that
* starts playback.
* downloaded. Moves [item] to the front of the Next Up queue unconditionally, even when
* playback then fails to start (issue #196): the currently-playing episode is meant to be a
* real, visible queue entry -- always the front one -- rather than hidden from Next Up
* entirely, so it shows up there clearly marked as playing instead of only via the mini/expanded
* player. Mirrors the move-to-front-then-play behavior [QueueRepository] callers used to have
* to do themselves (e.g. the old `QueueViewModel.playNow`), but now for every path that starts
* playback.
*/
suspend fun play(item: FeedItem, feedTitle: String?): Boolean {
queueRepository.remove(item.id)
queueRepository.moveToFront(item.id)
return loadMedia(item, feedTitle, autoPlay = true)
}

Expand All @@ -247,6 +248,11 @@ class PlaybackController @Inject constructor(
val itemId = settings.lastPlayingItemId ?: return
val item = feedRepository.getItem(itemId)?.takeIf { it.feedId == feedId && !it.isRead } ?: return
val feed = feedRepository.getFeed(feedId)
// issue #196: keeps the restored episode as the queue's front entry too, the same as any
// other path that makes an episode "current" -- it may already be there (a normal resume),
// but a cold start after the app's own process died mid-playback is also how a
// still-current episode could end up missing from the queue table altogether.
queueRepository.moveToFront(itemId)
loadMedia(item, feed?.userTitle ?: feed?.title, autoPlay = false)
}

Expand All @@ -256,7 +262,7 @@ class PlaybackController @Inject constructor(

val previousItemId = currentItemId
if (previousItemId != null && previousItemId != item.id) {
requeuePreviousEpisode(previousItemId)
dropFromQueueIfFinished(previousItemId)
}

currentFeedId = item.feedId
Expand Down Expand Up @@ -304,12 +310,17 @@ class PlaybackController @Inject constructor(

/**
* Whatever was playing before a switch stays in "Next Up" as long as it wasn't finished
* (issue #106) -- mirrors [restoreLastPlayingItem]'s `!isRead` check for "not completed".
* A no-op if it's already queued (e.g. the user played straight from the queue).
* (issue #106) -- since issue #196, it's already a real queue entry the whole time it's
* playing (see [play]/[QueueRepository.moveToFront]), so there's nothing to *add* here; this
* only needs to drop it if it turns out to have actually finished (mirrors
* [restoreLastPlayingItem]'s `!isRead` check for "not completed") -- a finished episode
* shouldn't linger in Next Up just because it still happened to be the queue's front entry
* when playback moved on. A no-op if it was never queued in the first place, or the user
* already removed it from Next Up themselves while it was still playing.
*/
private suspend fun requeuePreviousEpisode(itemId: String) {
val previous = feedRepository.getItem(itemId)?.takeIf { !it.isRead } ?: return
queueRepository.addToFront(previous.id)
private suspend fun dropFromQueueIfFinished(itemId: String) {
val previous = feedRepository.getItem(itemId)?.takeIf { it.isRead } ?: return
queueRepository.remove(previous.id)
}

fun pause() {
Expand Down Expand Up @@ -408,9 +419,10 @@ class PlaybackController @Inject constructor(
positionTickerScope.launch(Dispatchers.IO) {
settingsDataStore.setLastPlayingItem(null, null)
// Explicit stop just ends "now playing" -- unlike natural completion, the episode
// isn't finished, so it goes back into Next Up the same way switching to a different
// episode already does (issue #191), instead of disappearing from the app entirely.
itemId?.let { requeuePreviousEpisode(it) }
// isn't finished, so it stays in Next Up the same way switching to a different episode
// already does (issue #191); it's already there as the front entry (issue #196), so
// this is only a safety net for the edge case where it's since been marked read.
itemId?.let { dropFromQueueIfFinished(it) }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,16 +196,22 @@ class PlaybackService : MediaSessionService() {
}

/**
* Keeps [preloadManager] warming up whatever's currently at the head of Next Up (issue #87),
* reacting to every mutation that can change it -- add/remove/reorder/auto-queue eviction all
* flow through [QueueRepository.observeQueue]. Runs for the service's whole lifetime, not
* gated on anything currently playing, so the head is already warm by the time an episode
* actually finishes and [playNextQueued] needs it.
* Keeps [preloadManager] warming up whatever's next in line after the episode currently
* playing (issue #87), reacting to every mutation that can change it -- add/remove/reorder/
* auto-queue eviction all flow through [QueueRepository.observeQueue]. Runs for the service's
* whole lifetime, not gated on anything currently playing, so the head is already warm by the
* time an episode actually finishes and [playNextQueued] needs it.
*
* Skips the queue's own front entry when it matches [Player.getCurrentMediaItem] (issue #196):
* the currently-playing episode is itself a real queue entry now, always at the front, so the
* "head" this preloads has to be the entry *after* it -- preloading the front entry itself
* would just be re-preloading whatever's already playing.
*/
@OptIn(markerClass = [UnstableApi::class])
private fun startPreloadingQueueHead() {
serviceScope.launch {
queueRepository.observeQueue().map { it.firstOrNull() }.distinctUntilChanged { old, new -> old?.item?.id == new?.item?.id }
queueRepository.observeQueue().map { queue -> queue.firstOrNull { it.item.id != player.currentMediaItem?.mediaId } }
.distinctUntilChanged { old, new -> old?.item?.id == new?.item?.id }
.collect { head ->
preloadManager.reset()
preloadedNext = null
Expand Down Expand Up @@ -388,11 +394,19 @@ class PlaybackService : MediaSessionService() {
// (setEnclosurePosition/markRead/auto-delete) below -- this episode never actually
// completed, so none of that applies.
if (advancingFromError) return
val itemId = player.currentMediaItem?.mediaId
advancingFromError = true
advanceWakeLock.acquire(ADVANCE_WAKE_LOCK_TIMEOUT_MS)
serviceScope.launch {
try {
playNextQueued()
// issue #196: the failed episode is still the queue's own front entry (it
// never stopped being "current" until now) -- move it out of the front before
// peeking the next one, or playNextQueued would just find this same broken
// episode again and loop on it forever. Moved to the back rather than removed
// outright: unlike a finished episode, this one never actually played, so it's
// still worth surfacing in Next Up for the user to retry later.
itemId?.let { queueRepository.moveToEnd(it) }
playNextQueued(excludeItemId = itemId)
} finally {
advancingFromError = false
}
Expand All @@ -411,7 +425,14 @@ class PlaybackService : MediaSessionService() {
// done beforehand was pure added silence between episodes, on top of whatever
// buffering the next episode's own prepare() needs (worse for a streamed
// episode than a downloaded one, since that also has to open a connection).
playNextQueued()
// issue #196: the finished episode is still the queue's own front entry (it
// never stopped being "current" until now) -- excluded here rather than
// removed upfront (which would reintroduce the silence gap issue #82 just
// eliminated), since playNextQueued would otherwise just find this same
// already-finished episode again. The row itself is dropped for real just
// below, alongside the finished-episode bookkeeping it belongs next to.
playNextQueued(excludeItemId = itemId)
queueRepository.remove(itemId)
feedRepository.setEnclosurePosition(itemId, null)
feedRepository.markRead(itemId, true)
// Storage cap / auto-cleanup (issue #71): only ever deletes an episode that
Expand All @@ -437,9 +458,16 @@ class PlaybackService : MediaSessionService() {
* issue #179: this has to keep working even with no UI/MediaController attached (backgrounded
* or screen off), and this service is the one guaranteed to still be running when that happens.
*/
// issue #196: the currently-playing episode is now a real queue entry itself (the front one),
// kept there for as long as it's playing rather than popped off on advance the way the old
// pop-and-remove semantics did -- see peekFront()'s doc. [excludeItemId] covers the one case
// that still needs to look past the front entry regardless: a just-failed episode moved to the
// back of the queue (PlaybackService.onPlayerError's moveToEnd) has nowhere to actually move to
// if it was the queue's only entry, so it would otherwise still be sitting at the front here,
// and get "advanced" into again -- the exact same broken episode, looping forever.
@OptIn(markerClass = [UnstableApi::class])
private suspend fun playNextQueued() {
val itemId = queueRepository.popNext()
private suspend fun playNextQueued(excludeItemId: String? = null) {
val itemId = queueRepository.peekFront()?.takeIf { it != excludeItemId }
if (itemId == null) {
player.clearMediaItems()
settingsDataStore.setLastPlayingItem(null, null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.DragHandle
import androidx.compose.material.icons.filled.Downloading
import androidx.compose.material.icons.filled.OfflinePin
import androidx.compose.material.icons.filled.PlayArrow
import androidx.compose.material.icons.filled.PlaylistRemove
import androidx.compose.material.icons.filled.VerticalAlignBottom
import androidx.compose.material.icons.filled.VerticalAlignTop
Expand Down Expand Up @@ -611,12 +612,32 @@ private fun RowScope.QueueRowContent(
)
}
androidx.compose.foundation.layout.Row(verticalAlignment = Alignment.CenterVertically) {
Text(
text = EpisodeDateFormatter.format(episode.item.publishDate),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
maxLines = 1,
)
// issue #196: the currently-playing episode is a real (always-front) queue entry now,
// shown in this same list rather than hidden from it -- swaps the date for an explicit
// "Now Playing" label so that's unmistakable at a glance, not just inferable from the
// row's highlight color (issue #96) alone.
if (isCurrentlyPlaying) {
Icon(
Icons.Filled.PlayArrow,
contentDescription = null,
tint = MaterialTheme.colorScheme.primary,
modifier = Modifier.size(16.dp),
)
Text(
text = stringResource(R.string.now_playing_title),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.primary,
maxLines = 1,
modifier = Modifier.padding(start = 2.dp),
)
} else {
Text(
text = EpisodeDateFormatter.format(episode.item.publishDate),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
maxLines = 1,
)
}
// issue #192: EpisodeListScreen already shows this per-row (see its own doc for why
// OfflinePin over DownloadDone), but the queue had no equivalent -- useful now that
// issue #188 added a bulk "download all" action here, with no other way to see which
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,57 @@ class QueueRepositoryTest {
}

@Test
fun popNext_removesAndReturnsFrontOfQueue() = runTest {
fun peekFront_returnsFrontOfQueueWithoutRemovingIt() = runTest {
queueRepository.addToEnd("ep-1")
queueRepository.addToEnd("ep-2")

val next = queueRepository.popNext()
val next = queueRepository.peekFront()

assertEquals("ep-1", next)
assertEquals(listOf("ep-2"), queueRepository.observeQueue().first().map { it.item.id })
assertEquals(listOf("ep-1", "ep-2"), queueRepository.observeQueue().first().map { it.item.id })
}

@Test
fun popNext_emptyQueue_returnsNull() = runTest {
assertNull(queueRepository.popNext())
fun peekFront_emptyQueue_returnsNull() = runTest {
assertNull(queueRepository.peekFront())
}

@Test
fun moveToFront_alreadyQueued_movesExistingEntryToFront() = runTest {
queueRepository.addToEnd("ep-1")
queueRepository.addToEnd("ep-2")

queueRepository.moveToFront("ep-2")

assertEquals(listOf("ep-2", "ep-1"), queueRepository.observeQueue().first().map { it.item.id })
}

@Test
fun moveToFront_notQueued_insertsAtFront() = runTest {
queueRepository.addToEnd("ep-1")

queueRepository.moveToFront("ep-2")

assertEquals(listOf("ep-2", "ep-1"), queueRepository.observeQueue().first().map { it.item.id })
}

@Test
fun moveToEnd_queuedEntry_movesToBackOfQueue() = runTest {
queueRepository.addToEnd("ep-1")
queueRepository.addToEnd("ep-2")

queueRepository.moveToEnd("ep-1")

assertEquals(listOf("ep-2", "ep-1"), queueRepository.observeQueue().first().map { it.item.id })
}

@Test
fun moveToEnd_notQueued_doesNothing() = runTest {
queueRepository.addToEnd("ep-1")

queueRepository.moveToEnd("ep-2")

assertEquals(listOf("ep-1"), queueRepository.observeQueue().first().map { it.item.id })
}

@Test
Expand Down
Loading
Loading