fw/music: album art cover support - #1803
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecada632be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Ignore chunks that aren't for the track we currently have (art that raced with a track change). | ||
| // Don't tear down an in-progress transfer for the current track: just drop the stale chunk. | ||
| if (hdr->token != music_get_now_playing_generation()) { |
There was a problem hiding this comment.
If a transfer has already allocated s_art_rx.pixels/palette and the track changes before the Last chunk arrives, old-token chunks hit this return while the active receive state remains allocated. Because generation changes do not otherwise reset s_art_rx, closing the app or not requesting art for the new track can leave up to 40 KB of kernel heap pinned until disconnect; reset the active old-token transfer before dropping stale chunks.
Useful? React with 👍 / 👎.
| } | ||
| bmp->addr = s_art_rx.pixels; | ||
| bmp->row_size_bytes = s_art_rx.row_size_bytes; | ||
| bmp->info.format = GBitmapFormat4BitPalette; |
There was a problem hiding this comment.
On PBL_BW targets, the 1-bit blitter only accepts 1-bit, 1-bit-palettized, and 2-bit-palettized sources, so publishing every album-art response as GBitmapFormat4BitPalette means an Aplite/Diorite watch whose phone advertises album art treats art as present, moves/hides the normal text layout, and then draws no cover. Please either avoid requesting art on 1-bit builds or convert the received image to a supported bitmap format before handing it to the UI.
Useful? React with 👍 / 👎.
|
@jplexer tested on obelix, probably could use a bit more tweaking eventually of the scroll motion but it's a good starting point |
|
not 100% convinced by the layout (at least on oblx), will tweak around |
|
gabbro is not looking good yet, still working on it |
|
ok gabbro and emery are looking good now, @jplexer can you retest when you get a sec - not urgent tho, just sometime this week pls |
Add music cover art endpoint, service handling, and app rendering. Includes new music protocol message types, prefs for the feature, and status bar layer support. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Eric Migicovsky <ericmigi@gmail.com>
Rework the music app's album-art mode so the bottom section (times, progress bar and tape) keeps the exact stock layout and code. The cover fills the top, the artist is hidden, and the track title moves beside the tape where it ping-pong scrolls horizontally when it's too long to fit (the layout measurer over-reports width vs what is drawn, so back that out of the scroll range). The clock switches to the large outlined style so it stays legible over the art. Skip the track-change slide animation over art since it fights the art-mode positions, and repaint the whole window on a mode switch to clear moved layers. Default the "Show Album Art" pref to off for a first install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Eric Migicovsky <ericmigi@gmail.com>
Refine the album-art music screen: - Show the artist name centred and outlined over the bottom of the cover (does not scroll), so it pops against the art. - Replace the ping-pong title scroll with a slower, smoother one-way ticker (a "digital sign") that runs for two passes after a track change then rests showing the start. Draw two copies a measured-width apart so the repeat never overlaps the first. - Outline the clock over the art. - Tighten the title slot and centre it vertically between the progress bar and the bottom of the screen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Eric Migicovsky <ericmigi@gmail.com>
- Draw the over-art clock white with a black outline (matching the artist) instead of black, so the time reads over the cover. - Use the larger bold clock in the no-art screen too, for consistency. - Nudge the scrolling title up a few px so it sits centred between the progress bar and the bottom of the screen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Eric Migicovsky <ericmigi@gmail.com>
The music app and endpoint now reference the new album-art music-service functions, so the unit tests need stubs for them: - test_music: stub music_album_art_lock/unlock/is_current, music_is_album_art_supported, music_request_album_art, music_has_now_playing, music_get_now_playing_generation, and shell_prefs_get_music_show_album_art (no art in these tests). - test_music_endpoint: stub gbitmap_format_get_row_size_bytes (referenced by the album-art reassembly path, not exercised here). Refresh the music render goldens for the larger bold clock now used on the no-art screen (no_music is unchanged — it uses a separate window). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Eric Migicovsky <ericmigi@gmail.com>
Per review, album art no longer rides the music endpoint. Add a generic image-fetch endpoint (0x35) the watch pulls from: it sends an ImageRequest naming the image type, encoding, dimensions and type-specific parameters, and the phone streams back an ImageResponse. Album art is the first consumer; notification images etc. can reuse it. Key differences from the old music-endpoint album art: - The watch, not the phone, chooses the size and format. The music app asks for its art-square width (full display width on round so the cover spans edge to edge with the action bar overlapping; content width on rect). - The album-art request carries the current track's title and artist, so the phone returns art for the track the watch is actually showing rather than whatever is "current" when it happens to reply (fixes the art that lagged a track behind). Rename the CommSessionMusicAlbumArtSupport capability (bit 17) to the generic CommSessionImagingSupport. The music service registers an imaging handler that stores delivered art; music_is_album_art_supported / music_request_album_art and the album-art music-endpoint messages are removed. Add a shared imaging test stub. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Eric Migicovsky <ericmigi@gmail.com>
ed99b93 to
61227e4
Compare
2 participants
Album art on the Music app
Adds album-art cover support to the Music app (MOB-10062). When the paired phone provides artwork, the music screen shows the cover filling the top, the artist name centred and outlined over the bottom of the cover, and the track title as a one-way "digital-sign" ticker beside the tape (two passes after a track change, then rests at the start). The clock is drawn large + outlined so it reads over the art. When there's no art (or the feature is off/unsupported) the screen falls back to the exact original text layout.
Wire protocol
Watch-pull model over the existing Music endpoint (0x20):
GetAlbumArt(0x09) when a track needs art and the phone advertises support.AlbumArtResponse(0x14):[token][flags][offset][len]([w][h][paletteCount][palette]) [pixels], 4bpp palettized (GColor8). ANO_ARTchunk tells the watch to stay on the text screen.Capability gating / graceful fallback
Strictly pull-driven and capability-gated, so it stays inert unless both ends are new:
CommSessionMusicAlbumArtSupport(bit 17) and re-checks before sending.GetAlbumArt, so the phone never sends art.musicShowAlbumArtsynced pref: old firmware rejects the unknown key with a benignWRN(no crash).Testing
Companion phone-side change: coredevices/CoreApp#(album-art in libpebble3).
🤖 Generated with Claude Code