Skip to content

Add Support for Cover Art #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Conversation

mringwal
Copy link

This PR enables support for fetching Cover Art via AVRCP if supported by the remote device. Supported on current iOS and Android smartphones. On Android 12, you need to enable support for AVRCP 1.6 in the developer settings.

The cover is shown as an 25x25 pixel at offset 3/3. After 3 seconds, the display switches back to the spectrum analyzer.

To use, please update the BTstack submodule at pico-sdk/lib/btstack to the current version of develop / commit 6544475.

@Gadgetoid
Copy link
Owner

This is awesome, thank you. I'll let the CI run with the expectation that it'll fail (it has), but I don't mind including some code to hotfix the Pico SDK submodule to the correct version while we wait for an upstream update.

I'll test this tomorrow and see how it looks!

I wonder if it's worth porting to another display product to better showcase this, too. Though we currently have no good mix of hi-res display and audio, we have things in the pipeline. @MichaelBell reckon this would be feasible (if a little redundant) on the HDMI stick?

@MichaelBell
Copy link
Contributor

Awesome! I just got bluetooth audio with a spectrum analyzer working on the HDMI DV stick - this will be a great addition!

@Gadgetoid
Copy link
Owner

HDMI DV stick

Ahh curse the HDMI branding's dark and mysterious overlords.

@mringwal I needed the following changes to build:

diff --git a/bluetooth/bluetooth.cmake b/bluetooth/bluetooth.cmake
index a937e03..974e28c 100644
--- a/bluetooth/bluetooth.cmake
+++ b/bluetooth/bluetooth.cmake
@@ -9,6 +9,8 @@ add_library(picow_bt_example_common INTERFACE)
 target_sources(picow_bt_example_common INTERFACE
     ${CMAKE_CURRENT_LIST_DIR}/common.cpp
     ${PICO_BTSTACK_PATH}/src/btstack_audio.c
+    ${PICO_BTSTACK_PATH}/src/classic/avrcp_cover_art_client.c
+    ${PICO_BTSTACK_PATH}/src/hci_event.c
 )
 
 target_link_libraries(picow_bt_example_common INTERFACE

@mringwal
Copy link
Author

@Gadgetoid you're correct about the missing files in bluetooth.cmake. They will be there once pico-sdk updates next time.

@MichaelBell do you want to try showing the covers on a screen on the dv stick? (what's it's video output btw?).

@MichaelBell
Copy link
Contributor

The DV Stick is like a Pico version of a Fire Stick - it uses Luke Wren's PicoDVI to do HDMI output DVI over an HDMI connector. There's a Pico W on it and then a second RP2040 to drive the DVI.

Looks like I should be able to just grab what you've done here and port it over - I'll put a repo for it up on GitHub later if that's working today!

@MichaelBell
Copy link
Contributor

I got this working - see https://github.com/MichaelBell/dv-bt-audio
IMG_0991

@mringwal
Copy link
Author

Awesome. Just checked, you've used HALF size, so you can at least go to full scale. Maybe the cover on the left of the FFT, while the song title (I also prefer artist and album) could stay on the buttom.

It seems like I'll get a Pico DV Stick next week, too!

@MichaelBell
Copy link
Contributor

Yeah, this was just me quickly playing with it - layout and general look definitely needs work (and agree should have album and artist too!)

@peterharperuk
Copy link

peterharperuk commented May 31, 2023

This is failing to compile with the latest btstack. I think the fix is simple.

-            status = a2dp_sink_establish_stream(device_addr, stream_endpoint->a2dp_local_seid, &a2dp_connection->a2dp_cid);
+            status = a2dp_sink_establish_stream(device_addr, &a2dp_connection->a2dp_cid);

I'm also using this raspberrypi/pico-sdk#1379 so btstack_audio.ccan be removed from bluetooth.cmake in galactic-bluetooth-audio

@mringwal
Copy link
Author

Correct. The local seid param has been removed as it wasn't actually used in the upcoming 1.5.6 btstack release.
btstack_audio_pico.c is heavily patched to support the cover art display however. This could be fixed later.

@peterharperuk
Copy link

We only have btstack_audio_pico.c in the examples, so having your own version of that should be fine.

@kevinjwalters
Copy link

@Gadgetoid Is this PR going to get merged soon? I'm currently adding Spectrogram code to the effects...

@Gadgetoid
Copy link
Owner

Currently blocked, since the build has failed 😬

Sheesh I can't believe we were working on this back in May. I have been... distracted!

@mringwal any idea what that missing file business is?

@mringwal
Copy link
Author

Hi there. Yeah, long time. I still want to make a short video and already picked a few songs with covers that are well recognizable even with 25x25 pixels...

Just added the missing font to my fork.

I've also added a text scroller that shows artist/title/album while the cover art is fetched. Let me try to finish this - hopefully this week and push an update. I think it's just that the scroller is too fast.

In general, adding the display code into the audio driver is rather ugly. two alternatives: just send the PCM from the a2dp_sink_demo to the display code, or, add a way to register a 'new audio' callback in the driver. Still, I think it should be moved to the application.

@mringwal
Copy link
Author

@Gadgetoid could you trigger a new build?

Just compiled again and the scroller is already fine (last time I couldn't read anything as it run over the display).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants