diff --git a/AGENTS.md b/AGENTS.md index 53a2865..617237a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -305,6 +305,17 @@ Protocol details, distilled from `jukebox/multitimer.py` and `components/timers/ therefore has nothing in the last-value cache, which is why the timer sheet asks `get_state` once when it opens, and why the countdown is interpolated locally from `remainingSecondsAt`, the same way the progress bar interpolates elapsed time. +- **The last-value cache is a trap on this one topic, and only this one.** For every other topic the + cached message is current — `playerstatus` was published 250 ms ago. Here it is the message that + *set* the timer, carrying the `remaining_seconds` of that moment and no timestamp to date it by, so + a new subscriber that believes it restarts the countdown from the top. Sending the app to the + background closes the session (§8.3) and coming back opens a new one, which is how a 30-minute + timer used to read 30:00 again on every return. Two things in `PhonieboxSession` stop that: + `handshake` asks `get_state` on every connection, and a timer publish arriving within + `TIMER_REPLAY_WINDOW_MILLIS` of the SUB socket opening triggers a `get_state` instead of being + taken at face value — a replay and a change made that same second are indistinguishable, and asking + is right for both. Within a live session the replay after ZMQ's own silent TCP reconnect is already + dropped by `ZmqStatusSubscriber`'s raw-string compare; only a *new* subscriber sees it. The player screen's shuffle and repeat icons moved into one "playback options" menu alongside the timer: three mode toggles flanking the transport controls is more than that screen can carry. The @@ -312,6 +323,58 @@ menu's button is tinted when any of the three is active, and a running timer als "Stops in …" line under the transport row, because a countdown to silence should not be hidden behind a tap. +## The queue, and why skipping into it is stepped + +The player shows one song; `playerstatus` carries only `pos` and `playlistlength`. The queue behind +it comes from **`player.ctrl.playlistinfo`**, which is a real RPC on both `future3` branches even +though the box's own web UI never calls it and `src/webapp/src/commands/index.js` therefore omits +it. It is never published, so it has to be asked for. + +- **Asked once per queue change, never on a timer.** `PlayerRepositoryImpl.resolveQueueWhenItChanges` + refetches only when the cached queue can no longer be the one playing — `playlistlength` moved, or + `playerstatus.file` is not in it. A plain track change fits the cached queue and costs nothing, so + an album of twenty tracks is one RPC rather than twenty. There is a one-second debounce first, + because a queue change is usually a card tap and the box is still finishing the card handling on + that same sequential socket (§6). +- **`QueueEntry.title` falls back to the file name.** Most of a Phoniebox library is untagged rips, + so a missing title tag is the common case, not an edge one. +- **There is no command to play a queue position.** See `docs/protocol-notes.md`, "There is no way + to play a queue position", for the full list of routes ruled out. The one that looks like an + answer and is not is `play_single`: it clears the queue first, so using it to reach chapter seven + would leave the box silent when chapter seven ended. `PlayerRepository.playAt` is therefore a + ladder — try `play(pos=…)`, then walk with `next`/`prev`: + - **The probe is free.** An unpatched box rejects the kwarg in the plugin's signature *before the + body runs*, and the RPC server turns that into an error reply, so nothing about playback changes + for having asked. The answer is remembered per box **in memory only** — a box gets updated, and + a persisted "no" would outlive its reason. Only an `RpcErrorException` counts as "no"; a timeout + means the box is off and says nothing about its software. + - **The walk is closed-loop, not a blind burst.** It fires the gap, then reads + `playlistPosition` back and closes what is left one step at a time. That is what makes a command + marked `retryable = false` safe here: a step that went missing shows up as a position that did + not move, and nothing is ever resent blind. + - **Pause first, if it was playing.** From `pause` the box's `next` takes `mpd_client.next()` + rather than its stopped-state branch, and MPD may hold the pause across it — which would make + the whole walk silent. Unverified on hardware; if it does not hold, the walk is audible but + still correct. The original state is restored under `NonCancellable`, so cancelling half way + cannot leave the box paused. + - **Never past the last index**, which would run the box's own `end_of_playlist_next_action`. + - **Refused while shuffle is on.** MPD's `next` with `random` enabled goes to a *random* song, not + `pos + 1`, so no number of steps arrives anywhere in particular. Coil says so rather than + turning the user's shuffle off behind their back. The restriction lifts by itself on a box with + `play(pos=…)`, which ignores `random`. +- **The media session gets the real timeline**, and `timelineIndexFor` is the guard that makes it + safe. The queue and the status arrive independently, so there is a window where the box has moved + to another album and the cached queue still describes the old one. `SimpleBasePlayer` **throws** if + `currentMediaItemIndex` falls outside the playlist, and an index that merely points at the wrong + track puts another album's title on the lock screen — so both are checked, and either miss falls + back to the single-item timeline. It is a top-level function with its own test because `getState()` + needs a `Looper` and this does not. +- **No cover art per queued row**, in the sheet or the timeline: a cover is an RPC each on the shared + socket. Only the playing item has one, and it is the one item built from `playerstatus`. + +Upstream would close this with two lines — `def play(self, pos=None)` in `player/coordinator.py` and +the MPD backend. Coil needs no change when it lands: the ladder's first rung starts succeeding. + ## Stand-in cover art A Phoniebox library is largely ripped CDs and home-made folders, so a great deal of it has no diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c0073f..b6a3439 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,27 @@ automatically from the `## [x.y.z]` heading matching `versionName` in `app/build ## [Unreleased] +## [1.1.0] - 2026-08-11 + +### Added +- **The player now shows what the box has queued, and you can skip straight to a track in it.** A new + playlist button beside the transport controls opens the whole list with the playing track marked; + tapping a row goes there and leaves the rest of the album queued behind it, so an audio play carries + on into the next chapter instead of stopping. Reaching chapter seven no longer means tapping ⏭ six + times. The same list now reaches the lock screen, Android Auto and Assistant, which used to see a + single song with nothing around it +- The Phoniebox has no command for jumping to a playlist position, so Coil walks the list one track + at a time where it has to — the row it is heading for shows its progress and can be called off. This + is skipped entirely on a box whose software can jump directly, and a future Phoniebox release could + make it instant. Two limits worth knowing: it needs shuffle off, and Coil says so rather than + changing that setting for you + +### Fixed +- **A running sleep timer no longer starts its countdown over when you come back to the app.** The + timer on the box was always right, but the "Stops in …" line under the transport controls read the + full 30 minutes again every time Coil returned to the foreground. Coil now asks the box what is + actually left of the timer whenever it connects, so what you see counts down once + ## [1.0.1] - 2026-08-06 ### Added diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 9155ae7..63f98ca 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -20,8 +20,8 @@ android { applicationId = "app.coilforphoniebox" minSdk = 26 targetSdk = 36 - versionCode = 5 - versionName = "1.0.1" + versionCode = 6 + versionName = "1.1.0" } androidResources { diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerScreen.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerScreen.kt index 3d58734..daf52b6 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerScreen.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerScreen.kt @@ -27,6 +27,7 @@ import androidx.compose.material.icons.automirrored.rounded.VolumeUp import androidx.compose.material.icons.rounded.Bedtime import androidx.compose.material.icons.rounded.Pause import androidx.compose.material.icons.rounded.PlayArrow +import androidx.compose.material.icons.rounded.QueueMusic import androidx.compose.material.icons.rounded.Repeat import androidx.compose.material.icons.rounded.RepeatOne import androidx.compose.material.icons.rounded.Shuffle @@ -92,7 +93,26 @@ fun PlayerScreen( val scrub by viewModel.scrubPosition.collectAsStateWithLifecycle() val volumeTarget by viewModel.volumeTarget.collectAsStateWithLifecycle() val timerRemaining by viewModel.sleepTimerRemaining.collectAsStateWithLifecycle() + val queue by viewModel.queue.collectAsStateWithLifecycle() var timerSheetOpen by remember { mutableStateOf(false) } + var queueSheetOpen by remember { mutableStateOf(false) } + + if (queueSheetOpen) { + QueueSheet( + queue = queue, + currentPosition = state.status.playlistPosition, + onJumpTo = viewModel::jumpTo, + onCancelJump = viewModel::cancelJump, + onRetry = viewModel::refreshQueue, + onDismiss = { + queueSheetOpen = false + // Closing the sheet abandons a walk in progress: it is the only place its + // progress is visible, and a jump nobody can see arriving or cancel is worse + // than one that stops where it is. + viewModel.cancelJump() + }, + ) + } if (timerSheetOpen) { SleepTimerSheet( @@ -116,6 +136,10 @@ fun PlayerScreen( timerSheetOpen = true } + // No RPC here, unlike the timer: the queue is already resolved and kept in step in the + // background, once per queue change rather than once per opening (§6). + val openQueue = { queueSheetOpen = true } + BoxWithConstraints(modifier.fillMaxSize()) { // Two panes on a large screen, and also on any window that is merely wider than it is // tall: a phone on its side has width to spare and no height at all, which is the same @@ -135,6 +159,7 @@ fun PlayerScreen( timerRemaining = timerRemaining, viewModel = viewModel, onOpenTimer = openTimer, + onOpenQueue = openQueue, ) } else { CompactPlayer( @@ -151,6 +176,7 @@ fun PlayerScreen( timerRemaining = timerRemaining, viewModel = viewModel, onOpenTimer = openTimer, + onOpenQueue = openQueue, ) } } @@ -166,6 +192,7 @@ private fun CompactPlayer( timerRemaining: Int?, viewModel: PlayerViewModel, onOpenTimer: () -> Unit, + onOpenQueue: () -> Unit, ) { Column( modifier = Modifier @@ -197,6 +224,7 @@ private fun CompactPlayer( timerRemaining = timerRemaining, viewModel = viewModel, onOpenTimer = onOpenTimer, + onOpenQueue = onOpenQueue, ) Spacer(Modifier.height(24.dp)) @@ -220,6 +248,7 @@ private fun WidePlayer( timerRemaining: Int?, viewModel: PlayerViewModel, onOpenTimer: () -> Unit, + onOpenQueue: () -> Unit, ) { Row( modifier = Modifier @@ -258,6 +287,7 @@ private fun WidePlayer( timerRemaining = timerRemaining, viewModel = viewModel, onOpenTimer = onOpenTimer, + onOpenQueue = onOpenQueue, ) } } @@ -327,6 +357,7 @@ private fun ColumnScope.PlayerControls( timerRemaining: Int?, viewModel: PlayerViewModel, onOpenTimer: () -> Unit, + onOpenQueue: () -> Unit, ) { ProgressRow( elapsedSeconds = scrub?.toDouble() ?: state.status.elapsedSeconds, @@ -343,12 +374,14 @@ private fun ColumnScope.PlayerControls( repeat = state.status.repeat, timerRunning = state.sleepTimer.running, anyOptionActive = state.anyOptionActive, + queueLength = state.status.playlistLength, onToggle = viewModel::toggle, onNext = viewModel::next, onPrevious = viewModel::previous, onShuffle = viewModel::toggleShuffle, onRepeat = viewModel::cycleRepeat, onOpenTimer = onOpenTimer, + onOpenQueue = onOpenQueue, ) // Only while a timer is running: a countdown to something stopping is worth a line of its @@ -508,12 +541,14 @@ private fun TransportRow( repeat: RepeatMode, timerRunning: Boolean, anyOptionActive: Boolean, + queueLength: Int, onToggle: () -> Unit, onNext: () -> Unit, onPrevious: () -> Unit, onShuffle: () -> Unit, onRepeat: () -> Unit, onOpenTimer: () -> Unit, + onOpenQueue: () -> Unit, ) { Row( modifier = Modifier.fillMaxWidth(), @@ -563,8 +598,20 @@ private fun TransportRow( ) } - // Balances the options button on the other side; an IconButton's own footprint. - Spacer(Modifier.size(48.dp)) + // Balances the options button on the other side, whichever of the two it holds — both + // are an IconButton's own 48 dp, so the play button stays centred either way. There is + // nothing to open for a web radio stream or a single track, and `playlistLength` is how + // the box says so. + if (queueLength > 1) { + IconButton(onClick = onOpenQueue) { + Icon( + imageVector = Icons.Rounded.QueueMusic, + contentDescription = stringResource(R.string.action_show_queue), + ) + } + } else { + Spacer(Modifier.size(48.dp)) + } } } diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerViewModel.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerViewModel.kt index 2e8d123..670ca45 100644 --- a/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerViewModel.kt +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/player/PlayerViewModel.kt @@ -8,8 +8,10 @@ import app.coilforphoniebox.domain.model.Box import app.coilforphoniebox.domain.model.ConnectionState import app.coilforphoniebox.domain.model.Favorite import app.coilforphoniebox.domain.model.FavoriteType +import app.coilforphoniebox.domain.model.JumpOutcome import app.coilforphoniebox.domain.model.PlayTarget import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.RepeatMode import app.coilforphoniebox.domain.model.SleepTimerStatus import app.coilforphoniebox.domain.model.VolumeStatus @@ -20,6 +22,7 @@ import app.coilforphoniebox.ui.UiMessage import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.Job import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -161,6 +164,87 @@ class PlayerViewModel @Inject constructor( ) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), State()) + /** + * What the queue sheet shows. + * + * A [StateFlow] of its own rather than fields on [State], and deliberately so: reading the + * queue takes a round trip, and anything folded into that `combine` holds up the title, + * progress and controls until it has emitted — the trap the cover lookup is kept out of for + * the same reason. Nothing here can delay the player. + */ + data class QueueState( + val entries: List = emptyList(), + /** An answer is on its way, so an empty [entries] is not yet a failure. */ + val loading: Boolean = false, + /** + * Position the box is being sent to, while it is being sent there. + * + * On a box that cannot jump outright this is a walk of one `next` per track, which takes + * a visible moment — so the row being aimed at says so and can be called off. Null when + * nothing is in flight. + */ + val jumpTarget: Int? = null, + ) { + /** Nothing to show and nothing coming: the box was asked and did not answer. */ + val failed: Boolean get() = entries.isEmpty() && !loading + } + + private val _jumpTarget = MutableStateFlow(null) + private var jumpJob: Job? = null + + val queue: StateFlow = combine( + player.queue, + player.queueLoading, + _jumpTarget, + ) { entries, loading, jumpTarget -> QueueState(entries, loading, jumpTarget) } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), QueueState()) + + /** + * Sends the box to queue position [position]. + * + * The box has no command for this, so the repository may have to walk the queue there one + * track at a time — which is why this tracks a target the UI can show and cancel, and why + * three different outcomes are worth telling the user apart. + */ + fun jumpTo(position: Int) { + jumpJob?.cancel() + _jumpTarget.value = position + jumpJob = viewModelScope.launch { + try { + player.playAt(position) + .onFailure { messageChannel.emit(UiMessage(commandError())) } + .onSuccess { outcome -> + when (outcome) { + // Arriving is what was asked for; saying so would be noise. + JumpOutcome.Arrived -> Unit + JumpOutcome.BlockedByShuffle -> + messageChannel.emit(UiMessage(R.string.queue_needs_shuffle_off)) + // The box is playing *something*, just not this. Reporting success + // would leave the highlighted row lying about where it is. + is JumpOutcome.Incomplete -> + messageChannel.emit(UiMessage(R.string.queue_jump_incomplete)) + } + } + } finally { + // Guarded, because a newer jump may already have claimed the field. + if (_jumpTarget.value == position) _jumpTarget.value = null + } + } + } + + /** Abandons a walk in progress. The box keeps playing wherever it got to. */ + fun cancelJump() { + jumpJob?.cancel() + _jumpTarget.value = null + } + + /** For a sheet showing a list that failed to arrive; the ordinary case needs no help. */ + fun refreshQueue() { + viewModelScope.launch { + player.refreshQueue().onFailure { messageChannel.emit(UiMessage(commandError())) } + } + } + /** * Seconds left on the timer, recomputed once a second while one is running. * diff --git a/app/src/main/kotlin/app/coilforphoniebox/ui/player/QueueSheet.kt b/app/src/main/kotlin/app/coilforphoniebox/ui/player/QueueSheet.kt new file mode 100644 index 0000000..714f6bd --- /dev/null +++ b/app/src/main/kotlin/app/coilforphoniebox/ui/player/QueueSheet.kt @@ -0,0 +1,263 @@ +package app.coilforphoniebox.ui.player + +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.lazy.rememberLazyListState +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.rounded.QueueMusic +import androidx.compose.material.icons.rounded.VolumeUp +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.ModalBottomSheet +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.material3.rememberModalBottomSheetState +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.pluralStringResource +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import app.coilforphoniebox.R +import app.coilforphoniebox.domain.model.QueueEntry +import app.coilforphoniebox.ui.components.EmptyState +import app.coilforphoniebox.ui.components.formatDuration +import app.coilforphoniebox.ui.components.formatNumber + +/** + * What the box has queued, and a way into the middle of it. + * + * The player shows one song, and `playerstatus` carries only a position and a length — so before + * this there was no way to see what came next, and no way to reach chapter seven of an audio play + * except tapping ⏭ six times. That is the case this exists for. + * + * **The box cannot jump to a queue position**, which shapes what a tap does. There is no + * `play(pos)` in the Phoniebox RPC surface, and `play_single` — the one command that starts a + * named track — clears the queue first, so using it would leave the box silent at the end of + * whichever chapter was picked. Instead the repository walks the queue with `next`/`prev`, which + * keeps it whole and takes a visible moment; hence the spinner on the row being aimed at and the + * cancel beside it. + * + * No artwork per row. Resolving a cover is an RPC each on the socket the box shares with its card + * reader (§6), and a numbered list of chapters reads better as text anyway — this deliberately + * looks like `TrackRow` in the library rather than like the favourites grid. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun QueueSheet( + queue: PlayerViewModel.QueueState, + currentPosition: Int?, + onJumpTo: (Int) -> Unit, + onCancelJump: () -> Unit, + onRetry: () -> Unit, + onDismiss: () -> Unit, +) { + val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) + + ModalBottomSheet(onDismissRequest = onDismiss, sheetState = sheetState) { + Column(Modifier.padding(bottom = 24.dp)) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 24.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Column(Modifier.weight(1f)) { + Text( + text = stringResource(R.string.queue_title), + style = MaterialTheme.typography.titleLarge, + ) + if (queue.entries.isNotEmpty()) { + Spacer(Modifier.height(2.dp)) + Text( + text = pluralStringResource( + R.plurals.queue_track_count, + queue.entries.size, + queue.entries.size, + ), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + + // Only while a walk is in flight, which is the only thing here worth calling off. + if (queue.jumpTarget != null) { + TextButton(onClick = onCancelJump) { + Text(stringResource(R.string.action_cancel)) + } + } + } + + Spacer(Modifier.height(12.dp)) + + when { + queue.entries.isNotEmpty() -> QueueList( + entries = queue.entries, + currentPosition = currentPosition, + jumpTarget = queue.jumpTarget, + onJumpTo = onJumpTo, + ) + + queue.loading -> Box( + modifier = Modifier + .fillMaxWidth() + .height(160.dp), + contentAlignment = Alignment.Center, + ) { + CircularProgressIndicator() + } + + // Nothing to show and nothing coming. The box has to be asked for its queue — + // it is never published — so this is a failed request, not an empty playlist. + else -> EmptyState( + icon = Icons.Rounded.QueueMusic, + title = stringResource(R.string.queue_unavailable_title), + body = stringResource(R.string.queue_unavailable_body), + actionLabel = stringResource(R.string.action_retry), + onAction = onRetry, + ) + } + } + } +} + +@Composable +private fun QueueList( + entries: List, + currentPosition: Int?, + jumpTarget: Int?, + onJumpTo: (Int) -> Unit, +) { + val listState = rememberLazyListState() + + // Opening on track one of a twenty-chapter audio play would hide the whole point of the + // sheet. Follows the box afterwards too, so a track change while it is open keeps up. + LaunchedEffect(currentPosition, entries.size) { + val index = entries.indexOfFirst { it.position == currentPosition } + if (index >= 0) listState.scrollToItem(index) + } + + LazyColumn(state = listState) { + items(entries, key = { it.songId ?: "${it.position}:${it.url}" }) { entry -> + QueueRow( + entry = entry, + playing = entry.position == currentPosition, + // Only the row being walked to; the rest stay tappable so a mind can be changed + // mid-walk without cancelling first. + pending = entry.position == jumpTarget, + onClick = { onJumpTo(entry.position) }, + ) + } + } +} + +@Composable +private fun QueueRow( + entry: QueueEntry, + playing: Boolean, + pending: Boolean, + onClick: () -> Unit, +) { + val colour = if (playing) { + MaterialTheme.colorScheme.primary + } else { + MaterialTheme.colorScheme.onSurface + } + + Row( + modifier = Modifier + .fillMaxWidth() + .then( + if (playing) { + Modifier.background(MaterialTheme.colorScheme.surfaceContainerHigh) + } else { + Modifier + }, + ) + .clickable(onClick = onClick) + .padding(horizontal = 24.dp, vertical = 12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + // A fixed slot for the number, the playing marker and the walk's spinner, so rows do not + // shift sideways as any of the three appears. + Box( + modifier = Modifier.size(28.dp), + contentAlignment = Alignment.Center, + ) { + when { + pending -> CircularProgressIndicator(Modifier.size(18.dp), strokeWidth = 2.dp) + // The icon carries the description rather than the row: a `semantics` block on + // the row would replace the merged title and duration instead of adding to + // them, so "Now playing" would be all a screen reader got. + playing -> Icon( + imageVector = Icons.Rounded.VolumeUp, + contentDescription = stringResource(R.string.queue_now_playing), + tint = colour, + modifier = Modifier.size(18.dp), + ) + + else -> Text( + // Positions are zero-based on the box and one-based to a reader. + text = formatNumber(entry.position + 1), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + + Spacer(Modifier.width(16.dp)) + + Column(Modifier.weight(1f)) { + Text( + text = entry.title, + style = MaterialTheme.typography.bodyLarge, + color = colour, + fontWeight = if (playing) FontWeight.Medium else null, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + // Only when it adds something: within one album every row would otherwise repeat the + // same artist under the same album, which is noise at this density. + val subtitle = listOfNotNull(entry.artist, entry.album) + .distinct() + .joinToString(" · ") + .ifBlank { null } + if (subtitle != null) { + Text( + text = subtitle, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } + } + + // A stream has no duration, so the column is simply absent rather than showing 0:00. + entry.durationSeconds?.let { seconds -> + Spacer(Modifier.width(12.dp)) + Text( + text = formatDuration(seconds), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } +} diff --git a/app/src/main/res/values-de/strings.xml b/app/src/main/res/values-de/strings.xml index ab20dd5..63b10e3 100644 --- a/app/src/main/res/values-de/strings.xml +++ b/app/src/main/res/values-de/strings.xml @@ -31,6 +31,19 @@ Aus den Favoriten entfernen Lautstärke %1$d%% + Playlist anzeigen + Playlist + + %1$d Titel + %1$d Titel + + Läuft gerade + Playlist nicht verfügbar + Coil muss die Box nach ihrer Playlist fragen — dafür muss die Box erreichbar sein. + Erneut versuchen + Schalte die Zufallswiedergabe aus, um zu einem Titel zu springen. Diese Box kann die Playlist nur Titel für Titel durchgehen. + Dieser Titel wurde nicht erreicht. Die Wiedergabe ist an einer anderen Stelle der Playlist stehen geblieben. + Mit %1$s verbunden Verbinde mit %1$s… Verbinde erneut mit %1$s… diff --git a/app/src/main/res/values-es/strings.xml b/app/src/main/res/values-es/strings.xml index 6262f24..6062fc0 100644 --- a/app/src/main/res/values-es/strings.xml +++ b/app/src/main/res/values-es/strings.xml @@ -28,6 +28,20 @@ Quitar de favoritos Volumen %1$d%% + Mostrar la lista de reproducción + Lista de reproducción + + %1$d pista + %1$d de pistas + %1$d pistas + + Suena ahora + Lista de reproducción no disponible + Coil tiene que preguntar a la caja qué tiene en cola, así que la caja debe estar accesible. + Volver a intentarlo + Desactiva la reproducción aleatoria para saltar a una pista. Esta caja solo puede recorrer la lista pista a pista. + No se ha llegado a esa pista. La reproducción se ha detenido en otro punto de la lista. + Conectado a %1$s Conectando con %1$s… Reconectando con %1$s… diff --git a/app/src/main/res/values-fr/strings.xml b/app/src/main/res/values-fr/strings.xml index cc2550e..0d74089 100644 --- a/app/src/main/res/values-fr/strings.xml +++ b/app/src/main/res/values-fr/strings.xml @@ -32,6 +32,20 @@ Retirer des favoris Volume %1$d%% + Afficher la playlist + Playlist + + %1$d piste + %1$d de pistes + %1$d pistes + + En cours de lecture + Playlist indisponible + Coil doit demander à la box ce qu’elle a en file d’attente : elle doit donc être joignable. + Réessayer + Désactive la lecture aléatoire pour passer à une piste. Cette box ne peut parcourir la playlist que piste par piste. + Cette piste n’a pas été atteinte. La lecture s’est arrêtée ailleurs dans la playlist. + Connecté à %1$s Connexion à %1$s… Reconnexion à %1$s… diff --git a/app/src/main/res/values-nl/strings.xml b/app/src/main/res/values-nl/strings.xml index 4a47d43..98c1416 100644 --- a/app/src/main/res/values-nl/strings.xml +++ b/app/src/main/res/values-nl/strings.xml @@ -31,6 +31,19 @@ Uit favorieten verwijderen Volume %1$d%% + Playlist weergeven + Playlist + + %1$d nummer + %1$d nummers + + Speelt nu + Playlist niet beschikbaar + Coil moet de box vragen wat er in de wachtrij staat, dus de box moet bereikbaar zijn. + Opnieuw proberen + Zet willekeurige weergave uit om naar een nummer te springen. Deze box kan de playlist alleen nummer voor nummer doorlopen. + Dat nummer is niet bereikt. De weergave is elders in de playlist gestopt. + Verbonden met %1$s Verbinden met %1$s… Opnieuw verbinden met %1$s… diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 66fed66..0445700 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -65,6 +65,27 @@ Volume %1$d%% + + Show playlist + Playlist + + + %1$d track + %1$d tracks + + + Now playing + Playlist unavailable + Coil has to ask the box what it has queued, so this needs the box to be reachable. + Try again + + Turn shuffle off to skip to a track. This box can only step through the playlist one track at a time. + Could not get to that track. Playback stopped elsewhere in the playlist. + Connected to %1$s Connecting to %1$s… diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt index 7ad4b77..4dd20fe 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt @@ -10,9 +10,11 @@ import app.coilforphoniebox.domain.model.FolderContent import app.coilforphoniebox.domain.model.LibraryAlbum import app.coilforphoniebox.domain.model.LibraryIndexResult import app.coilforphoniebox.domain.model.LibraryIndexState +import app.coilforphoniebox.domain.model.JumpOutcome import app.coilforphoniebox.domain.model.LibrarySearchResults import app.coilforphoniebox.domain.model.PlayTarget import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.RepeatMode import app.coilforphoniebox.domain.model.SessionMode import app.coilforphoniebox.domain.model.SleepTimerStatus @@ -55,6 +57,12 @@ class FakePlayerRepository( sleepTimer: SleepTimerStatus = SleepTimerStatus.Off, boxVersion: String? = "future3/main", private val coverFile: String? = null, + /** + * What the box has queued. Empty by default, which is also what the player screen shows for + * it: with no queue there is no playlist button, so a golden that does not name one captures + * the same picture it always did. + */ + queue: List = emptyList(), ) : PlayerRepository { override val status = MutableStateFlow(status) override val volume = MutableStateFlow(volume) @@ -63,6 +71,8 @@ class FakePlayerRepository( override val coverUrl = MutableStateFlow(coverUrl) override val coverPending = MutableStateFlow(coverPending) override val sleepTimer = MutableStateFlow(sleepTimer) + override val queue = MutableStateFlow(queue) + override val queueLoading = MutableStateFlow(false) override fun currentCoverFile(): String? = coverFile @@ -80,6 +90,18 @@ class FakePlayerRepository( override suspend fun startSleepTimer(minutes: Int): Result = Result.success(Unit) override suspend fun cancelSleepTimer(): Result = Result.success(Unit) override suspend fun refreshSleepTimer(): Result = Result.success(Unit) + override suspend fun refreshQueue(): Result = Result.success(Unit) + + /** + * Arrives immediately, without moving [status]. + * + * A golden is one settled moment, so there is nothing here to simulate — and the walk this + * stands in for is the repository's own business, tested where the stepping lives rather + * than through a screenshot. + */ + override suspend fun playAt(position: Int): Result = + Result.success(JumpOutcome.Arrived) + override suspend fun play(target: PlayTarget): Result = Result.success(Unit) override suspend fun playOn(boxId: String, target: PlayTarget): Result = Result.success(Unit) } diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt index e4c07fa..0a9dd84 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/Fixtures.kt @@ -10,6 +10,7 @@ import app.coilforphoniebox.domain.model.LibrarySearchResults import app.coilforphoniebox.domain.model.LibraryTrack import app.coilforphoniebox.domain.model.PlaybackState import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.RepeatMode import app.coilforphoniebox.domain.model.SleepTimerStatus import java.util.concurrent.TimeUnit @@ -63,7 +64,9 @@ object Fixtures { elapsedSeconds = 95.0, durationSeconds = 372.0, songId = "17", - playlistPosition = 3, + // Third of twelve, zero-based — so in [queue] the highlighted row is "Chapter 3" and + // the numbering either side of it reads the way a reader would expect. + playlistPosition = 2, playlistLength = 12, ) @@ -81,6 +84,37 @@ object Fixtures { playlistLength = 1, ) + /** + * The queue [playing] is playing from — twelve tracks, matching its `playlistLength`, and + * carrying its title and URL at its `playlistPosition` so the sheet's highlighted row is the + * same track the player above it names. + * + * Deliberately mixed, because a row here has three things to get wrong: one track has no + * duration the way a stream does (row 10), one has no artist or album so the subtitle line + * has to disappear rather than render blank (row 6), and one title is long enough that + * ellipsising is in the picture (row 8). + */ + val queue = List(12) { index -> + val number = index + 1 + QueueEntry( + position = index, + url = if (index == playing.playlistPosition) { + playing.file!! + } else { + "Detective Stories/The Missing Key/${"%02d".format(number)} Chapter.mp3" + }, + title = when (index) { + playing.playlistPosition -> playing.title!! + 7 -> "Chapter 8 — A Long Title That Has To Be Cut Off Somewhere Around Here" + else -> "Chapter $number" + }, + artist = "Detective Stories".takeUnless { index == 5 }, + album = "The Missing Key".takeUnless { index == 5 }, + durationSeconds = if (index == 9) null else 240.0 + index * 17, + songId = (14 + index).toString(), + ) + } + val timerRunning = SleepTimerStatus( running = true, remainingSeconds = 1_800, diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/PlayerScreenshotTest.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/PlayerScreenshotTest.kt index bdfd55b..ff4eae1 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/PlayerScreenshotTest.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/PlayerScreenshotTest.kt @@ -2,7 +2,10 @@ package app.coilforphoniebox.screenshot import app.coilforphoniebox.domain.model.ConnectionState import app.coilforphoniebox.domain.model.Favorite +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.performClick import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.SleepTimerStatus import app.coilforphoniebox.domain.model.VolumeStatus import app.coilforphoniebox.ui.player.PlayerScreen @@ -29,6 +32,7 @@ class PlayerScreenshotTest : ScreenshotTest() { coverPending: Boolean = false, sleepTimer: SleepTimerStatus = SleepTimerStatus.Off, favorites: List = Fixtures.favorites, + queue: List = Fixtures.queue, ) = PlayerViewModel( player = FakePlayerRepository( status = status, @@ -37,6 +41,7 @@ class PlayerScreenshotTest : ScreenshotTest() { coverUrl = coverUrl, coverPending = coverPending, sleepTimer = sleepTimer, + queue = queue, ), boxes = FakeBoxRepository(), favorites = FakeFavoriteRepository(favorites), @@ -106,6 +111,45 @@ class PlayerScreenshotTest : ScreenshotTest() { capture("player/sleep_timer_light") { PlayerScreen(vm) } } + /** + * The playlist sheet, reached by *clicking* the button that opens it. + * + * Not by composing `QueueSheet` directly, on purpose and following `favourites_compact_*`: + * the button lives in the transport row and only appears when the box reports more than one + * queued track, so a change that stripped it out — or that stopped it opening anything — + * has to fail this test rather than quietly keep the old picture. + * + * What the golden is worth looking at: the fourth row is highlighted and marked as playing, + * the list is scrolled to it rather than sitting at track one, one row has no duration and + * one has no subtitle. + */ + @Test + fun queue_sheet() { + val vm = viewModel() + show { PlayerScreen(vm) } + compose.onNodeWithContentDescription("Show playlist").performClick() + // A modal sheet slides in, and Robolectric's clock does not move on its own — without + // this the golden is the player with the sheet still off the bottom of the screen. + advanceTime(SHEET_SETTLE_MILLIS) + captureScreen("player/queue_light") + } + + /** + * The same sheet with nothing in it and nothing on its way. + * + * The queue is never published — it has to be asked for — so an empty list here means the + * request failed, not that the playlist is empty. `playlistLength` still says twelve, which + * is what keeps the button on screen and makes the state reachable at all. + */ + @Test + fun queue_unavailable() { + val vm = viewModel(queue = emptyList()) + show { PlayerScreen(vm) } + compose.onNodeWithContentDescription("Show playlist").performClick() + advanceTime(SHEET_SETTLE_MILLIS) + captureScreen("player/queue_unavailable_light") + } + /** * A phone on its side: 731×411 dp, under the two-pane breakpoint but far too short for a * full-width cover. @@ -140,4 +184,9 @@ class PlayerScreenshotTest : ScreenshotTest() { ) capture("player/idle_de") { PlayerScreen(vm) } } + + private companion object { + /** Comfortably longer than the sheet's slide-in, so the golden is the settled state. */ + const val SHEET_SETTLE_MILLIS = 1_000L + } } diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/ScreenshotTest.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/ScreenshotTest.kt index e1770de..b40e5ce 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/ScreenshotTest.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/ScreenshotTest.kt @@ -20,7 +20,9 @@ import coil.ImageLoader import coil.decode.DataSource import coil.request.SuccessResult import coil.test.FakeImageLoaderEngine +import com.github.takahirom.roborazzi.ExperimentalRoborazziApi import com.github.takahirom.roborazzi.captureRoboImage +import com.github.takahirom.roborazzi.captureScreenRoboImage import dagger.hilt.android.testing.HiltAndroidRule import org.junit.Before import org.junit.Rule @@ -134,6 +136,19 @@ abstract class ScreenshotTest { captureTo("$GOLDEN_DIR/$name.png") } + /** + * Captures every window at once, for UI that opens one of its own. + * + * A `ModalBottomSheet` composes into a separate window rather than into the activity's, so + * [captureRoot] fails outright there — `onRoot()` matches two roots and cannot choose. This + * composites the lot, which is also the only way to get the sheet *and* the screen behind + * it into one picture, scrim included. + */ + @OptIn(ExperimentalRoborazziApi::class) + protected fun captureScreen(name: String) { + captureScreenRoboImage("$GOLDEN_DIR/$name.png") + } + /** * Writes the window to an arbitrary path, relative to the `:app` module directory. * diff --git a/app/src/testDebug/screenshots/app/player_dark_phone.png b/app/src/testDebug/screenshots/app/player_dark_phone.png index 58753fe..ba07b7a 100644 Binary files a/app/src/testDebug/screenshots/app/player_dark_phone.png and b/app/src/testDebug/screenshots/app/player_dark_phone.png differ diff --git a/app/src/testDebug/screenshots/app/player_dark_small.png b/app/src/testDebug/screenshots/app/player_dark_small.png index a89f7dd..c83c4ab 100644 Binary files a/app/src/testDebug/screenshots/app/player_dark_small.png and b/app/src/testDebug/screenshots/app/player_dark_small.png differ diff --git a/app/src/testDebug/screenshots/app/player_dark_tablet.png b/app/src/testDebug/screenshots/app/player_dark_tablet.png index 1df8117..bbed979 100644 Binary files a/app/src/testDebug/screenshots/app/player_dark_tablet.png and b/app/src/testDebug/screenshots/app/player_dark_tablet.png differ diff --git a/app/src/testDebug/screenshots/app/player_phone.png b/app/src/testDebug/screenshots/app/player_phone.png index b15f943..34187c4 100644 Binary files a/app/src/testDebug/screenshots/app/player_phone.png and b/app/src/testDebug/screenshots/app/player_phone.png differ diff --git a/app/src/testDebug/screenshots/app/player_small.png b/app/src/testDebug/screenshots/app/player_small.png index ce9da36..624af8a 100644 Binary files a/app/src/testDebug/screenshots/app/player_small.png and b/app/src/testDebug/screenshots/app/player_small.png differ diff --git a/app/src/testDebug/screenshots/app/player_tablet.png b/app/src/testDebug/screenshots/app/player_tablet.png index 6a4ef3d..16aa0c6 100644 Binary files a/app/src/testDebug/screenshots/app/player_tablet.png and b/app/src/testDebug/screenshots/app/player_tablet.png differ diff --git a/app/src/testDebug/screenshots/player/cover_resolving_light.png b/app/src/testDebug/screenshots/player/cover_resolving_light.png index 48eda7e..e8dac91 100644 Binary files a/app/src/testDebug/screenshots/player/cover_resolving_light.png and b/app/src/testDebug/screenshots/player/cover_resolving_light.png differ diff --git a/app/src/testDebug/screenshots/player/paused_light.png b/app/src/testDebug/screenshots/player/paused_light.png index e025bbb..446867e 100644 Binary files a/app/src/testDebug/screenshots/player/paused_light.png and b/app/src/testDebug/screenshots/player/paused_light.png differ diff --git a/app/src/testDebug/screenshots/player/playing_dark.png b/app/src/testDebug/screenshots/player/playing_dark.png index 9de6c3d..4388892 100644 Binary files a/app/src/testDebug/screenshots/player/playing_dark.png and b/app/src/testDebug/screenshots/player/playing_dark.png differ diff --git a/app/src/testDebug/screenshots/player/playing_landscape.png b/app/src/testDebug/screenshots/player/playing_landscape.png index 9a7d7ac..f24c670 100644 Binary files a/app/src/testDebug/screenshots/player/playing_landscape.png and b/app/src/testDebug/screenshots/player/playing_landscape.png differ diff --git a/app/src/testDebug/screenshots/player/playing_light.png b/app/src/testDebug/screenshots/player/playing_light.png index b6a5748..02008b4 100644 Binary files a/app/src/testDebug/screenshots/player/playing_light.png and b/app/src/testDebug/screenshots/player/playing_light.png differ diff --git a/app/src/testDebug/screenshots/player/queue_light.png b/app/src/testDebug/screenshots/player/queue_light.png new file mode 100644 index 0000000..e09135b Binary files /dev/null and b/app/src/testDebug/screenshots/player/queue_light.png differ diff --git a/app/src/testDebug/screenshots/player/queue_unavailable_light.png b/app/src/testDebug/screenshots/player/queue_unavailable_light.png new file mode 100644 index 0000000..91ac1d0 Binary files /dev/null and b/app/src/testDebug/screenshots/player/queue_unavailable_light.png differ diff --git a/app/src/testDebug/screenshots/player/sleep_timer_light.png b/app/src/testDebug/screenshots/player/sleep_timer_light.png index 79e93a5..446d35d 100644 Binary files a/app/src/testDebug/screenshots/player/sleep_timer_light.png and b/app/src/testDebug/screenshots/player/sleep_timer_light.png differ diff --git a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt index 9dee84d..147ca05 100644 --- a/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt +++ b/core-data/src/main/kotlin/app/coilforphoniebox/data/repository/PlayerRepositoryImpl.kt @@ -2,8 +2,10 @@ package app.coilforphoniebox.data.repository import app.coilforphoniebox.domain.model.Box import app.coilforphoniebox.domain.model.ConnectionState +import app.coilforphoniebox.domain.model.JumpOutcome import app.coilforphoniebox.domain.model.PlayTarget import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.RepeatMode import app.coilforphoniebox.domain.model.SleepTimerStatus import app.coilforphoniebox.domain.model.VolumeStatus @@ -11,8 +13,13 @@ import app.coilforphoniebox.domain.repository.PlayerRepository import app.coilforphoniebox.transport.Commands import app.coilforphoniebox.transport.ConnectionManager import app.coilforphoniebox.transport.LibraryParser +import app.coilforphoniebox.transport.NotConnectedException +import app.coilforphoniebox.transport.QueueParser +import app.coilforphoniebox.transport.RpcErrorException +import app.coilforphoniebox.transport.UnexpectedPayloadException import app.coilforphoniebox.transport.di.TransportScope import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow @@ -23,10 +30,14 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext import kotlinx.serialization.json.JsonElement import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject import javax.inject.Singleton +import kotlin.math.abs /** * Live player state and playback commands, both straight from the active box. @@ -90,8 +101,121 @@ class PlayerRepositoryImpl @Inject constructor( override fun currentCoverFile(): String? = _coverFile.value + private val _queue = MutableStateFlow>(emptyList()) + + override val queue: StateFlow> = _queue.asStateFlow() + + private val _queueLoading = MutableStateFlow(false) + + override val queueLoading: StateFlow = _queueLoading.asStateFlow() + + /** + * One queue fetch at a time. Two overlapping `playlistinfo` calls would put two library-sized + * requests on the socket the box shares with its card reader, for one answer. + */ + private val queueFetch = Mutex() + + /** + * Boxes that have been asked to `play(pos=…)`, and whether they took it. + * + * Kept in memory and keyed by box, never persisted: a box gets its software updated, and a + * stored "no" would outlive the reason for it. Absent means "not asked yet". + */ + private val acceptsJump = ConcurrentHashMap() + init { scope.launch { resolveCoversForCurrentSong() } + scope.launch { resolveQueueWhenItChanges() } + } + + /** What makes one queue a different queue from another. */ + private data class QueueTrigger( + val boxId: String?, + val connected: Boolean, + val length: Int, + val file: String?, + ) + + /** + * Keeps [queue] in step with the box, asking `playlistinfo` **once per queue change**. + * + * The queue is not published, so the only way to know it changed is to notice that + * `playerstatus` no longer fits the cached one — a different length, or a playing file that + * is not in it. Both mean a new `play_folder`/`play_album`/card tap; a plain track change + * fits the cached queue and costs nothing. An album of twenty tracks is therefore one RPC + * rather than twenty, which is what makes this safe on the socket the box shares with its + * card reader (§6). + */ + private suspend fun resolveQueueWhenItChanges() { + var loadedFor: String? = null + + combine( + transport.activeBox.map { it?.id }, + transport.connection.map { it == ConnectionState.CONNECTED }, + transport.status.map { it.playlistLength to it.file }, + ) { boxId, connected, queueShape -> + QueueTrigger(boxId, connected, queueShape.first, queueShape.second) + } + .distinctUntilChanged() + // A change while a fetch is in flight abandons it: the answer would describe a + // queue that is no longer the current one. + .collectLatest { trigger -> + // A queue belongs to a box and to a live connection. Showing the one from + // before a switch would put another box's tracks on screen. + if (trigger.boxId == null || !trigger.connected || trigger.length == 0) { + _queue.value = emptyList() + _queueLoading.value = false + loadedFor = null + return@collectLatest + } + if (trigger.boxId != loadedFor) { + _queue.value = emptyList() + loadedFor = null + } + if (queueStillFits(trigger)) return@collectLatest + + // Said before the wait, not after: from here on an empty queue means "the + // answer is coming", which is what stops the sheet reading it as a failure. + _queueLoading.value = true + try { + // A queue change is usually a card tap, and the box is still finishing the + // card handling on this same sequential socket. Racing it is precisely what + // §6 rules out, so let the status settle first. + delay(QUEUE_SETTLE_MILLIS) + // A failure leaves the last known queue in place and does not retry here: + // the next track change re-runs this, a bounded retry rather than a loop. + if (fetchQueue().isSuccess) loadedFor = trigger.boxId + } finally { + // Also on the way out of a `collectLatest` cancellation, or the flag would + // be stuck true for a queue nobody is fetching any more. + _queueLoading.value = false + } + } + } + + /** Reads the queue and publishes it. */ + private suspend fun fetchQueue(): Result = queueFetch.withLock { + transport.call(Commands.playlistInfo).map { payload -> + _queue.value = QueueParser.queue(payload) + } + } + + override suspend fun refreshQueue(): Result { + _queueLoading.value = true + return try { + fetchQueue() + } finally { + _queueLoading.value = false + } + } + + /** Whether the cached queue can still be the one the box is playing from. */ + private fun queueStillFits(trigger: QueueTrigger): Boolean { + val cached = _queue.value + if (cached.size != trigger.length) return false + // A stopped box reports no file; the queue it holds is still the one it had. + val file = trigger.file ?: return true + return cached.any { it.url == file } } private suspend fun resolveCoversForCurrentSong() { @@ -174,6 +298,124 @@ class PlayerRepositoryImpl @Inject constructor( override suspend fun seekTo(positionSeconds: Double): Result = transport.call(Commands.seek(positionSeconds.coerceAtLeast(0.0))).unit() + /** + * Two routes to the same place, tried in order of preference. + * + * The box has no command for this (see [Commands.playAt]), so the first attempt is a probe: + * `play(pos=…)` either works or is rejected by the plugin's own signature *before its body + * runs*, which makes a rejected attempt free of consequences. What the answer must not be + * confused with is a timeout — a box that is switched off says nothing about whether its + * software has the command, so only an `error` reply is taken as "no". + */ + override suspend fun playAt(position: Int): Result { + val boxId = transport.activeBox.value?.id + ?: return Result.failure(NotConnectedException()) + val target = position.coerceAtLeast(0) + + if (acceptsJump[boxId] != false) { + val attempt = transport.call(Commands.playAt(target)) + attempt.onSuccess { + acceptsJump[boxId] = true + return Result.success(JumpOutcome.Arrived) + } + val failure = attempt.exceptionOrNull() + if (failure !is RpcErrorException) { + // Unreachable, torn down or too slow. The capability is still unknown. + return Result.failure(failure ?: NotConnectedException()) + } + acceptsJump[boxId] = false + } + + return stepTo(target) + } + + /** + * Walks the queue to [target] with `next`/`prev`, for a box that cannot jump. + * + * The queue survives this, which is the whole point of not reaching for `play_single`: it + * would replace the queue with the one track and leave the box silent when it ended. The + * cost is that it takes a moment and that the caller has to be told when it fell short. + */ + private suspend fun stepTo(target: Int): Result { + val start = transport.currentStatus() + val from = start.playlistPosition + ?: return Result.failure(UnexpectedPayloadException(Commands.next.name)) + + if (from == target) return Result.success(JumpOutcome.Arrived) + // Stepping past the end runs the box's own `end_of_playlist_next_action`, which is not + // ours to trigger. + if (target > start.playlistLength - 1) return Result.success(JumpOutcome.Incomplete(from)) + // MPD's `next` with `random` enabled goes to a *random* song rather than the following + // one, so no number of steps arrives anywhere in particular. Nothing is sent. + if (start.shuffle) return Result.success(JumpOutcome.BlockedByShuffle) + + val wasPlaying = start.isPlaying + // From `pause`, the box's `next` takes `mpd_client.next()` rather than its stopped-state + // branch, and MPD may hold the pause across it — which makes the whole walk silent. If + // it does not hold, this has cost one command and changed nothing else. + if (wasPlaying) transport.call(Commands.pause) + + try { + // Each call waits for its reply, so by the last one the box has already moved; the + // loop below is mostly confirmation. A step that times out is *not* resent — that + // is what `retryable = false` on `next` means — and shows up instead as a position + // that did not move. + val step = stepToward(target, from) + val gap = abs(target - from) + var sent = 0 + while (sent < gap && transport.call(step).isSuccess) sent++ + + var landed = awaitSettled(target) + var corrections = 0 + while (landed != target && corrections < MAX_STEP_CORRECTIONS) { + if (transport.call(stepToward(target, landed)).isFailure) break + landed = awaitSettled(target) + corrections++ + } + + return Result.success( + if (landed == target) JumpOutcome.Arrived else JumpOutcome.Incomplete(landed), + ) + } finally { + // Including when the walk was cancelled half way: leaving the box paused because + // the user closed the sheet would be a worse outcome than not arriving. + if (wasPlaying) withContext(NonCancellable) { transport.call(Commands.play) } + } + } + + private fun stepToward(target: Int, current: Int) = + if (target > current) Commands.next else Commands.previous + + /** + * Waits until the box reports [target] or stops moving, and returns where it ended up. + * + * Both halves are needed. Waiting only for the target hangs forever on a step that went + * missing, and reading the position after a fixed delay would "correct" a walk that was + * still arriving — overshooting by however many steps were still in flight. The box + * publishes about four times a second, so a few quiet ticks mean it has finished moving + * rather than merely not having published yet. Nothing here touches the box: this reads the + * status already arriving over PubSub. + */ + private suspend fun awaitSettled(target: Int): Int { + var position = transport.currentStatus().playlistPosition ?: -1 + var quietTicks = 0 + + repeat(STEP_TICK_LIMIT) { + delay(STEP_TICK_MILLIS) + val reported = transport.currentStatus().playlistPosition ?: position + when { + reported == target -> return target + reported != position -> { + position = reported + quietTicks = 0 + } + + else -> if (++quietTicks >= STEP_QUIET_TICKS) return position + } + } + return position + } + override suspend fun setShuffle(enabled: Boolean): Result = transport.call(Commands.shuffle(enabled)).unit() @@ -225,5 +467,23 @@ class PlayerRepositoryImpl @Inject constructor( /** The timer plugin takes `wait_seconds`; the UI offers whole minutes. */ const val SECONDS_PER_MINUTE = 60 + + /** + * Long enough for a card read to finish on the box's sequential RPC socket before the + * queue is asked for, short enough that the sheet does not feel stuck opening. + */ + const val QUEUE_SETTLE_MILLIS = 1_000L + + /** Roughly the box's own publish interval, so a step is seen on the next tick. */ + const val STEP_TICK_MILLIS = 250L + + /** Four quiet ticks — a second of no movement — mean the walk has finished arriving. */ + const val STEP_QUIET_TICKS = 4 + + /** Ceiling on one wait, so a box that stopped reporting cannot hang the walk. */ + const val STEP_TICK_LIMIT = 60 + + /** A correction covers a lost or doubled step; a run of them means give up and say so. */ + const val MAX_STEP_CORRECTIONS = 3 } } diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/JumpOutcome.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/JumpOutcome.kt new file mode 100644 index 0000000..c385ff3 --- /dev/null +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/JumpOutcome.kt @@ -0,0 +1,33 @@ +package app.coilforphoniebox.domain.model + +/** + * What came of asking the box to jump to a queue position. + * + * Three outcomes rather than a boolean, because the box has no single command for this and the + * ways it can fall short are different enough that the user needs telling apart. A transport + * failure is *not* one of them — that stays a `Result` failure, as everywhere else. + */ +sealed interface JumpOutcome { + + /** The box is playing the requested position. */ + data object Arrived : JumpOutcome + + /** + * The queue had to be walked with `next`/`prev` and stopped at [position] instead of the + * target — a step went missing, or it ran out of attempts or time. + * + * Worth surfacing rather than swallowing: the box *is* playing something, just not what + * was asked for, and silently reporting success would leave the highlighted row lying. + */ + data class Incomplete(val position: Int) : JumpOutcome + + /** + * Nothing was sent, because this box can only reach a position by walking and shuffle is + * on: MPD's `next` with `random` enabled goes to a *random* song rather than the following + * one, so no number of steps arrives anywhere in particular. + * + * Coil says so instead of turning shuffle off on the user's behalf — and the restriction + * disappears once a box has `play(pos=…)`, which ignores `random`. + */ + data object BlockedByShuffle : JumpOutcome +} diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/QueueEntry.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/QueueEntry.kt new file mode 100644 index 0000000..c152409 --- /dev/null +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/model/QueueEntry.kt @@ -0,0 +1,33 @@ +package app.coilforphoniebox.domain.model + +/** + * One entry of the box's current MPD queue. + * + * The queue is live playback state, in the same category as [PlayerStatus]: it comes from the + * box, it is never persisted, and it is thrown away when the active box changes (§6.2). It is + * *not* library data — the tracks in it may sit in folders Coil has never browsed, and the + * order is MPD's, not the library cache's. + * + * Every field except [position] and [url] is optional, for the same reason every field of + * [PlayerStatus] is: MPD omits whatever the file has no tag for, and a Phoniebox library is + * largely untagged rips. + */ +data class QueueEntry( + /** Zero-based position in the queue, matching [PlayerStatus.playlistPosition]. */ + val position: Int, + /** MPD URL of the song, relative to the box's music library root. */ + val url: String, + /** + * What to show. Falls back to the file name when the song carries no title tag, which is + * what the library rows do too — a row with no text at all would be unusable. + */ + val title: String, + val artist: String? = null, + val album: String? = null, + val durationSeconds: Double? = null, + /** + * MPD's queue id, unique within the queue and stable while it lasts — which is what the + * media session needs for a timeline item's UID. Null when the box did not send one. + */ + val songId: String? = null, +) diff --git a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/PlayerRepository.kt b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/PlayerRepository.kt index 3994666..2ca290f 100644 --- a/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/PlayerRepository.kt +++ b/core-domain/src/main/kotlin/app/coilforphoniebox/domain/repository/PlayerRepository.kt @@ -1,12 +1,15 @@ package app.coilforphoniebox.domain.repository import app.coilforphoniebox.domain.model.ConnectionState +import app.coilforphoniebox.domain.model.JumpOutcome import app.coilforphoniebox.domain.model.PlayTarget import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.RepeatMode import app.coilforphoniebox.domain.model.SleepTimerStatus import app.coilforphoniebox.domain.model.VolumeStatus import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow /** * Live state of the active box. Nothing here is persisted: `playerstatus` and @@ -56,12 +59,55 @@ interface PlayerRepository { */ val sleepTimer: Flow + /** + * The box's current queue, in play order, or an empty list when it is not known. + * + * A flow rather than a one-shot because the media session consumes it too — but a flow + * that is **resolved per queue change, never polled**: `playlistinfo` is asked when + * `playlistLength` moves or the playing file is not in the cached list, which happens on a + * `play_folder`/`play_album`/card tap and *not* on a track change. An album of twenty + * tracks therefore costs one RPC, not twenty (§6). + * + * It starts empty and stays empty until the first answer arrives, so it can never hold up + * a screen that combines it with the rest of the player state — the trap [coverUrl] exists + * to avoid. Every consumer has to cope with an empty or momentarily stale list. + */ + val queue: StateFlow> + + /** + * Whether an answer is on its way, so an empty [queue] can be read as "could not be read" + * rather than "not yet" — the same distinction [coverPending] draws for artwork, and for the + * same reason: without it a failed fetch is a spinner that never stops. + */ + val queueLoading: StateFlow + + /** + * Asks for the queue again now, for a user who is looking at a list that failed to load. + * + * The background resolver handles every ordinary case; this exists so a retry does not have + * to wait for the next track change. + */ + suspend fun refreshQueue(): Result + suspend fun play(): Result suspend fun pause(): Result suspend fun toggle(): Result suspend fun next(): Result suspend fun previous(): Result suspend fun seekTo(positionSeconds: Double): Result + + /** + * Start playing queue position [position], leaving the rest of the queue in place. + * + * **The box does not have a command for this**, so the implementation is a ladder: it + * tries `play(pos=…)` — which upstream `future3` rejects harmlessly — and otherwise walks + * the queue with `next`/`prev`. A walk takes a moment and is visible, so callers should + * show that it is happening; see [JumpOutcome] for what can come back. + * + * Suspends until the box has arrived (or the attempt gave up), unlike the fire-and-forget + * commands above, because "did it get there" is the only useful answer. + */ + suspend fun playAt(position: Int): Result suspend fun setShuffle(enabled: Boolean): Result suspend fun setRepeat(mode: RepeatMode): Result diff --git a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt index 83f6761..84f83c7 100644 --- a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt +++ b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxCommand.kt @@ -96,6 +96,27 @@ object Commands { val next = player("next") val previous = player("prev") + /** + * Start playing the queue entry at [position]. + * + * **Not part of upstream `future3` yet.** `playermpd.play` is `def play(self)` on both + * `future3/main` and `future3/develop`, so an unpatched box answers this with + * `TypeError: play() got an unexpected keyword argument 'pos'` — `jukebox/rpc/server.py` + * wraps `plugs.call` in a try/except and formats any exception as an error reply. The + * important part is that the exception happens *before* the body runs, so a rejected + * call changes nothing on the box and can be used as a free capability probe. + * + * MPD itself has always supported it, and the box uses it internally + * (`_next_in_stopped_state` calls `self.mpd_client.play(pos)`); only the RPC signature is + * missing. Until it lands upstream, callers fall back to stepping with [next]/[previous] + * — see `PlayerRepository.playAt`. + */ + fun playAt(position: Int) = player( + "play", + kwargs = mapOf("pos" to JsonPrimitive(position)), + retryable = true, + ) + fun seek(positionSeconds: Double) = player( "seek", kwargs = mapOf("new_time" to JsonPrimitive(positionSeconds)), @@ -157,6 +178,24 @@ object Commands { retryable = true, ) + /** + * The box's current MPD queue, one entry per track. + * + * The queue is **never published** — `playermpd` only ever sends `playerstatus` — so this + * is the sole way to learn what is queued behind the playing song. The web UI does not + * call it, which is why `docs/protocol-notes.md` used not to list it, but it is a real + * `@plugs.tag` method on both `future3/main` and `future3/develop`. + * + * It is a synchronous MPD call on the socket the box shares with its card reader, and a + * recursively added artist folder can be thousands of entries — hence the library + * timeout, and hence it is asked once per *queue change* and never on a timer (§6). + */ + val playlistInfo = player( + "playlistinfo", + timeoutMillis = PhonieboxCommand.LIBRARY_TIMEOUT_MILLIS, + retryable = true, + ) + fun singleCoverArt(songUrl: String) = player( "get_single_coverart", kwargs = mapOf("song_url" to JsonPrimitive(songUrl)), diff --git a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxSession.kt b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxSession.kt index b3ddad5..84806d1 100644 --- a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxSession.kt +++ b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/PhonieboxSession.kt @@ -58,6 +58,21 @@ class PhonieboxSession(val box: Box) : AutoCloseable { */ val sleepTimer: StateFlow = _sleepTimer.asStateFlow() + /** + * When this session's SUB socket was opened, on the elapsed-realtime clock. + * + * The last-value cache is what makes a new session cheap for every other topic, and a trap + * for this one. What it replays on the timer topic is the message that *set* the timer, + * carrying the seconds that were left back then and nothing to date it by — so taken at + * face value it restarts the countdown from the top every time the app comes back to the + * foreground and the connection is rebuilt. A timer publish arriving this soon after + * subscribing is therefore read as "there is a timer, go ask what is left of it". + * + * Declared above [subscriber] on purpose: [openSubscriber] writes it. + */ + @Volatile + private var subscribedAtElapsed: Long = 0L + @Volatile private var rpc: ZmqRpcClient = ZmqRpcClient(box.host, box.rpcPort) @@ -78,8 +93,9 @@ class PhonieboxSession(val box: Box) : AutoCloseable { } /** - * Asks for the timer's state. Called when the timer UI opens, never on a schedule: - * the published topic covers every later change. + * Asks for the timer's state: once per connection, when the timer UI opens, and when the + * topic says something whose age cannot be established (see [subscribedAtElapsed]). Never + * on a schedule — the published topic covers every later change. */ suspend fun refreshSleepTimer(): Result = call(Commands.sleepTimerState).map { result -> @@ -97,12 +113,15 @@ class PhonieboxSession(val box: Box) : AutoCloseable { Unit } - private fun openSubscriber() = ZmqStatusSubscriber( - host = box.host, - port = box.pubPort, - topics = ZmqStatusSubscriber.DEFAULT_TOPICS, - onMessage = ::onPublished, - ) + private fun openSubscriber(): ZmqStatusSubscriber { + subscribedAtElapsed = SystemClock.elapsedRealtime() + return ZmqStatusSubscriber( + host = box.host, + port = box.pubPort, + topics = ZmqStatusSubscriber.DEFAULT_TOPICS, + onMessage = ::onPublished, + ) + } /** Called on the subscriber's own thread; every write here is to a StateFlow. */ private fun onPublished(topic: String, payload: String) { @@ -117,9 +136,18 @@ class PhonieboxSession(val box: Box) : AutoCloseable { } } - topic == ZmqStatusSubscriber.TOPIC_SLEEP_TIMER -> - StatusParser.sleepTimer(payload, SystemClock.elapsedRealtime()) - ?.let { _sleepTimer.value = it } + topic == ZmqStatusSubscriber.TOPIC_SLEEP_TIMER -> { + val sinceSubscribed = SystemClock.elapsedRealtime() - subscribedAtElapsed + if (sinceSubscribed < TIMER_REPLAY_WINDOW_MILLIS) { + // A replayed cache entry and a change made this second look identical + // (see [subscribedAtElapsed]), so neither is believed: `get_state` answers + // both cases with a remaining count that is true now. + scope.launch { refreshSleepTimer() } + } else { + StatusParser.sleepTimer(payload, SystemClock.elapsedRealtime()) + ?.let { _sleepTimer.value = it } + } + } topic.startsWith(ZmqStatusSubscriber.TOPIC_VOLUME) -> StatusParser.volume(payload, _volume.value)?.let { _volume.value = it } @@ -145,6 +173,11 @@ class PhonieboxSession(val box: Box) : AutoCloseable { * * The ping is `playerstatus`, so its reply doubles as the initial state — the * last-value cache would deliver that anyway, this just makes it immediate. + * + * The timer is the one piece of state the cache cannot supply, because what it holds is + * dated (see [subscribedAtElapsed]), so it is asked for outright on every connection. A + * running countdown then survives the app being sent to the background and brought back, + * which closes the session and opens a new one. */ private suspend fun handshake() { call(Commands.ping) @@ -165,6 +198,8 @@ class PhonieboxSession(val box: Box) : AutoCloseable { _volume.value = _volume.value.copy(maxLevel = max) } } + + refreshSleepTimer() } /** @@ -234,6 +269,13 @@ class PhonieboxSession(val box: Box) : AutoCloseable { /** 20 s of nothing at all, against an expected four messages a second. */ const val SILENCE_LIMIT_MILLIS = 20_000L + + /** + * How long a timer publish is treated as possibly a replayed cache entry. The cache is + * handed over as the subscription is accepted, so this only has to outlast the socket + * setup — and asking is harmless when it turns out to have been a real change. + */ + const val TIMER_REPLAY_WINDOW_MILLIS = 2_000L const val INITIAL_BACKOFF_MILLIS = 1_000L const val MAX_BACKOFF_MILLIS = 30_000L } diff --git a/core-transport/src/main/kotlin/app/coilforphoniebox/transport/QueueParser.kt b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/QueueParser.kt new file mode 100644 index 0000000..7312409 --- /dev/null +++ b/core-transport/src/main/kotlin/app/coilforphoniebox/transport/QueueParser.kt @@ -0,0 +1,68 @@ +package app.coilforphoniebox.transport + +import app.coilforphoniebox.domain.model.QueueEntry +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject + +/** + * Parses `player.ctrl.playlistinfo` — the box's current MPD queue. + * + * Its own file rather than a method on one of the neighbours, because it belongs to neither: + * [LibraryParser] handles "RPC results that feed the library cache" and a queue is never + * cached, while [StatusParser] handles the published topics and the queue is never published. + * + * Read as leniently as everything else from the box. Keys arrive lower-cased from python-mpd2 + * (`pos`, `title`, `duration`), but MPD's own spelling is capitalised (`Pos:`, `Title:`) and + * the layer in between is not a contract Coil can rely on, so both are accepted — the same + * belt-and-braces [StatusParser] applies when it reads `songid` and `id` (§4.2). + */ +object QueueParser { + + /** + * `playlistinfo` returns one object per queued song: + * `[{"file": "Bibi/01.mp3", "pos": "0", "id": "17", "title": "Kapitel 1", + * "artist": "…", "album": "…", "duration": "243.1"}, …]` + * + * An entry with no `file` is dropped: the URL is the one field the app cannot do + * without — it identifies the row against `playerstatus.file` and there is nothing to + * fall back to. + */ + fun queue(result: JsonElement?): List { + val entries = result as? JsonArray ?: return emptyList() + + return entries.mapIndexedNotNull { index, element -> + val entry = element as? JsonObject ?: return@mapIndexedNotNull null + val url = entry.anyString("file", "File") ?: return@mapIndexedNotNull null + + QueueEntry( + // A missing `pos` falls back to the array index: the queue arrives in play + // order, so the index *is* the position — but the box's own value wins when + // it is there, because that is what `playerstatus` will be compared against. + position = entry.anyInt("pos", "Pos") ?: index, + url = url, + // Most of a Phoniebox library is untagged rips, so the title tag is missing + // far more often than not. The file name is what the library rows show for + // exactly the same reason (`LibraryParser` keeps the entry's `name`). + title = entry.anyString("title", "Title") ?: url.substringAfterLast('/'), + // `albumartist` is the better label within one folder — `artist` is + // per-track and can differ track by track, same as in `playerstatus`. + artist = entry.anyString("albumartist", "AlbumArtist", "artist", "Artist"), + album = entry.anyString("album", "Album"), + // Absent on streams. `time` is the older protocol's integer seconds. + durationSeconds = entry.anyDouble("duration", "Duration") + ?: entry.anyDouble("time", "Time"), + songId = entry.anyString("id", "Id"), + ) + } + } + + private fun JsonObject.anyString(vararg keys: String): String? = + keys.firstNotNullOfOrNull { string(it) } + + private fun JsonObject.anyInt(vararg keys: String): Int? = + keys.firstNotNullOfOrNull { int(it) } + + private fun JsonObject.anyDouble(vararg keys: String): Double? = + keys.firstNotNullOfOrNull { double(it) } +} diff --git a/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt index c8df5ae..dd2152a 100644 --- a/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt +++ b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/CommandsTest.kt @@ -53,6 +53,7 @@ class CommandsTest { Commands.albumCoverArt("X", "A"), Commands.updateLibrary, Commands.play(PlayTarget.Folder("x")), + Commands.playlistInfo, ) commands.forEach { command -> assertNull("${command.name} must not set as_thread", command.toJson("id")["as_thread"]) @@ -106,6 +107,37 @@ class CommandsTest { assertEquals("12.5", Commands.seek(12.5).kwargs["new_time"]?.jsonPrimitive?.content) } + /** + * The queue is a synchronous MPD call on the socket the box shares with its card reader, + * and a recursively added artist folder can be thousands of entries. + */ + @Test + fun `the queue is read with playlistinfo, on the library timeout`() { + assertEquals("player.ctrl.playlistinfo", Commands.playlistInfo.name) + assertNull(Commands.playlistInfo.toJson("id")["kwargs"]) + assertEquals( + PhonieboxCommand.LIBRARY_TIMEOUT_MILLIS, + Commands.playlistInfo.timeoutMillis, + ) + assertTrue(Commands.playlistInfo.retryable) + } + + /** + * `playAt` is `play` with a `pos` kwarg, which upstream `future3` does not accept yet — + * `def play(self)` on both branches. The name and the argument have to match the patch + * exactly, because a box that rejects it is how the fallback is triggered. + */ + @Test + fun `playAt is play with a pos kwarg`() { + val json = Commands.playAt(7).toJson("id") + + assertEquals("player", json["package"]?.jsonPrimitive?.content) + assertEquals("ctrl", json["plugin"]?.jsonPrimitive?.content) + assertEquals("play", json["method"]?.jsonPrimitive?.content) + assertEquals("7", json["kwargs"]?.jsonObject?.get("pos")?.jsonPrimitive?.content) + assertTrue("an absolute position survives a retry", Commands.playAt(7).retryable) + } + @Test fun `album playback needs both keys`() { val command = Commands.play(PlayTarget.Album("Bibi Blocksberg", "Hexerei")) @@ -182,6 +214,8 @@ class CommandsTest { Commands.singleCoverArt("x"), Commands.albumCoverArt("x", "y"), Commands.play(PlayTarget.Folder("x")), + Commands.playlistInfo, + Commands.playAt(0), ).map { it.name } assertTrue(everyName.none { it.contains("shutdown") }) diff --git a/core-transport/src/test/kotlin/app/coilforphoniebox/transport/QueueParserTest.kt b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/QueueParserTest.kt new file mode 100644 index 0000000..0b895f8 --- /dev/null +++ b/core-transport/src/test/kotlin/app/coilforphoniebox/transport/QueueParserTest.kt @@ -0,0 +1,126 @@ +package app.coilforphoniebox.transport + +import kotlinx.serialization.json.Json +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class QueueParserTest { + + private fun parse(raw: String) = Json.parseToJsonElement(raw) + + @Test + fun `a queue keeps the box's own positions and order`() { + val raw = """ + [ + {"file":"Audiobooks/Bibi/01.mp3","pos":"0","id":"17","title":"Kapitel 1", + "albumartist":"Bibi Blocksberg","album":"Hexerei","duration":"243.5"}, + {"file":"Audiobooks/Bibi/02.mp3","pos":"1","id":"18","title":"Kapitel 2", + "albumartist":"Bibi Blocksberg","album":"Hexerei","duration":"301"} + ] + """.trimIndent() + + val queue = QueueParser.queue(parse(raw)) + + assertEquals(2, queue.size) + assertEquals(0, queue[0].position) + assertEquals("Audiobooks/Bibi/01.mp3", queue[0].url) + assertEquals("Kapitel 1", queue[0].title) + assertEquals("Bibi Blocksberg", queue[0].artist) + assertEquals("Hexerei", queue[0].album) + assertEquals(243.5, queue[0].durationSeconds!!, 0.001) + assertEquals("17", queue[0].songId) + + assertEquals(1, queue[1].position) + assertEquals(301.0, queue[1].durationSeconds!!, 0.001) + } + + /** + * python-mpd2 lower-cases the keys, but MPD's own spelling is capitalised and the layer in + * between is not a contract Coil can rely on. + */ + @Test + fun `capitalised MPD keys parse the same as lower-cased ones`() { + val raw = """ + [{"File":"A/01.mp3","Pos":"3","Id":"9","Title":"Track","Album":"X","Duration":"12.5"}] + """.trimIndent() + + val entry = QueueParser.queue(parse(raw)).single() + + assertEquals("A/01.mp3", entry.url) + assertEquals(3, entry.position) + assertEquals("Track", entry.title) + assertEquals("X", entry.album) + assertEquals("9", entry.songId) + assertEquals(12.5, entry.durationSeconds!!, 0.001) + } + + /** + * Most of a Phoniebox library is untagged rips, so this is the common case rather than an + * edge one — a row with no text at all would be unusable. + */ + @Test + fun `an untagged song falls back to its file name`() { + val raw = """[{"file":"Audiobooks/Folge 12/03 - Track.mp3","pos":"2"}]""" + + val entry = QueueParser.queue(parse(raw)).single() + + assertEquals("03 - Track.mp3", entry.title) + assertNull(entry.artist) + assertNull(entry.album) + assertNull(entry.durationSeconds) + assertNull(entry.songId) + } + + /** A stream has no duration, the same way it has no `duration` in `playerstatus`. */ + @Test + fun `a stream entry parses without a duration`() { + val raw = """[{"file":"http://host/station.mp3","pos":"0","title":"Radio"}]""" + + val entry = QueueParser.queue(parse(raw)).single() + + assertEquals("Radio", entry.title) + assertNull(entry.durationSeconds) + } + + /** The older protocol spells the duration `time`, in whole seconds. */ + @Test + fun `time stands in for a missing duration`() { + val raw = """[{"file":"A/01.mp3","pos":"0","time":"180"}]""" + + assertEquals(180.0, QueueParser.queue(parse(raw)).single().durationSeconds!!, 0.001) + } + + /** + * The queue arrives in play order, so the index is the position — but only as a fallback, + * because it is the box's own value that `playerstatus.playlistPosition` is compared to. + */ + @Test + fun `a missing position falls back to the index`() { + val raw = """[{"file":"A/01.mp3"},{"file":"A/02.mp3"}]""" + + val queue = QueueParser.queue(parse(raw)) + + assertEquals(0, queue[0].position) + assertEquals(1, queue[1].position) + } + + /** The one field with no fallback: without a URL the row cannot be matched or played. */ + @Test + fun `an entry with no file is dropped rather than shown blank`() { + val raw = """[{"file":"A/01.mp3"},{"pos":"1","title":"Ghost"},{"file":"A/03.mp3"}]""" + + val queue = QueueParser.queue(parse(raw)) + + assertEquals(2, queue.size) + assertTrue(queue.none { it.title == "Ghost" }) + } + + @Test + fun `an empty or unusable payload is an empty queue, not a failure`() { + assertTrue(QueueParser.queue(parse("[]")).isEmpty()) + assertTrue(QueueParser.queue(parse("""{"state":"stop"}""")).isEmpty()) + assertTrue(QueueParser.queue(null).isEmpty()) + } +} diff --git a/docs/pages/assets/screenshots/01_player.png b/docs/pages/assets/screenshots/01_player.png index df32bd0..81297fb 100644 Binary files a/docs/pages/assets/screenshots/01_player.png and b/docs/pages/assets/screenshots/01_player.png differ diff --git a/docs/pages/assets/screenshots/05_sleep_timer.png b/docs/pages/assets/screenshots/05_sleep_timer.png index 60c0b29..1dad47a 100644 Binary files a/docs/pages/assets/screenshots/05_sleep_timer.png and b/docs/pages/assets/screenshots/05_sleep_timer.png differ diff --git a/docs/pages/assets/screenshots/tablet10/01_player.png b/docs/pages/assets/screenshots/tablet10/01_player.png index 82220a5..35c86a8 100644 Binary files a/docs/pages/assets/screenshots/tablet10/01_player.png and b/docs/pages/assets/screenshots/tablet10/01_player.png differ diff --git a/docs/pages/assets/screenshots/tablet10/05_sleep_timer.png b/docs/pages/assets/screenshots/tablet10/05_sleep_timer.png index 445f440..6e1466f 100644 Binary files a/docs/pages/assets/screenshots/tablet10/05_sleep_timer.png and b/docs/pages/assets/screenshots/tablet10/05_sleep_timer.png differ diff --git a/docs/pages/assets/screenshots/tablet7/01_player.png b/docs/pages/assets/screenshots/tablet7/01_player.png index e07a30c..f282325 100644 Binary files a/docs/pages/assets/screenshots/tablet7/01_player.png and b/docs/pages/assets/screenshots/tablet7/01_player.png differ diff --git a/docs/pages/assets/screenshots/tablet7/05_sleep_timer.png b/docs/pages/assets/screenshots/tablet7/05_sleep_timer.png index c347d44..d0ec892 100644 Binary files a/docs/pages/assets/screenshots/tablet7/05_sleep_timer.png and b/docs/pages/assets/screenshots/tablet7/05_sleep_timer.png differ diff --git a/docs/protocol-notes.md b/docs/protocol-notes.md index 859e09d..5822bc5 100644 --- a/docs/protocol-notes.md +++ b/docs/protocol-notes.md @@ -124,7 +124,13 @@ Full reference: `src/webapp/src/commands/index.js` in the Phoniebox repository. ### Content — `player.ctrl.*` -`get_folder_content`, `list_albums`, `list_songs_by_artist_and_album(albumartist, album)`, `get_single_coverart`, `get_album_coverart`, `play_folder(folder)`, `play_album(albumartist, album)`, `play_single(song_url)`, `update`, `update_wait` +`get_folder_content`, `list_albums`, `list_songs_by_artist_and_album(albumartist, album)`, `get_single_coverart`, `get_album_coverart`, `play_folder(folder)`, `play_album(albumartist, album)`, `play_single(song_url)`, `playlistinfo`, `update`, `update_wait` + +`playlistinfo` returns the **current MPD queue**, one object per track with `file`, `pos`, `id`, +`title`, `artist`, `album`, `track` and `duration`. It is a real `@plugs.tag` method on both +`future3/main` and `future3/develop`, but the web UI never calls it, so it is absent from +`src/webapp/src/commands/index.js` — do not take that file as the full surface. The queue is never +published on a topic, so this is the only way to learn it. Do **not** use `list_all_dirs` — the repository explicitly warns about memory consumption on large libraries. Use `get_folder_content` lazily, one level at a time. @@ -136,6 +142,33 @@ library root. `shuffle(option)` and `repeat(option)` take named strings, not MPD flags — `enable`/`disable` for shuffle, `disable`/`enable_repeat`/`enable_repeat_single` for repeat. +### There is no way to play a queue position + +MPD has `play `, and `playermpd` **uses it internally** — `_next_in_stopped_state` calls +`self.mpd_client.play(pos)`. It is simply never exposed over RPC. Every route was checked on +`future3/main` and `future3/develop`; all of them are closed: + +| Route | Why not | +|---|---| +| `play` | `def play(self)` — no arguments, on both branches | +| `resume` | The only RPC that *does* jump (`mpd_client.seek(songpos, elapsed)`), but `songpos` is read from `current_folder_status["CURRENTSONGPOS"]`, internal state written by the box's own status poll, and **no RPC writes it** | +| `map_filename_to_playlist_pos`, `remove`, `move` | all three `raise NotImplementedError` | +| `queue_load(folder)` | body is `pass` — a stub | +| `seek(new_time)` | `mpd_client.seekcur(...)`, so time within the current song only | +| `play_single(song_url)` | `clear()` + `addid()` + `play()`: **destroys the queue**, leaving one track, so next/prev stop working and the box falls silent at its end. Never use it to reach a track inside something already playing | +| `misc.py` | no generic passthrough; nothing can invoke an arbitrary method or argument | +| `rpc_command_alias.py` | aliases only rename existing methods; no positional variant | +| MPD on 6600 | `mpd.default.conf` has `bind_to_address "localhost"` — unreachable from the LAN | +| an `.m3u` in `shared/playlists/` | no filesystem access to the box, and no `load` RPC to read one | + +So Coil walks the queue with `next`/`prev` instead — see AGENTS.md, "The queue, and why skipping +into it is stepped". Sending `play` with a `pos` kwarg anyway is a **safe capability probe**: the +plugin's signature rejects it before its body runs, and `jukebox/rpc/server.py` wraps `plugs.call` +in a try/except, so the reply is +`{"error": {"message": "TypeError: play() got an unexpected keyword argument 'pos'"}}` and playback +is untouched. Note this is also why an *unknown method name* and an *unknown argument* look alike +from the outside: both come back as an error reply, not a crash. + `update` triggers the MPD database scan. `update_wait` blocks until completion and should be avoided because of the shared socket. ### Timers — `timers.*` diff --git a/fastlane/metadata/android/de-DE/changelogs/6.txt b/fastlane/metadata/android/de-DE/changelogs/6.txt new file mode 100644 index 0000000..68e0ed8 --- /dev/null +++ b/fastlane/metadata/android/de-DE/changelogs/6.txt @@ -0,0 +1,3 @@ +Der Player zeigt jetzt die Playlist, aus der die Box spielt. Die neue Schaltfläche listet alle Titel auf und markiert den laufenden; ein Tippen auf eine Zeile springt direkt dorthin — Kapitel sieben bedeutet nicht mehr sechsmal Weiter. + +Ein laufender Einschlaftimer zählt beim erneuten Öffnen von Coil richtig weiter. diff --git a/fastlane/metadata/android/en-US/changelogs/6.txt b/fastlane/metadata/android/en-US/changelogs/6.txt new file mode 100644 index 0000000..caf54f5 --- /dev/null +++ b/fastlane/metadata/android/en-US/changelogs/6.txt @@ -0,0 +1,3 @@ +The player now shows the playlist your box is playing from. The new playlist button lists every track with the playing one marked, and tapping a row goes straight there — chapter seven no longer means tapping skip six times. + +A running sleep timer now keeps its countdown when you reopen Coil. diff --git a/fastlane/metadata/android/en-US/images/phoneScreenshots/01_player.png b/fastlane/metadata/android/en-US/images/phoneScreenshots/01_player.png index df32bd0..81297fb 100644 Binary files a/fastlane/metadata/android/en-US/images/phoneScreenshots/01_player.png and b/fastlane/metadata/android/en-US/images/phoneScreenshots/01_player.png differ diff --git a/fastlane/metadata/android/en-US/images/phoneScreenshots/05_sleep_timer.png b/fastlane/metadata/android/en-US/images/phoneScreenshots/05_sleep_timer.png index 60c0b29..1dad47a 100644 Binary files a/fastlane/metadata/android/en-US/images/phoneScreenshots/05_sleep_timer.png and b/fastlane/metadata/android/en-US/images/phoneScreenshots/05_sleep_timer.png differ diff --git a/fastlane/metadata/android/en-US/images/sevenInchScreenshots/01_player.png b/fastlane/metadata/android/en-US/images/sevenInchScreenshots/01_player.png index e07a30c..f282325 100644 Binary files a/fastlane/metadata/android/en-US/images/sevenInchScreenshots/01_player.png and b/fastlane/metadata/android/en-US/images/sevenInchScreenshots/01_player.png differ diff --git a/fastlane/metadata/android/en-US/images/sevenInchScreenshots/05_sleep_timer.png b/fastlane/metadata/android/en-US/images/sevenInchScreenshots/05_sleep_timer.png index c347d44..d0ec892 100644 Binary files a/fastlane/metadata/android/en-US/images/sevenInchScreenshots/05_sleep_timer.png and b/fastlane/metadata/android/en-US/images/sevenInchScreenshots/05_sleep_timer.png differ diff --git a/fastlane/metadata/android/en-US/images/tenInchScreenshots/01_player.png b/fastlane/metadata/android/en-US/images/tenInchScreenshots/01_player.png index 82220a5..35c86a8 100644 Binary files a/fastlane/metadata/android/en-US/images/tenInchScreenshots/01_player.png and b/fastlane/metadata/android/en-US/images/tenInchScreenshots/01_player.png differ diff --git a/fastlane/metadata/android/en-US/images/tenInchScreenshots/05_sleep_timer.png b/fastlane/metadata/android/en-US/images/tenInchScreenshots/05_sleep_timer.png index 445f440..6e1466f 100644 Binary files a/fastlane/metadata/android/en-US/images/tenInchScreenshots/05_sleep_timer.png and b/fastlane/metadata/android/en-US/images/tenInchScreenshots/05_sleep_timer.png differ diff --git a/fastlane/metadata/android/es-ES/changelogs/6.txt b/fastlane/metadata/android/es-ES/changelogs/6.txt new file mode 100644 index 0000000..32ebfd9 --- /dev/null +++ b/fastlane/metadata/android/es-ES/changelogs/6.txt @@ -0,0 +1,3 @@ +El reproductor ya muestra la lista de reproducción que suena en la caja. El nuevo botón enumera todas las pistas y marca la que suena; al tocar una fila se salta directamente a ella, así que el capítulo siete ya no exige pulsar Siguiente seis veces. + +El temporizador sigue su cuenta atrás al volver a abrir Coil. diff --git a/fastlane/metadata/android/fr-FR/changelogs/6.txt b/fastlane/metadata/android/fr-FR/changelogs/6.txt new file mode 100644 index 0000000..5d66d52 --- /dev/null +++ b/fastlane/metadata/android/fr-FR/changelogs/6.txt @@ -0,0 +1,3 @@ +Le lecteur affiche désormais la playlist en cours sur la box. Le nouveau bouton liste tous les titres et signale celui en cours ; toucher une ligne y accède directement — le chapitre sept ne demande plus six appuis sur Suivant. + +Une minuterie d'arrêt en cours reprend son décompte à la réouverture de Coil. diff --git a/fastlane/metadata/android/nl-NL/changelogs/6.txt b/fastlane/metadata/android/nl-NL/changelogs/6.txt new file mode 100644 index 0000000..02ad5af --- /dev/null +++ b/fastlane/metadata/android/nl-NL/changelogs/6.txt @@ -0,0 +1,3 @@ +De speler toont nu de playlist waaruit de box speelt. De nieuwe knop laat alle nummers zien en markeert het huidige; tik op een regel en je springt er meteen naartoe — hoofdstuk zeven kost niet langer zes keer Volgende. + +Een lopende slaaptimer telt bij het opnieuw openen van Coil gewoon verder. diff --git a/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt b/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt index 655934d..9004142 100644 --- a/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt +++ b/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt @@ -11,6 +11,7 @@ import androidx.media3.common.util.UnstableApi import app.coilforphoniebox.domain.model.Box import app.coilforphoniebox.domain.model.PlaybackState import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry import app.coilforphoniebox.domain.model.RepeatMode import app.coilforphoniebox.domain.model.VolumeStatus import app.coilforphoniebox.domain.repository.BoxRepository @@ -23,6 +24,33 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch +/** + * Where [status] sits in [queue], or null when the two cannot be reconciled. + * + * The queue and the status arrive independently — the status four times a second, the queue once + * per queue change — so there is a real window in which the box has moved on to a different + * album and the cached queue still describes the old one. Two things go wrong if that window is + * not guarded, and they are of different severity: + * + * - `SimpleBasePlayer` **throws** if `currentMediaItemIndex` falls outside the playlist, so a + * queue shorter than the reported position would crash the media session rather than look odd. + * - A position that is merely *pointing at the wrong track* puts another album's title on the + * lock screen, which is quieter and arguably worse for being believable. + * + * So both are checked: the index has to exist, and the entry there has to be the file the box + * says it is playing. A null sends the caller back to the single-item timeline, which is what + * shipped before there was a queue at all and is always safe. + * + * A top-level function because it is the load-bearing part of this file and worth testing on its + * own — `getState()` needs a `Looper` and this does not. + */ +internal fun timelineIndexFor(queue: List, status: PlayerStatus): Int? { + val position = status.playlistPosition ?: return null + if (position !in queue.indices) return null + if (queue[position].url != status.file) return null + return position +} + /** * A media3 player whose playback happens somewhere else entirely. * @@ -47,6 +75,7 @@ class PhonieboxPlayer( val coverUrl: String? = null, val activeBox: Box? = null, val boxCount: Int = 0, + val queue: List = emptyList(), ) @Volatile @@ -54,19 +83,25 @@ class PhonieboxPlayer( init { scope.launch { + // Nested rather than one call: `combine` is only typed up to five flows, and the + // queue arrives on its own schedule anyway — once per queue change, not per status. combine( - player.status, - player.volume, - player.coverUrl, - boxes.activeBox, - boxes.boxes.map { it.size }.distinctUntilChanged(), - ) { status, volume, coverUrl, box, boxCount -> - Snapshot(status, volume, coverUrl, box, boxCount) - }.collect { next -> - snapshot = next - // Must happen on the player's own thread, which this scope runs on. - invalidateState() - } + combine( + player.status, + player.volume, + player.coverUrl, + boxes.activeBox, + boxes.boxes.map { it.size }.distinctUntilChanged(), + ) { status, volume, coverUrl, box, boxCount -> + Snapshot(status, volume, coverUrl, box, boxCount) + }, + player.queue, + ) { base, queue -> base.copy(queue = queue) } + .collect { next -> + snapshot = next + // Must happen on the player's own thread, which this scope runs on. + invalidateState() + } } } @@ -98,10 +133,25 @@ class PhonieboxPlayer( .setDeviceVolume(current.volume.level.coerceAtLeast(0)) .setIsDeviceMuted(current.volume.muted) - if (status.hasContent) { + // The real queue when it can be trusted, the single playing song otherwise. The index + // has to be inside the playlist or `SimpleBasePlayer` throws, which is why the decision + // lives in one checked function rather than being spelled out here. + val index = timelineIndexFor(current.queue, status) + if (index != null) { + builder.setPlaylist( + current.queue.mapIndexed { position, entry -> + // The playing item keeps being built from `playerstatus`: that is the + // authoritative metadata, and the only item there is cover art for. + if (position == index) mediaItemFor(current) else mediaItemFor(entry) + }, + ) + builder.setCurrentMediaItemIndex(index) + } else if (status.hasContent) { builder.setPlaylist(listOf(mediaItemFor(current))) builder.setCurrentMediaItemIndex(0) + } + if (status.hasContent) { val positionMs = ((status.elapsedSeconds ?: 0.0) * 1000).toLong().coerceAtLeast(0L) builder.setContentPositionMs( // Published four times a second, which is smooth enough on its own — @@ -117,6 +167,42 @@ class PhonieboxPlayer( return builder.build() } + /** + * One queued track that is not the playing one. + * + * No artwork: only the current song's cover is ever resolved, because asking the box for a + * cover per queued track would be exactly the RPC storm §6 forbids. A queue list with no + * thumbnails is a fair trade for a queue list at all. + */ + private fun mediaItemFor(entry: QueueEntry): MediaItemData { + val durationUs = entry.durationSeconds + ?.takeIf { it > 0 } + ?.let { (it * 1_000_000).toLong() } + ?: androidx.media3.common.C.TIME_UNSET + + val metadata = MediaMetadata.Builder() + .setTitle(entry.title) + .setArtist(entry.artist) + .setAlbumTitle(entry.album) + .setIsBrowsable(false) + .setIsPlayable(true) + .build() + + // MPD's queue id is unique within the queue; without one, the position keeps two + // copies of the same file in one queue from colliding — media3 requires unique UIDs. + return MediaItemData.Builder(entry.songId ?: "${entry.position}:${entry.url}") + .setMediaItem( + MediaItem.Builder() + .setMediaId(entry.url) + .setMediaMetadata(metadata) + .build(), + ) + .setDurationUs(durationUs) + .setIsSeekable(durationUs != androidx.media3.common.C.TIME_UNSET) + .setIsDynamic(false) + .build() + } + private fun mediaItemFor(current: Snapshot): MediaItemData { val status = current.status val durationUs = status.durationSeconds @@ -170,6 +256,12 @@ class PhonieboxPlayer( Player.COMMAND_SEEK_TO_PREVIOUS, Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM -> player.previous() + // A seek naming an item is a jump to that queue position — a tap in Android Auto's + // queue, or "play track five". Any `positionMs` that came with it is dropped: the + // box has no way to arrive at a position *and* an offset in one command, and + // getting to the right track matters more than starting it at second 30. + Player.COMMAND_SEEK_TO_MEDIA_ITEM -> player.playAt(mediaItemIndex) + else -> player.seekTo(positionMs / 1000.0) } } @@ -222,6 +314,9 @@ class PhonieboxPlayer( Player.COMMAND_SEEK_TO_PREVIOUS, Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM, Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, + // Offered even though the box has no command for it: `playAt` walks the queue + // where it must, so a tap in a queue list always does something honest. + Player.COMMAND_SEEK_TO_MEDIA_ITEM, Player.COMMAND_GET_TIMELINE, Player.COMMAND_GET_METADATA, Player.COMMAND_SET_SHUFFLE_MODE, diff --git a/feature-media/src/test/kotlin/app/coilforphoniebox/media/TimelineIndexTest.kt b/feature-media/src/test/kotlin/app/coilforphoniebox/media/TimelineIndexTest.kt new file mode 100644 index 0000000..e4de9f7 --- /dev/null +++ b/feature-media/src/test/kotlin/app/coilforphoniebox/media/TimelineIndexTest.kt @@ -0,0 +1,69 @@ +package app.coilforphoniebox.media + +import app.coilforphoniebox.domain.model.PlayerStatus +import app.coilforphoniebox.domain.model.QueueEntry +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * The queue and `playerstatus` arrive independently, so every disagreement between them has to + * end up on the safe side of `SimpleBasePlayer`, which throws when the current index falls + * outside the playlist. + */ +class TimelineIndexTest { + + private fun queue(vararg urls: String) = urls.mapIndexed { index, url -> + QueueEntry(position = index, url = url, title = url.substringAfterLast('/')) + } + + private fun status(file: String?, position: Int?) = + PlayerStatus(file = file, playlistPosition = position, playlistLength = 3) + + @Test + fun `a queue that matches the status gives its position`() { + val index = timelineIndexFor( + queue("A/01.mp3", "A/02.mp3", "A/03.mp3"), + status("A/02.mp3", 1), + ) + + assertEquals(1, index) + } + + /** The crash case: the box moved to a longer album before the refetch landed. */ + @Test + fun `a position past the end of the cached queue is refused`() { + assertNull(timelineIndexFor(queue("A/01.mp3"), status("B/07.mp3", 6))) + } + + /** + * The quiet case, and the more insidious one: the index exists but describes a track from + * the album that was playing a moment ago. + */ + @Test + fun `a queue of the right length but the wrong album is refused`() { + assertNull( + timelineIndexFor( + queue("A/01.mp3", "A/02.mp3", "A/03.mp3"), + status("B/02.mp3", 1), + ), + ) + } + + @Test + fun `an empty queue is refused whatever the status says`() { + assertNull(timelineIndexFor(emptyList(), status("A/01.mp3", 0))) + assertNull(timelineIndexFor(emptyList(), status(null, null))) + } + + /** A stopped box, or one whose status has not arrived, reports no position. */ + @Test + fun `no reported position is refused`() { + assertNull(timelineIndexFor(queue("A/01.mp3", "A/02.mp3"), status("A/01.mp3", null))) + } + + @Test + fun `a negative position is refused rather than clamped`() { + assertNull(timelineIndexFor(queue("A/01.mp3", "A/02.mp3"), status("A/01.mp3", -1))) + } +}