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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/google-play.yml
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,13 @@ jobs:
tracks: ${{ inputs.track || 'internal' }}
whatsNewDirectory: whatsnew/
mappingFile: release-assets/mapping.txt
# Commit the edit without submitting it for review, and press "Send for review" in the
# Play Console instead. Without this, `Edits.commit` refuses an app whose changes it may
# not submit on its own — "Changes cannot be sent for review automatically. Please set
# the query parameter changesNotSentForReview to true" — which is every upload to an app
# that has not had a release reviewed yet, and any upload made while a console-side edit
# is still pending. The internal track needs no review at all, so nothing is held up
# there; a production release is, until someone submits it. Drop this once the listing
# has been through review and nothing is pending, if the manual step becomes the bigger
# risk of the two.
changesNotSentForReview: true
26 changes: 24 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ condensed map of it, not a replacement. See "Implementation status" for what is
handles only the AAB, mapping and release notes. The track reaches that action as `tracks` — the
singular `track` is deprecated there, and setting both is a hard error. This workflow's own input
stays singular because it passes exactly one, and it defaults twice over: the action uploads to
**production** when neither input is given, which is not a default to reach by accident.
**production** when neither input is given, which is not a default to reach by accident. It also
passes `changesNotSentForReview: true`, because `Edits.commit` refuses to commit an edit it may
not submit for review itself — "Changes cannot be sent for review automatically. Please set the
query parameter changesNotSentForReview to true", which is every upload to an app that has not
had a release reviewed yet, and any upload racing a pending console-side edit. The cost is that
a production upload waits for someone to press **Send for review** in the Play Console; the
internal track needs no review, so nothing waits there.
- `pages.yml` — deploys `docs/pages/` via Jekyll to GitHub Pages on push to `main` (path-filtered
to `docs/pages/**`). GitHub Pages must be enabled in repo settings with source "GitHub Actions",
the custom domain must be set there, and `coilforphoniebox.app` DNS must point at GitHub Pages.
Expand Down Expand Up @@ -171,6 +177,19 @@ server in this repo.
- Parse `playerstatus` leniently (`ignoreUnknownKeys = true`, tolerate stringified numbers and
missing fields like `duration`/`album` on web radio streams). Field names are not a stable
contract upstream.
- **`playerstatus` tags belong to some earlier song, not necessarily this one.** The box builds
the payload as `self.mpd_status.update(status())` then `.update(currentsong())` into a dict
created once at startup and never cleared — `playermpd/__init__.py` on `future3/main`, the same
code in `player/backends/mpd.py` on `future3/develop`. MPD *omits* the tags a file has no value
for rather than sending them empty, and `dict.update` only overwrites, so an untagged track
inherits the `title`, `artist` and `album` of the last tagged one the box played and keeps them
for the whole album. `file`, `pos`, `songid`, `state` and the counters are always current; every
tag is suspect. **`playlistinfo` is not merged like that** — each row is MPD's own answer for
that file — so the queue row matching `playerstatus.file` is the only account of what the
playing song is really tagged with, which is what `PlayerStatus.reconciledWith` reads it for —
applied once, in `PlayerRepositoryImpl.status`, so the three surfaces that show a title cannot
disagree. Do not "fix" a screen by reading `status.title` or `status.album`
directly: the repository hands out the reconciled status and the raw one is a bug.
- **Never send `as_thread`.** `jukebox/plugs.py` starts a daemon thread and returns the `Thread`
object instead of the function's result, so any call carrying it answers with something
unusable — it is fire-and-forget only. The implementation plan §6.2 recommends it for slow
Expand Down Expand Up @@ -409,7 +428,10 @@ logs, nothing throws, a control simply does nothing — and all five were broken
genuinely never changes: same null title, same albumartist, same folder cover, so
`SimpleBasePlayer` has nothing to report and the same wrong line stays up track after track. The
playing item's title therefore goes through `PlayerStatus.displayTitle`, which falls back to the
file name the way `QueueParser` and `LibraryTrack.displayTitle` already do.
file name the way `QueueEntry` and `LibraryTrack` do with theirs. Half the report was literally
stale metadata, though, and from the box rather than from media3: an untagged track that follows
a tagged one is *published* under the earlier one's title and album — see the `playerstatus`
tags bullet under "Protocol essentials", and `PlayerStatus.reconciledWith`, which stops it.
- **`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
Expand Down
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ automatically from the `## [x.y.z]` heading matching `versionName` in `app/build

## [Unreleased]

## [1.2.2] - 2026-08-27

### Fixed
- **The album line names the album that is playing.** A track with no tags of its own was shown
under the title, artist and album of the last tagged track the box had played — a rip from one
album sitting under another album's name, with an artist beside it that happened to be right.
The box publishes it that way: it merges each song's tags into one record it never clears, so a
song with nothing to say inherits whatever the one before it said. Coil now checks the playing
song's tags against the box's queue, which answers per song, and shows the file name for a track
the queue says has no title — on the player screen, in the mini player, in the queue and on the
lock screen alike

## [1.2.1] - 2026-08-27

### Fixed
Expand Down
4 changes: 2 additions & 2 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ android {
applicationId = "app.coilforphoniebox"
minSdk = 26
targetSdk = 36
versionCode = 9
versionName = "1.2.1"
versionCode = 10
versionName = "1.2.2"
}

androidResources {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ private fun QueueRow(

Column(Modifier.weight(1f)) {
Text(
text = entry.title,
text = entry.displayTitle,
style = MaterialTheme.typography.bodyLarge,
color = colour,
fontWeight = if (playing) FontWeight.Medium else null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,30 @@ class PlayerRepositoryImpl @Inject constructor(
@TransportScope private val scope: CoroutineScope,
) : PlayerRepository {

override val status: Flow<PlayerStatus> = transport.status
/**
* Declared before [status], which reads it: a property initialiser that runs before the
* property it names has been assigned sees null, and Kotlin does not warn about it.
*/
private val _queue = MutableStateFlow<List<QueueEntry>>(emptyList())

override val queue: StateFlow<List<QueueEntry>> = _queue.asStateFlow()

/**
* The published status with the current song's tags checked against the queue — see
* [PlayerStatus.reconciledWith] for what the box gets wrong and why the queue knows better.
*
* Done here rather than in each of the three places that show a title, so that the media
* session, the player screen and the mini player cannot disagree about what is playing.
* Only [transport] is read raw, by the queue resolver below: it looks at the length and the
* file, neither of which this touches, and feeding it this flow instead would be a loop.
*/
override val status: Flow<PlayerStatus> =
combine(transport.status, _queue) { status, queue -> status.reconciledWith(queue) }
// The queue arrives on its own schedule and usually changes nothing here, while the
// status arrives four times a second — without this every queue fetch would wake
// every screen for an identical value.
.distinctUntilChanged()

override val volume: Flow<VolumeStatus> = transport.volume
override val connectionState: Flow<ConnectionState> = transport.connection
override val boxVersion: Flow<String?> = transport.boxVersion
Expand Down Expand Up @@ -114,10 +137,6 @@ class PlayerRepositoryImpl @Inject constructor(

override fun currentCoverFile(): String? = _coverFile.value

private val _queue = MutableStateFlow<List<QueueEntry>>(emptyList())

override val queue: StateFlow<List<QueueEntry>> = _queue.asStateFlow()

private val _queueLoading = MutableStateFlow(false)

override val queueLoading: StateFlow<Boolean> = _queueLoading.asStateFlow()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,42 @@ data class PlayerStatus(
get() = title?.takeIf { it.isNotBlank() }
?: file?.substringAfterLast('/')?.takeIf { it.isNotBlank() }

/**
* The same status with [title], [artist] and [album] checked against the box's queue —
* because `playerstatus` carries the *previous* song's tags for a song that has none.
*
* The box builds that payload as `mpd_status.update(status())` then
* `.update(currentsong())` into one dictionary it created at startup and never clears
* (`playermpd/__init__.py`, and the same code in the provider-neutral player). MPD omits
* the tags a file does not have rather than sending them empty, so a `dict.update` only
* ever *overwrites* tags — an untagged track inherits the title, artist and album of the
* last tagged one the box played, and keeps them for the whole album. Which is why a rip
* with no tags shows up under some other album's name, its own artist beside it: the
* artist happened to be overwritten and the album did not.
*
* `playlistinfo` is not merged like that — each entry is MPD's own answer for that file —
* so the queue row matching [file] is the only account of what this song is really tagged
* with, and a tag missing there cannot belong to this song.
*
* Two things it deliberately does not do:
* - **Streams keep the published tags.** ICY metadata reaches `currentsong` live while the
* cached queue row still describes the station, so there the payload is the fresher of
* the two.
* - **An unmatched file is left alone.** The queue is fetched once per queue change, so
* between a card tap and the answer the new song is in no cached row; the previous tags
* stay up for that second rather than blinking out on every tagged album too.
*/
fun reconciledWith(queue: List<QueueEntry>): PlayerStatus {
val file = file ?: return this
if (file.contains("://")) return this
val entry = queue.firstOrNull { it.url == file } ?: return this

val reconciled = copy(title = entry.title, artist = entry.artist, album = entry.album)
// The same instance when the box was right, so the four-a-second status path stays
// free of copies nothing downstream can tell apart.
return if (reconciled == this) this else reconciled
}

companion object {
val Idle = PlayerStatus()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@ data class QueueEntry(
/** 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.
* The title tag, and null when the file has none — which is most of a Phoniebox library.
*
* Nullable rather than pre-filled with the file name because absence is information:
* [PlayerStatus.reconciledWith] reads these three tags as *the* account of what this file
* is tagged with, and a fallback baked in here would make an untagged song indistinguishable
* from one tagged with its own file name. [displayTitle] is what goes on screen.
*/
val title: String,
val title: String? = null,
val artist: String? = null,
val album: String? = null,
val durationSeconds: Double? = null,
Expand All @@ -30,4 +34,13 @@ data class QueueEntry(
* media session needs for a timeline item's UID. Null when the box did not send one.
*/
val songId: String? = null,
)
) {
/**
* What to show. Falls back to the file name the way [LibraryTrack.displayTitle] and
* [PlayerStatus.displayTitle] do, and for the same reason: a row with no text at all would
* be unusable, and a media3 item with no title at all is what the platform answers with
* "<app> is running".
*/
val displayTitle: String
get() = title?.takeIf { it.isNotBlank() } ?: url.substringAfterLast('/')
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app.coilforphoniebox.domain.model

import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertSame
import org.junit.Test

class PlayerStatusTest {
Expand Down Expand Up @@ -50,4 +51,97 @@ class PlayerStatusTest {
fun `an idle box has no title`() {
assertNull(PlayerStatus.Idle.displayTitle)
}

private fun entry(
position: Int,
url: String,
title: String? = null,
artist: String? = null,
album: String? = null,
) = QueueEntry(position = position, url = url, title = title, artist = artist, album = album)

/**
* The bug this exists for: the box merges `currentsong` into a dictionary it never clears,
* so an untagged track keeps the tags of the last tagged one it played — another album's
* name, with an artist that happens to be right.
*/
@Test
fun `tags the file does not have are dropped`() {
val status = PlayerStatus(
title = "Kapitel 1",
artist = "Petronella",
album = "Petronella Apfelmus 1",
file = "Bibi/02.mp3",
)
val queue = listOf(
entry(0, "Bibi/01.mp3"),
entry(1, "Bibi/02.mp3"),
)

val reconciled = status.reconciledWith(queue)

assertNull(reconciled.title)
assertNull(reconciled.artist)
assertNull(reconciled.album)
// The file name, the way an untagged track is named everywhere else.
assertEquals("02.mp3", reconciled.displayTitle)
}

/** A tagged song keeps everything, and keeps it without a copy nobody can tell apart. */
@Test
fun `a tagged file is left exactly as it came`() {
val status = PlayerStatus(
title = "Kapitel 2",
artist = "Bibi",
album = "Folge 12",
file = "Bibi/02.mp3",
)
val queue = listOf(entry(1, "Bibi/02.mp3", "Kapitel 2", "Bibi", "Folge 12"))

assertSame(status, status.reconciledWith(queue))
}

/** The queue is the account of this file's tags, including one the box left out. */
@Test
fun `a tag only the queue has is taken from it`() {
val status = PlayerStatus(title = "Kapitel 2", file = "Bibi/02.mp3")
val queue = listOf(entry(1, "Bibi/02.mp3", "Kapitel 2", album = "Folge 12"))

assertEquals("Folge 12", status.reconciledWith(queue).album)
}

/**
* The queue is fetched once per queue change, so for the second between a card tap and the
* answer the playing file is in no cached row. Leaving it alone keeps a tagged album's
* title on screen instead of blinking it out on every tap.
*/
@Test
fun `a file the cached queue does not have is left alone`() {
val status = PlayerStatus(title = "Kapitel 1", album = "Folge 11", file = "Bibi/11/01.mp3")

assertSame(status, status.reconciledWith(listOf(entry(0, "Bibi/12/01.mp3"))))
assertSame(status, status.reconciledWith(emptyList()))
}

/**
* Web radio is the one case where the published payload is the fresher of the two: ICY
* metadata reaches `currentsong` live, while the cached queue row still names the station.
*/
@Test
fun `a stream keeps the tags the box published`() {
val status = PlayerStatus(
title = "Some Song",
artist = "Some Artist",
file = "http://stream.example/radio.mp3",
)
val queue = listOf(entry(0, "http://stream.example/radio.mp3", "Radio Example"))

assertSame(status, status.reconciledWith(queue))
}

/** Nothing is playing, so there is nothing to check against. */
@Test
fun `an idle box is left alone`() {
assertSame(PlayerStatus.Idle, PlayerStatus.Idle.reconciledWith(emptyList()))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ object QueueParser {
// 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('/'),
// Left null when the file has no title tag, which is most of a Phoniebox
// library: `QueueEntry.displayTitle` puts the file name on screen, and
// `PlayerStatus.reconciledWith` needs to be able to see that the tag is
// absent — that is what tells it the box is publishing a stale one.
title = entry.anyString("title", "Title"),
// `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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,18 @@ class QueueParserTest {

/**
* 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.
* edge one — a row with no text at all would be unusable. The tag itself stays null: that
* absence is what `PlayerStatus.reconciledWith` reads to catch the box publishing the
* previous song's title for this one.
*/
@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.title)
assertEquals("03 - Track.mp3", entry.displayTitle)
assertNull(entry.artist)
assertNull(entry.album)
assertNull(entry.durationSeconds)
Expand Down
Loading
Loading