From 579ce0f50b510dfcc8f15ade12fcfca21f689c7d Mon Sep 17 00:00:00 2001 From: Nico Wiedemann Date: Tue, 11 Aug 2026 16:55:14 +0200 Subject: [PATCH 1/4] fix(media): register the session so its notification appears MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MediaNotificationManager.updateNotification asks isSessionAdded before anything else, and media3 registers a session itself only for a bind carrying its own service action or an incoming media button intent — onBind returns null for any other action without ever consulting onGetSession. Coil built its session, returned it from onGetSession and never called addSession, and its own binding used a bare component intent, so the session was never registered and no media notification was ever posted in either session mode. The same early return calls maybeStopForegroundService(removeNotifications = true), which cancels media3's notification id — the id the quiet "ready" notification shared — so automatic mode could also be left as a foreground service with nothing on screen at all. That notification now has an id of its own and follows whether the box has anything loaded, which is the exact complement of the condition media3 posts under: one of the two is up and never both, whichever collector runs first. A paused box keeps its controls, which is when they are wanted most. Two smaller repairs alongside: the bind now carries MediaSessionService.SERVICE_INTERFACE, so the bound-client count is scoring a connection the platform actually made; and the service stops itself only after a moment's grace, because currentStatus() lags a command by a publish interval and tapping play before pocketing the phone would tear the session down exactly as the notification was about to appear. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 + .../media/MediaSessionBinder.kt | 10 +- .../media/PhonieboxMediaService.kt | 134 +++++++++++++++--- 3 files changed, 129 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6a3439..3ed6780 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,13 @@ automatically from the `## [x.y.z]` heading matching `versionName` in `app/build ## [Unreleased] +### Fixed +- **The playback controls now actually appear.** Coil is supposed to put the playing track in your + notification shade and on your lock screen, and it never did — the session was built correctly and + then never handed to the part of media3 that posts the notification, so nothing was ever shown in + either of the two control modes. In automatic mode the same fault could leave the quiet "ready" + notification cancelled behind it + ## [1.1.0] - 2026-08-11 ### Added diff --git a/feature-media/src/main/kotlin/app/coilforphoniebox/media/MediaSessionBinder.kt b/feature-media/src/main/kotlin/app/coilforphoniebox/media/MediaSessionBinder.kt index 49aa128..e65fd03 100644 --- a/feature-media/src/main/kotlin/app/coilforphoniebox/media/MediaSessionBinder.kt +++ b/feature-media/src/main/kotlin/app/coilforphoniebox/media/MediaSessionBinder.kt @@ -6,6 +6,7 @@ import android.content.ServiceConnection import android.os.IBinder import android.util.Log import androidx.media3.common.util.UnstableApi +import androidx.media3.session.MediaSessionService import app.coilforphoniebox.domain.model.SessionMode import app.coilforphoniebox.domain.repository.SettingsRepository import dagger.hilt.android.qualifiers.ApplicationContext @@ -83,11 +84,18 @@ class MediaSessionBinder @Inject constructor( if (wanted && allowed) bindNow() else unbindNow() } + /** + * The action matters. `MediaSessionService.onBind` answers only its own service action and + * the legacy browser one, and returns null for anything else — so an actionless bind keeps + * the service alive without ever establishing a connection, which left the bound-client + * count here scoring a binding the platform had not made. + */ private fun bindNow() { if (bound) return bound = runCatching { context.bindService( - PhonieboxMediaService.serviceIntent(context), + PhonieboxMediaService.serviceIntent(context) + .setAction(MediaSessionService.SERVICE_INTERFACE), connection, Context.BIND_AUTO_CREATE, ) diff --git a/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxMediaService.kt b/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxMediaService.kt index 5d82065..9f74013 100644 --- a/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxMediaService.kt +++ b/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxMediaService.kt @@ -26,8 +26,10 @@ import app.coilforphoniebox.transport.ConnectionManager import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch @@ -37,8 +39,9 @@ import javax.inject.Inject * Holds the media session, and with it the connection to the box. * * In the default "only while Coil is open" mode the app binds to this service, so it - * lives exactly as long as the UI and posts a notification only once the box actually - * plays — media3 handles that transition. In automatic mode the service is started + * lives exactly as long as the UI and posts a notification only once the box has something + * loaded — media3 handles that transition, provided the session was handed to it with + * [addSession], without which it posts nothing at all. In automatic mode the service is started * outright and keeps a low-priority notification up while it waits, because a foreground * service has to be visible (§8.3). When controls are switched off entirely the service is * not meant to exist at all: nothing binds or starts it, and it stops itself if it is @@ -66,6 +69,9 @@ class PhonieboxMediaService : MediaSessionService() { private var automatic = false private var networkCallback: ConnectivityManager.NetworkCallback? = null + /** The pending "nothing needs this any more" stop, so a rebind can call it off. */ + private var stopJob: Job? = null + /** * How many clients are bound. In the default mode the service should exist while the * UI does and while something plays, and not a moment longer — a connection nobody is @@ -83,8 +89,8 @@ class PhonieboxMediaService : MediaSessionService() { val player = PhonieboxPlayer(scope, playerRepository, boxRepository) phonieboxPlayer = player - // Shares its notification id with the quiet status notification below, so the - // media notification simply replaces it once playback starts. + // An id of its own, and media3 owns it: it cancels that id whenever it has nothing to + // show, which is why the quiet status notification cannot live there too. setMediaNotificationProvider( DefaultMediaNotificationProvider.Builder(this) .setChannelId(CHANNEL_PLAYBACK) @@ -93,9 +99,32 @@ class PhonieboxMediaService : MediaSessionService() { .apply { setSmallIcon(texts.smallIcon) }, ) - session = MediaSession.Builder(this, player) + val built = MediaSession.Builder(this, player) .apply { launchIntent()?.let { setSessionActivity(it) } } .build() + session = built + + // media3 shows nothing for a session it has not been handed: the first thing + // `MediaNotificationManager.updateNotification` asks is `isSessionAdded`, and media3 + // only registers a session itself when a controller binds with its own service action + // or a media button intent arrives. Coil's binding is neither, so without this line the + // notification is never posted at all — and the same early return cancels the id below. + addSession(built) + + // media3 catches Android's refusal to promote the service and reports it here; with no + // listener set it is swallowed, and the symptom is controls that silently never appear. + setListener( + object : MediaSessionService.Listener { + override fun onForegroundServiceStartNotAllowedException() { + Log.w(TAG, "Android refused to start the media session in the foreground") + // Automatic mode has to show something, and this notification is allowed + // where a foreground promotion is not. + if (automatic) { + notificationManager().notify(STATUS_NOTIFICATION_ID, statusNotification()) + } + } + }, + ) observeIdleState() observeSessionModeSetting() @@ -109,13 +138,22 @@ class PhonieboxMediaService : MediaSessionService() { if (intent?.action == ACTION_START_AUTOMATIC) { automatic = true // Started with startForegroundService, so something visible has to go up now. - startForeground(NOTIFICATION_ID, statusNotification()) + promoteWithStatusNotification() watchNetwork() } return result } + /** + * Automatic mode is a foreground service by contract (§8.3) — it was started outright and + * has to stay visible whether or not the box happens to be playing this second. media3 + * detaches the service as soon as playback stops, so the requirement is forced back on here. + */ + override fun onUpdateNotification(session: MediaSession, startInForegroundRequired: Boolean) { + super.onUpdateNotification(session, startInForegroundRequired || automatic) + } + override fun onBind(intent: Intent?): IBinder? { boundClients++ return super.onBind(intent) @@ -133,32 +171,69 @@ class PhonieboxMediaService : MediaSessionService() { } /** - * While automatic mode waits for the box to start playing there is nothing for media3 - * to show, but the service still has to be visible. Once playback begins, media3's own - * notification takes over the same id, and this quiet one goes with it. + * Keeps the quiet status notification in step with media3's own, and ends the service in + * the default mode when there is nothing left to do. * - * The same signal ends the service in the default mode: nothing playing, nobody bound, - * nothing to do. + * The status notification follows whether the box has anything *loaded*, not whether it is + * playing. `hasContent` is the exact complement of the condition media3 posts under — + * `MediaNotificationManager.shouldShowNotification` wants a non-empty timeline and a state + * other than idle — so one of the two is on screen and never both, and which one no longer + * depends on the order two collectors happen to run in. A *paused* box keeps its media + * controls, which is when they are wanted most. */ private fun observeIdleState() { scope.launch { playerRepository.status - .map { it.state } + .map { it.hasContent to it.state } .distinctUntilChanged() - .collect { state -> - if (automatic && state != PlaybackState.PLAY) { - notificationManager().notify(NOTIFICATION_ID, statusNotification()) - } + .collect { (hasContent, state) -> + if (automatic) updateStatusNotification(hasContent) if (state != PlaybackState.PLAY) stopIfNoLongerNeeded() } } } + private fun updateStatusNotification(hasContent: Boolean) { + if (hasContent) { + // media3 has the shade from here, on its own id. Two notifications for one box + // would be one too many. + notificationManager().cancel(STATUS_NOTIFICATION_ID) + } else { + // `startForeground` rather than `notify`: media3 has just detached the service and + // taken its notification down, and automatic mode has to stay foreground. + promoteWithStatusNotification() + } + } + + private fun promoteWithStatusNotification() { + runCatching { startForeground(STATUS_NOTIFICATION_ID, statusNotification()) } + .onFailure { Log.w(TAG, "Could not keep the automatic session in the foreground", it) } + } + + /** + * Ends the service when nothing needs it, after a moment's grace. + * + * The grace is the point. `currentStatus()` is what the box last *published*, which lags a + * command by up to a quarter of a second and by however long the box's own sequential + * socket takes (§6) — so a user who taps play and pockets the phone unbinds the UI while + * the box still reports "not playing", and stopping on that reading would tear the session + * down exactly as the notification was about to appear. + */ private fun stopIfNoLongerNeeded() { - if (automatic || boundClients > 0) return - if (connectionManager.currentStatus().state == PlaybackState.PLAY) return - Log.i(TAG, "Nothing playing and nothing bound; stopping") - stopSelf() + if (automatic || boundClients > 0) { + stopJob?.cancel() + stopJob = null + return + } + + stopJob?.cancel() + stopJob = scope.launch { + delay(STOP_GRACE_MILLIS) + if (automatic || boundClients > 0) return@launch + if (connectionManager.currentStatus().state == PlaybackState.PLAY) return@launch + Log.i(TAG, "Nothing playing and nothing bound; stopping") + stopSelf() + } } /** @@ -240,6 +315,11 @@ class PhonieboxMediaService : MediaSessionService() { } override fun onDestroy() { + stopJob?.cancel() + stopJob = null + + clearListener() + networkCallback?.let { callback -> runCatching { getSystemService(ConnectivityManager::class.java)?.unregisterNetworkCallback(callback) @@ -296,9 +376,21 @@ class PhonieboxMediaService : MediaSessionService() { const val CHANNEL_PLAYBACK = "playback" const val CHANNEL_STATUS = "connection" - /** Shared with media3's own notification, so one replaces the other. */ + /** media3's own, and media3's alone — it cancels this id whenever it has nothing to show. */ const val NOTIFICATION_ID = 1001 + /** + * The quiet "ready" notification automatic mode waits behind. + * + * An id of its own rather than media3's: sharing one meant two writers with no ordering + * between them, and media3 cancelling the id was enough to leave a foreground service + * with no notification at all. + */ + const val STATUS_NOTIFICATION_ID = 1002 + + /** Long enough for a command in flight to come back as a published status (§8.1). */ + private const val STOP_GRACE_MILLIS = 2_000L + const val ACTION_START_AUTOMATIC = "app.coilforphoniebox.media.START_AUTOMATIC" fun automaticIntent(context: Context): Intent = From b8fc1ccf050fb903214d2606d7fc7e6bd46ecfb7 Mon Sep 17 00:00:00 2001 From: Nico Wiedemann Date: Tue, 11 Aug 2026 16:58:32 +0200 Subject: [PATCH 2/4] fix(media): publish the current item and give timeline rows stable ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things the session was telling controllers wrongly. COMMAND_GET_CURRENT_MEDIA_ITEM was never in the available commands, and PlayerWrapper gates the legacy session's position and duration on it — so createPlaybackStateCompat published PLAYBACK_POSITION_UNKNOWN and TIME_UNSET, leaving the system's own media control with no seek bar and no elapsed time. Every remote controller got a null current media item with it, because PlayerInfo.filterByAvailableCommands strips the same fields. And the playing row took its uid from playerstatus while its neighbours took theirs from the queue. media3 diffs UIDs position by position to tell a track change from a new playlist, so on a box that sends no songid every track change rewrote two of them and read as a wholesale playlist change. The sharper edge is that a queue holding one file twice can leave playerstatus.songid pointing at the other copy — a duplicate uid, which State.Builder.setPlaylist rejects by throwing out of getState(). The row it occupies is the honest source for its identity; only its metadata needs to come from the status. Co-Authored-By: Claude Opus 5 (1M context) --- .../coilforphoniebox/media/PhonieboxPlayer.kt | 50 ++++++++++++--- .../coilforphoniebox/media/TimelineUidTest.kt | 62 +++++++++++++++++++ 2 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 feature-media/src/test/kotlin/app/coilforphoniebox/media/TimelineUidTest.kt 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 9004142..dd3cc40 100644 --- a/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt +++ b/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt @@ -51,6 +51,31 @@ internal fun timelineIndexFor(queue: List, status: PlayerStatus): In return position } +/** + * A timeline item's identity, and it has to be the queue row's rather than the status's. + * + * media3 compares UIDs position by position to decide what changed + * (`SimpleBasePlayer.getTimelineChangeReason`), and refuses a playlist that repeats one + * (`"Duplicate MediaItemData UID in playlist"`, thrown straight out of `getState()`). The playing + * item's *metadata* comes from `playerstatus`, so it used to take its uid from there too — from a + * field the box need not send, in which case every track change rewrote two UIDs and each one read + * as a wholesale playlist change. Worse, a queue holding the same file twice can leave + * `playerstatus.songid` pointing at the other copy, and that collision is the crash. + * + * MPD's queue id is unique within the queue; without one the position separates two copies of the + * same file. + */ +internal fun uidFor(entry: QueueEntry): String = entry.songId ?: "${entry.position}:${entry.url}" + +/** + * Identity for the single-item timeline, where there is no queue row to borrow one from. Nothing + * compares it against a neighbour, so anything stable for as long as the song lasts will do. + */ +internal fun fallbackUid(status: PlayerStatus): String = + status.songId ?: status.file ?: CURRENT_ITEM_UID + +internal const val CURRENT_ITEM_UID = "coil-current" + /** * A media3 player whose playback happens somewhere else entirely. * @@ -141,13 +166,18 @@ class PhonieboxPlayer( 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) + // authoritative metadata, and the only item there is cover art for. Its + // *identity* stays the queue row's, though — see [uidFor]. + if (position == index) { + mediaItemFor(current, uid = uidFor(entry)) + } else { + mediaItemFor(entry) + } }, ) builder.setCurrentMediaItemIndex(index) } else if (status.hasContent) { - builder.setPlaylist(listOf(mediaItemFor(current))) + builder.setPlaylist(listOf(mediaItemFor(current, uid = fallbackUid(status)))) builder.setCurrentMediaItemIndex(0) } @@ -188,9 +218,7 @@ class PhonieboxPlayer( .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}") + return MediaItemData.Builder(uidFor(entry)) .setMediaItem( MediaItem.Builder() .setMediaId(entry.url) @@ -203,7 +231,7 @@ class PhonieboxPlayer( .build() } - private fun mediaItemFor(current: Snapshot): MediaItemData { + private fun mediaItemFor(current: Snapshot, uid: Any): MediaItemData { val status = current.status val durationUs = status.durationSeconds ?.takeIf { it > 0 } @@ -222,7 +250,7 @@ class PhonieboxPlayer( .setIsPlayable(true) .build() - return MediaItemData.Builder(status.songId ?: status.file ?: CURRENT_ITEM_UID) + return MediaItemData.Builder(uid) .setMediaItem( MediaItem.Builder() .setMediaId(status.file ?: CURRENT_ITEM_UID) @@ -301,7 +329,6 @@ class PhonieboxPlayer( } private companion object { - const val CURRENT_ITEM_UID = "coil-current" const val VOLUME_STEP = 5 val AVAILABLE_COMMANDS: Player.Commands = Player.Commands.Builder() @@ -318,6 +345,11 @@ class PhonieboxPlayer( // where it must, so a tap in a queue list always does something honest. Player.COMMAND_SEEK_TO_MEDIA_ITEM, Player.COMMAND_GET_TIMELINE, + // Without this the legacy session publishes PLAYBACK_POSITION_UNKNOWN and no + // duration — `PlayerWrapper.createPlaybackStateCompat` reads it to decide + // whether positions may be shared at all — so the system's own media control + // shows no seek bar, and every remote controller sees a null current item. + Player.COMMAND_GET_CURRENT_MEDIA_ITEM, Player.COMMAND_GET_METADATA, Player.COMMAND_SET_SHUFFLE_MODE, Player.COMMAND_SET_REPEAT_MODE, diff --git a/feature-media/src/test/kotlin/app/coilforphoniebox/media/TimelineUidTest.kt b/feature-media/src/test/kotlin/app/coilforphoniebox/media/TimelineUidTest.kt new file mode 100644 index 0000000..60aa3a9 --- /dev/null +++ b/feature-media/src/test/kotlin/app/coilforphoniebox/media/TimelineUidTest.kt @@ -0,0 +1,62 @@ +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.assertNotEquals +import org.junit.Test + +/** + * media3 identifies timeline items by UID: it diffs them position by position to tell a track + * change from a new playlist, and throws `"Duplicate MediaItemData UID in playlist"` out of + * `getState()` if one repeats. Both of those used to be reachable, because the playing item took + * its identity from `playerstatus` while its neighbours took theirs from the queue. + */ +class TimelineUidTest { + + private fun entry(position: Int, url: String, songId: String? = null) = + QueueEntry(position = position, url = url, title = url, songId = songId) + + @Test + fun `a queue id identifies the row`() { + assertEquals("42", uidFor(entry(0, "A/01.mp3", songId = "42"))) + } + + /** MPD did not send ids, so position has to separate two copies of one file. */ + @Test + fun `without a queue id the position separates identical files`() { + val first = uidFor(entry(0, "A/01.mp3")) + val second = uidFor(entry(1, "A/01.mp3")) + + assertNotEquals(first, second) + } + + /** + * The crash. The same file twice, and a status whose `songid` belongs to the *other* copy: + * building the playing item's uid from the status would repeat the id already used one row + * up. Taking it from the row it actually occupies cannot collide, whatever the status says. + */ + @Test + fun `the playing row keeps its own identity whatever the status reports`() { + val queue = listOf( + entry(0, "A/01.mp3", songId = "7"), + entry(1, "A/01.mp3", songId = "8"), + ) + val status = PlayerStatus(file = "A/01.mp3", playlistPosition = 1, songId = "7") + + val index = timelineIndexFor(queue, status) + assertEquals(1, index) + + val uids = queue.map { uidFor(it) } + assertEquals(uids.size, uids.toSet().size) + // The row's id, not the status's — which is the id of the copy above it. + assertEquals("8", uidFor(queue[index!!])) + } + + @Test + fun `the single item timeline falls back through what the status has`() { + assertEquals("9", fallbackUid(PlayerStatus(file = "A/01.mp3", songId = "9"))) + assertEquals("A/01.mp3", fallbackUid(PlayerStatus(file = "A/01.mp3"))) + assertEquals(CURRENT_ITEM_UID, fallbackUid(PlayerStatus())) + } +} From 07cd55097644faa501d8f88187e9f626d78807a5 Mon Sep 17 00:00:00 2001 From: Nico Wiedemann Date: Tue, 11 Aug 2026 17:00:32 +0200 Subject: [PATCH 3/4] fix(media): honour what media3 asked for, and let its optimism last MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three faults in how incoming commands were read, all of which made a working control look broken. Every handler returned Futures.immediateVoidFuture(). SimpleBasePlayer shows its optimistic placeholder state only while a returned future is pending — updateStateForPendingOperation short-circuits on isDone() and calls getState() straight back, which still describes the box as it was before the command was sent. So no listener fired and the button did not move until the next published status, a quarter of a second later at best and longer while the box's sequential socket is busy. Commands now stay pending until the box has been told and, where the outcome is published, until it says so, bounded by a second so a box that never agrees cannot leave the session pinned to a placeholder. handleSetDeviceMuted ignored the state it was handed and toggled instead. VolumeProviderCompat turns a car stereo's mute key into setDeviceMuted (true), so muting an already-muted box unmuted it. The box's own command is absolute, so PlayerRepository can be too; toggleMute stays for the in-app button, which really does mean "the other one". And handleSeek confused two index spaces. mediaItemIndex is a timeline index, only a queue position while the timeline is the queue — in the single-item fallback the one index there means "the playing song", and passing it to playAt walked the box to queue position zero and restarted the album. It also read C.INDEX_UNSET, media3's way of saying it resolved the seek to nothing, as a command, sending next past the end of a queue and running the box's own end_of_playlist_next_action. Both now go through seekIntentFor, which also honours the position media3 resolved a previous against: part-way into a track it restarts it, near the beginning it goes back one, and outside the queue the same boundary is applied to what the box reports having played. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 44 ++++ CHANGELOG.md | 6 + .../screenshot/FakeRepositories.kt | 2 + .../data/repository/PlayerRepositoryImpl.kt | 5 +- .../domain/repository/PlayerRepository.kt | 9 + .../coilforphoniebox/media/PhonieboxPlayer.kt | 249 +++++++++++++++--- .../coilforphoniebox/media/SeekIntentTest.kt | 170 ++++++++++++ 7 files changed, 441 insertions(+), 44 deletions(-) create mode 100644 feature-media/src/test/kotlin/app/coilforphoniebox/media/SeekIntentTest.kt diff --git a/AGENTS.md b/AGENTS.md index 617237a..5a22163 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -375,6 +375,50 @@ it. It is never published, so it has to be asked for. 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. +## The media session, and the four things media3 will not tell you + +`PhonieboxPlayer` is a `SimpleBasePlayer` whose playback happens on the box, and `feature-media`'s +whole job is translating between the two. Four of media3's contracts are silent when broken — nothing +logs, nothing throws, a control simply does nothing — and all four were broken at some point: + +- **A session the service was never handed shows no notification.** + `MediaNotificationManager.updateNotification` asks `isSessionAdded` before anything else, and media3 + registers a session itself only when a controller binds with `MediaSessionService.SERVICE_INTERFACE` + (or the legacy browser action) or a media button intent arrives — `onBind` returns null for any + other action *without* consulting `onGetSession`. Coil's own binding is neither, so + `PhonieboxMediaService.onCreate` must call **`addSession`** itself. Without it no notification is + ever posted in either session mode, and the same early return calls + `maybeStopForegroundService(removeNotifications = true)`, which cancels media3's notification id. + That is why `MediaSessionBinder` now binds *with* the media3 action, too. +- **Notification ids have one owner each.** 1001 is media3's, and it cancels that id whenever it has + nothing to show; 1002 is the quiet "ready" notification automatic mode waits behind. They used to + share one id, which meant two writers with no ordering between them and a foreground service that + could end up with no notification at all. The quiet one follows `PlayerStatus.hasContent` — the + exact complement of media3's `shouldShowNotification` — so one of the two is up and never both. +- **A command's future must not be complete when it is returned.** `SimpleBasePlayer` shows its + optimistic placeholder state only while a returned future is pending: `updateStateForPendingOperation` + short-circuits on `isDone()` and calls `getState()` straight back, which still describes the box as + it was before the command was sent. `PhonieboxPlayer.send` therefore stays pending until the box has + been told and, where the outcome is published, until it says so — bounded, because `invalidateState` + is ignored while anything is pending. +- **`mediaItemIndex` is a timeline index, not a queue position**, and `C.INDEX_UNSET` means "media3 + resolved this to nothing". Both are read in one place, `seekIntentFor`, whose KDoc carries the + reasoning; the short version is that a fallback timeline's only index means "the playing song", so + passing it to `playAt` restarts the album, and that "unset" only carries meaning when the timeline + really is the queue and shuffle is off. + +Two smaller ones, same character: `COMMAND_GET_CURRENT_MEDIA_ITEM` has to be in the available +commands or the legacy session publishes no position and no duration (`PlayerWrapper` gates them on +it) and the system's media control shows no seek bar; and a timeline item's UID must come from the +queue row it occupies rather than from `playerstatus`, or a track change reads as a whole new playlist +and a queue holding one file twice can throw `"Duplicate MediaItemData UID in playlist"` out of +`getState()`. + +`feature-media` has no Robolectric and should keep it that way: the load-bearing decisions live in +top-level functions — `timelineIndexFor`, `seekIntentFor`, `uidFor` — precisely because `getState()` +needs a `Looper` and they do not. Everything else here is service lifecycle and platform notification +behaviour, which only a device can tell you about. + ## 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 3ed6780..8df7d84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,12 @@ automatically from the `## [x.y.z]` heading matching `versionName` in `app/build then never handed to the part of media3 that posts the notification, so nothing was ever shown in either of the two control modes. In automatic mode the same fault could leave the quiet "ready" notification cancelled behind it +- The controls also work properly now that they are there. The lock screen shows a **seek bar with + elapsed and remaining time**; play and pause **respond to the first tap** instead of appearing to + do nothing until the box got round to answering; ⏮ restarts the track part-way in and goes back a + track near the beginning, the way every other player does; a mute button sent from a car stereo + mutes rather than toggling, so it no longer unmutes an already-muted box; and tapping a track in a + car's playlist goes to that track instead of occasionally restarting the album ## [1.1.0] - 2026-08-11 diff --git a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt index 4dd20fe..d534494 100644 --- a/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt +++ b/app/src/testDebug/kotlin/app/coilforphoniebox/screenshot/FakeRepositories.kt @@ -87,6 +87,8 @@ class FakePlayerRepository( override suspend fun setVolume(level: Int): Result = Result.success(Unit) override suspend fun changeVolume(step: Int): Result = Result.success(Unit) override suspend fun toggleMute(): Result = Result.success(Unit) + + override suspend fun setMuted(muted: Boolean): Result = Result.success(Unit) 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) 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 147ca05..314dd3d 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 @@ -433,7 +433,10 @@ class PlayerRepositoryImpl @Inject constructor( transport.call(Commands.changeVolume(step)).unit() override suspend fun toggleMute(): Result = - transport.call(Commands.mute(!transport.currentVolume().muted)).unit() + setMuted(!transport.currentVolume().muted) + + override suspend fun setMuted(muted: Boolean): Result = + transport.call(Commands.mute(muted)).unit() override suspend fun startSleepTimer(minutes: Int): Result { // `GenericTimerClass.start` logs "Ignoring start command" and returns when its timer 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 2ca290f..6c296b8 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 @@ -115,6 +115,15 @@ interface PlayerRepository { suspend fun changeVolume(step: Int): Result suspend fun toggleMute(): Result + /** + * Mutes or unmutes outright, for a caller that knows which of the two it wants. + * + * The media session is one: a controller asking to mute sends "muted = true", not "the + * other one" — and answering that with [toggleMute] unmutes a box that was already muted. + * The box's own command takes an absolute state, so nothing is lost by saying so. + */ + suspend fun setMuted(muted: Boolean): Result + /** * Sets the timer that stops playback after [minutes], replacing a running one. * 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 dd3cc40..494250b 100644 --- a/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt +++ b/feature-media/src/main/kotlin/app/coilforphoniebox/media/PhonieboxPlayer.kt @@ -2,6 +2,7 @@ package app.coilforphoniebox.media import android.net.Uri import android.os.Looper +import androidx.media3.common.C import androidx.media3.common.DeviceInfo import androidx.media3.common.MediaItem import androidx.media3.common.MediaMetadata @@ -18,11 +19,15 @@ import app.coilforphoniebox.domain.repository.BoxRepository import app.coilforphoniebox.domain.repository.PlayerRepository import com.google.common.util.concurrent.Futures import com.google.common.util.concurrent.ListenableFuture +import com.google.common.util.concurrent.SettableFuture import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull /** * Where [status] sits in [queue], or null when the two cannot be reconciled. @@ -51,6 +56,104 @@ internal fun timelineIndexFor(queue: List, status: PlayerStatus): In return position } +/** What a media3 seek turns into on the box. */ +internal sealed interface SeekIntent { + /** media3 resolved the seek to nothing, so nothing is sent. */ + data object Ignore : SeekIntent + data object Next : SeekIntent + data object Previous : SeekIntent + + /** A jump to a *queue position* — never a timeline index unless the two are the same thing. */ + data class ToQueuePosition(val position: Int) : SeekIntent + data class WithinTrack(val seconds: Double) : SeekIntent +} + +/** + * What the box should be told, for a seek media3 has already resolved against its own timeline. + * + * Three things make this more than a `when` on the command, and all three were wrong before it + * existed: + * + * - **Two index spaces.** `mediaItemIndex` is a *timeline* index. It is also a queue position, but + * only while the timeline is the queue — when [timelineIndexFor] declined and the timeline is the + * playing song alone, the only index there is means "this song", and handing that to `playAt` + * walks the box to queue position zero and restarts the album. Hence [queuePosition], which is + * that function's answer: non-null means the indices are queue positions. + * - **`C.INDEX_UNSET` means "do nothing".** `BasePlayer.ignoreSeek` sends it when there is nothing + * to seek to, and sending `next` anyway runs the box's own `end_of_playlist_next_action` at the + * end of a queue (§"The queue"). It only carries that meaning in the real queue, though: a + * one-item timeline has no next or previous item, so media3 says "unset" about every one of them + * and the signal is noise. With [shuffle] on it is noise too — media3's timeline has no shuffle + * order, so it reports the last *position* as the end even though MPD would pick another track. + * - **Previous is position-dependent.** Past `maxSeekToPreviousPosition` (media3's default three + * seconds) "previous" means restarting the current track, which media3 passes down as position + * zero on the current index. In the fallback timeline it cannot make that call for us, so + * [elapsedSeconds] makes it here against the same boundary. + * + * A top-level function for the same reason [timelineIndexFor] is one: `getState()` needs a `Looper` + * and this does not. + */ +internal fun seekIntentFor( + seekCommand: Int, + mediaItemIndex: Int, + positionMs: Long, + queuePosition: Int?, + shuffle: Boolean, + elapsedSeconds: Double?, +): SeekIntent { + val queued = queuePosition != null + val resolvedToNothing = mediaItemIndex == C.INDEX_UNSET && queued && !shuffle + + return when (seekCommand) { + Player.COMMAND_SEEK_TO_NEXT, Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM -> + if (resolvedToNothing) SeekIntent.Ignore else SeekIntent.Next + + Player.COMMAND_SEEK_TO_PREVIOUS -> when { + resolvedToNothing -> SeekIntent.Ignore + queued -> + if (mediaItemIndex == queuePosition && positionMs == 0L) { + SeekIntent.WithinTrack(0.0) + } else { + SeekIntent.Previous + } + + (elapsedSeconds ?: 0.0) > MAX_SEEK_TO_PREVIOUS_SECONDS -> SeekIntent.WithinTrack(0.0) + else -> SeekIntent.Previous + } + + Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM -> + if (resolvedToNothing) SeekIntent.Ignore else SeekIntent.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 + // cannot 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 -> + if (queued && mediaItemIndex >= 0) { + SeekIntent.ToQueuePosition(mediaItemIndex) + } else { + // The one item of a fallback timeline is the playing song, so this is a seek + // inside it — commonly back to the start. + withinTrack(positionMs) + } + + // COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, and anything later media3 routes the same way. + else -> withinTrack(positionMs) + } +} + +private fun withinTrack(positionMs: Long): SeekIntent = + if (positionMs == C.TIME_UNSET) { + // "The default position", which for a box already playing that song is where it is. + SeekIntent.Ignore + } else { + SeekIntent.WithinTrack(positionMs.coerceAtLeast(0L) / 1000.0) + } + +/** media3's own boundary between "previous track" and "start this one again". */ +private const val MAX_SEEK_TO_PREVIOUS_SECONDS = + C.DEFAULT_MAX_SEEK_TO_PREVIOUS_POSITION_MS / 1000.0 + /** * A timeline item's identity, and it has to be the queue row's rather than the status's. * @@ -80,9 +183,10 @@ internal const val CURRENT_ITEM_UID = "coil-current" * A media3 player whose playback happens somewhere else entirely. * * `SimpleBasePlayer` exists precisely for this case (the Cast scenario): [getState] is - * assembled from the most recent `playerstatus`, and every command fires an RPC and - * returns immediately. The UI is therefore optimistic and gets corrected by the next - * published status a quarter of a second later (§8.1). + * assembled from the most recent `playerstatus`, and every command fires an RPC. No button + * waits on the network — media3 shows the outcome optimistically the moment it is asked — but + * the command does stay *pending* until the box has caught up, which is what makes that + * optimism last long enough to see (§8.1, and [send]). * * The useful side effect of `COMMAND_SET_DEVICE_VOLUME` is that the phone's hardware * volume buttons change the Phoniebox while the session is active. @@ -208,7 +312,7 @@ class PhonieboxPlayer( val durationUs = entry.durationSeconds ?.takeIf { it > 0 } ?.let { (it * 1_000_000).toLong() } - ?: androidx.media3.common.C.TIME_UNSET + ?: C.TIME_UNSET val metadata = MediaMetadata.Builder() .setTitle(entry.title) @@ -226,7 +330,7 @@ class PhonieboxPlayer( .build(), ) .setDurationUs(durationUs) - .setIsSeekable(durationUs != androidx.media3.common.C.TIME_UNSET) + .setIsSeekable(durationUs != C.TIME_UNSET) .setIsDynamic(false) .build() } @@ -236,7 +340,7 @@ class PhonieboxPlayer( val durationUs = status.durationSeconds ?.takeIf { it > 0 } ?.let { (it * 1_000_000).toLong() } - ?: androidx.media3.common.C.TIME_UNSET + ?: C.TIME_UNSET val metadata = MediaMetadata.Builder() .setTitle(status.title) @@ -258,79 +362,138 @@ class PhonieboxPlayer( .build(), ) .setDurationUs(durationUs) - .setIsSeekable(durationUs != androidx.media3.common.C.TIME_UNSET) + .setIsSeekable(durationUs != C.TIME_UNSET) .setIsDynamic(false) .build() } - override fun handleSetPlayWhenReady(playWhenReady: Boolean): ListenableFuture<*> = fireAndForget { + override fun handleSetPlayWhenReady(playWhenReady: Boolean): ListenableFuture<*> = send { if (playWhenReady) player.play() else player.pause() + awaitStatus { (it.state == PlaybackState.PLAY) == playWhenReady } } override fun handlePrepare(): ListenableFuture<*> = Futures.immediateVoidFuture() - override fun handleStop(): ListenableFuture<*> = fireAndForget { + override fun handleStop(): ListenableFuture<*> = send { // Coil never sends anything beyond playback control; pausing is as far as it goes. player.pause() + awaitStatus { it.state != PlaybackState.PLAY } } override fun handleSeek( mediaItemIndex: Int, positionMs: Long, seekCommand: Int, - ): ListenableFuture<*> = fireAndForget { - when (seekCommand) { - Player.COMMAND_SEEK_TO_NEXT, Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM -> player.next() - 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) + ): ListenableFuture<*> { + val current = snapshot + val intent = seekIntentFor( + seekCommand = seekCommand, + mediaItemIndex = mediaItemIndex, + positionMs = positionMs, + // The same call [getState] makes, so the handler and the timeline it published + // cannot disagree about which index space media3 is speaking in. + queuePosition = timelineIndexFor(current.queue, current.status), + shuffle = current.status.shuffle, + elapsedSeconds = current.status.elapsedSeconds, + ) + + return when (intent) { + SeekIntent.Ignore -> Futures.immediateVoidFuture() + SeekIntent.Next -> send { player.next() } + SeekIntent.Previous -> send { player.previous() } + is SeekIntent.ToQueuePosition -> send { player.playAt(intent.position) } + is SeekIntent.WithinTrack -> send { player.seekTo(intent.seconds) } } } - override fun handleSetDeviceVolume(deviceVolume: Int, flags: Int): ListenableFuture<*> = - fireAndForget { player.setVolume(deviceVolume) } + override fun handleSetDeviceVolume(deviceVolume: Int, flags: Int): ListenableFuture<*> = send { + player.setVolume(deviceVolume) + awaitVolume { it.level == deviceVolume } + } + // No confirmation for the two relative ones: media3's placeholder assumes a step of one, so + // holding it until the box answers would only show a wrong number for longer. override fun handleIncreaseDeviceVolume(flags: Int): ListenableFuture<*> = - fireAndForget { player.changeVolume(VOLUME_STEP) } + send { player.changeVolume(VOLUME_STEP) } override fun handleDecreaseDeviceVolume(flags: Int): ListenableFuture<*> = - fireAndForget { player.changeVolume(-VOLUME_STEP) } + send { player.changeVolume(-VOLUME_STEP) } - override fun handleSetDeviceMuted(muted: Boolean, flags: Int): ListenableFuture<*> = - fireAndForget { player.toggleMute() } + /** + * Mute is absolute, not a toggle: `VolumeProviderCompat` turns a car stereo's mute key into + * `setDeviceMuted(true)`, and answering that with a toggle unmutes a box already muted. + */ + override fun handleSetDeviceMuted(muted: Boolean, flags: Int): ListenableFuture<*> = send { + player.setMuted(muted) + awaitVolume { it.muted == muted } + } override fun handleSetShuffleModeEnabled(shuffleModeEnabled: Boolean): ListenableFuture<*> = - fireAndForget { player.setShuffle(shuffleModeEnabled) } - - override fun handleSetRepeatMode(repeatMode: Int): ListenableFuture<*> = fireAndForget { - player.setRepeat( - when (repeatMode) { - Player.REPEAT_MODE_ONE -> RepeatMode.ONE - Player.REPEAT_MODE_ALL -> RepeatMode.ALL - else -> RepeatMode.OFF - }, - ) + send { + player.setShuffle(shuffleModeEnabled) + awaitStatus { it.shuffle == shuffleModeEnabled } + } + + override fun handleSetRepeatMode(repeatMode: Int): ListenableFuture<*> = send { + val mode = when (repeatMode) { + Player.REPEAT_MODE_ONE -> RepeatMode.ONE + Player.REPEAT_MODE_ALL -> RepeatMode.ALL + else -> RepeatMode.OFF + } + player.setRepeat(mode) + awaitStatus { it.repeat == mode } } /** - * Every command returns immediately rather than waiting for the box: a lock screen - * button that blocks for a network round trip feels broken even when it works. + * Sends a command, and stays pending until the box has caught up. + * + * The future is the whole mechanism, and an immediate one throws it away. + * `SimpleBasePlayer` shows its optimistic placeholder — the pressed pause button, the new + * volume — only while a returned future is still running: hand it one that is already + * complete and `updateStateForPendingOperation` takes its `isDone()` short circuit and asks + * `getState()` straight back, which still describes the box as it was before the command was + * even sent. The button then does not move until the next published status, a quarter of a + * second later at best, which is exactly how a working control comes to look broken. + * + * So this finishes when the box has been told and, where the outcome is something the box + * reports, when it says so — [awaitStatus] and [awaitVolume] bound that wait, because + * `invalidateState` is ignored while anything is pending and a future that never completes + * would freeze the session. */ - private fun fireAndForget(block: suspend () -> Unit): ListenableFuture<*> { - scope.launch { block() } - return Futures.immediateVoidFuture() + private fun send(block: suspend () -> Unit): ListenableFuture<*> { + // A command arriving as the service goes down: the coroutine would never start, and so + // would never complete the future either. + if (!scope.isActive) return Futures.immediateVoidFuture() + + val pending = SettableFuture.create() + scope.launch { + try { + block() + } finally { + pending.set(Unit) + } + } + return pending + } + + private suspend fun awaitStatus(reached: (PlayerStatus) -> Boolean) { + withTimeoutOrNull(CONFIRM_MILLIS) { player.status.first(reached) } + } + + private suspend fun awaitVolume(reached: (VolumeStatus) -> Boolean) { + withTimeoutOrNull(CONFIRM_MILLIS) { player.volume.first(reached) } } private companion object { const val VOLUME_STEP = 5 + /** + * How long a command may hold the optimistic state while waiting for the box to publish + * the outcome. Long enough for the box's sequential socket to get round to it (§6), + * short enough that a box which never agrees is not left holding the session. + */ + const val CONFIRM_MILLIS = 1_000L + val AVAILABLE_COMMANDS: Player.Commands = Player.Commands.Builder() .addAll( Player.COMMAND_PLAY_PAUSE, diff --git a/feature-media/src/test/kotlin/app/coilforphoniebox/media/SeekIntentTest.kt b/feature-media/src/test/kotlin/app/coilforphoniebox/media/SeekIntentTest.kt new file mode 100644 index 0000000..b8598c4 --- /dev/null +++ b/feature-media/src/test/kotlin/app/coilforphoniebox/media/SeekIntentTest.kt @@ -0,0 +1,170 @@ +package app.coilforphoniebox.media + +import androidx.media3.common.C +import androidx.media3.common.Player +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * media3 resolves a seek against the timeline it was given and hands down the result; these are + * the readings of that result that were wrong, and each one reached the box as a command it + * should not have been sent. + */ +class SeekIntentTest { + + private fun intent( + seekCommand: Int, + mediaItemIndex: Int = 0, + positionMs: Long = C.TIME_UNSET, + queuePosition: Int? = 3, + shuffle: Boolean = false, + elapsedSeconds: Double? = null, + ) = seekIntentFor( + seekCommand = seekCommand, + mediaItemIndex = mediaItemIndex, + positionMs = positionMs, + queuePosition = queuePosition, + shuffle = shuffle, + elapsedSeconds = elapsedSeconds, + ) + + /** The end of a trusted queue: sending `next` here runs the box's end-of-playlist action. */ + @Test + fun `an unresolved next in the real queue is ignored`() { + assertEquals( + SeekIntent.Ignore, + intent(Player.COMMAND_SEEK_TO_NEXT, mediaItemIndex = C.INDEX_UNSET), + ) + assertEquals( + SeekIntent.Ignore, + intent(Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM, mediaItemIndex = C.INDEX_UNSET), + ) + } + + /** + * The fallback timeline is one item, so media3 says "nothing to seek to" about every next and + * previous. Reading that as "do nothing" would disable the notification's buttons outright. + */ + @Test + fun `an unresolved next outside the queue still means next`() { + assertEquals( + SeekIntent.Next, + intent( + Player.COMMAND_SEEK_TO_NEXT, + mediaItemIndex = C.INDEX_UNSET, + queuePosition = null, + ), + ) + } + + /** + * With shuffle on, media3 calls the last position the end of the timeline — it has no shuffle + * order to consult — while MPD would happily pick another track. + */ + @Test + fun `an unresolved next with shuffle on still means next`() { + assertEquals( + SeekIntent.Next, + intent(Player.COMMAND_SEEK_TO_NEXT, mediaItemIndex = C.INDEX_UNSET, shuffle = true), + ) + } + + @Test + fun `a resolved next means next`() { + assertEquals(SeekIntent.Next, intent(Player.COMMAND_SEEK_TO_NEXT, mediaItemIndex = 4)) + } + + /** Position zero on the current item is how media3 spells "start this track again". */ + @Test + fun `previous on the current item at position zero restarts the track`() { + assertEquals( + SeekIntent.WithinTrack(0.0), + intent(Player.COMMAND_SEEK_TO_PREVIOUS, mediaItemIndex = 3, positionMs = 0L), + ) + } + + @Test + fun `previous naming another item goes back a track`() { + assertEquals( + SeekIntent.Previous, + intent(Player.COMMAND_SEEK_TO_PREVIOUS, mediaItemIndex = 2, positionMs = 0L), + ) + } + + /** + * Outside the queue media3 cannot tell the two apart — a one-item timeline has no previous + * item, so every previous arrives as position zero on index zero — so the same boundary is + * applied here against what the box says it has played. + */ + @Test + fun `previous outside the queue follows the elapsed position`() { + assertEquals( + SeekIntent.WithinTrack(0.0), + intent( + Player.COMMAND_SEEK_TO_PREVIOUS, + positionMs = 0L, + queuePosition = null, + elapsedSeconds = 40.0, + ), + ) + assertEquals( + SeekIntent.Previous, + intent( + Player.COMMAND_SEEK_TO_PREVIOUS, + positionMs = 0L, + queuePosition = null, + elapsedSeconds = 1.5, + ), + ) + } + + @Test + fun `a queue row tapped in a controller is a jump to that position`() { + assertEquals( + SeekIntent.ToQueuePosition(6), + intent(Player.COMMAND_SEEK_TO_MEDIA_ITEM, mediaItemIndex = 6, positionMs = 0L), + ) + } + + /** + * The crash-adjacent one: outside the queue, index 0 is the *playing song*, not queue + * position 0 — sending it to `playAt` would restart the album from its first track. + */ + @Test + fun `seeking to the only item of a fallback timeline stays inside the track`() { + assertEquals( + SeekIntent.WithinTrack(12.5), + intent( + Player.COMMAND_SEEK_TO_MEDIA_ITEM, + mediaItemIndex = 0, + positionMs = 12_500L, + queuePosition = null, + ), + ) + } + + @Test + fun `an ordinary scrub seeks inside the track`() { + assertEquals( + SeekIntent.WithinTrack(90.0), + intent(Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, positionMs = 90_000L), + ) + } + + /** `C.TIME_UNSET` divided down is a seek to some 292 million years before the track began. */ + @Test + fun `an unset position is ignored rather than sent as a number`() { + assertEquals( + SeekIntent.Ignore, + intent(Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, positionMs = C.TIME_UNSET), + ) + } + + @Test + fun `a negative position is clamped rather than sent`() { + assertEquals( + SeekIntent.WithinTrack(0.0), + intent(Player.COMMAND_SEEK_IN_CURRENT_MEDIA_ITEM, positionMs = -5_000L), + ) + } +} From 9f0b8b72747840e01c916fe4244f037417561356 Mon Sep 17 00:00:00 2001 From: Nico Wiedemann Date: Wed, 12 Aug 2026 01:30:59 +0200 Subject: [PATCH 4/4] chore(release): bump version to 1.1.1 --- CHANGELOG.md | 2 ++ app/build.gradle.kts | 4 ++-- fastlane/metadata/android/de-DE/changelogs/7.txt | 3 +++ fastlane/metadata/android/en-US/changelogs/7.txt | 3 +++ fastlane/metadata/android/es-ES/changelogs/7.txt | 3 +++ fastlane/metadata/android/fr-FR/changelogs/7.txt | 3 +++ fastlane/metadata/android/nl-NL/changelogs/7.txt | 3 +++ 7 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 fastlane/metadata/android/de-DE/changelogs/7.txt create mode 100644 fastlane/metadata/android/en-US/changelogs/7.txt create mode 100644 fastlane/metadata/android/es-ES/changelogs/7.txt create mode 100644 fastlane/metadata/android/fr-FR/changelogs/7.txt create mode 100644 fastlane/metadata/android/nl-NL/changelogs/7.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 8df7d84..4b39302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ automatically from the `## [x.y.z]` heading matching `versionName` in `app/build ## [Unreleased] +## [1.1.1] - 2026-08-12 + ### Fixed - **The playback controls now actually appear.** Coil is supposed to put the playing track in your notification shade and on your lock screen, and it never did — the session was built correctly and diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 63f98ca..7e55ddd 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -20,8 +20,8 @@ android { applicationId = "app.coilforphoniebox" minSdk = 26 targetSdk = 36 - versionCode = 6 - versionName = "1.1.0" + versionCode = 7 + versionName = "1.1.1" } androidResources { diff --git a/fastlane/metadata/android/de-DE/changelogs/7.txt b/fastlane/metadata/android/de-DE/changelogs/7.txt new file mode 100644 index 0000000..b25626d --- /dev/null +++ b/fastlane/metadata/android/de-DE/changelogs/7.txt @@ -0,0 +1,3 @@ +Die Wiedergabesteuerung von Coil erscheint jetzt in der Benachrichtigungsleiste und auf dem Sperrbildschirm. Bisher war sie dort überhaupt nicht zu sehen. + +Und sie funktioniert richtig: mit Fortschrittsleiste und Zeitanzeige, mit Play und Pause, die schon beim ersten Tippen reagieren, und einer Zurück-Taste, die einen laufenden Titel neu startet. diff --git a/fastlane/metadata/android/en-US/changelogs/7.txt b/fastlane/metadata/android/en-US/changelogs/7.txt new file mode 100644 index 0000000..e2693a7 --- /dev/null +++ b/fastlane/metadata/android/en-US/changelogs/7.txt @@ -0,0 +1,3 @@ +Coil's playback controls now appear in your notification shade and on the lock screen. They never showed up at all before this. + +They work properly too: a progress bar with the time, play and pause that react to the first tap, and a previous button that starts the current track again part-way in. diff --git a/fastlane/metadata/android/es-ES/changelogs/7.txt b/fastlane/metadata/android/es-ES/changelogs/7.txt new file mode 100644 index 0000000..ce18b06 --- /dev/null +++ b/fastlane/metadata/android/es-ES/changelogs/7.txt @@ -0,0 +1,3 @@ +Los controles de reproducción de Coil ya aparecen en el panel de notificaciones y en la pantalla de bloqueo: hasta ahora no se mostraban en absoluto. + +Y funcionan de verdad: barra de progreso con el tiempo, reproducir y pausar que responden al primer toque, y un botón de anterior que reinicia la pista que ya ha empezado. diff --git a/fastlane/metadata/android/fr-FR/changelogs/7.txt b/fastlane/metadata/android/fr-FR/changelogs/7.txt new file mode 100644 index 0000000..1ef46ab --- /dev/null +++ b/fastlane/metadata/android/fr-FR/changelogs/7.txt @@ -0,0 +1,3 @@ +Les commandes de lecture de Coil apparaissent enfin dans le volet des notifications et sur l'écran de verrouillage : elles ne s'y affichaient pas du tout jusqu'ici. + +Et elles fonctionnent vraiment : barre de progression avec la durée, lecture et pause qui répondent au premier appui, et un bouton précédent qui reprend le titre en cours depuis le début. diff --git a/fastlane/metadata/android/nl-NL/changelogs/7.txt b/fastlane/metadata/android/nl-NL/changelogs/7.txt new file mode 100644 index 0000000..dd3e451 --- /dev/null +++ b/fastlane/metadata/android/nl-NL/changelogs/7.txt @@ -0,0 +1,3 @@ +De afspeelknoppen van Coil verschijnen nu in het meldingenpaneel en op het vergrendelscherm. Tot nu toe waren ze daar helemaal niet te zien. + +En ze werken ook echt: een voortgangsbalk met de tijd, afspelen en pauzeren die meteen reageren, en een vorige-knop die een al begonnen nummer opnieuw start.