Skip to content
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

add optional base64 image parameter to newscrobble #324

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

duckfromdiscord
Copy link
Contributor

What?

I would like to add a new optional parameter to the scrobble API function newscrobble that allows (but does not require) a scrobble client to include a base64 encoded album/track art image alongside a scrobble.

Why?

For example, many songs and albums on Bandcamp do not have their art stored on the usual sources like Deezer or Spotify. Scrobble clients that include album art alongside their scrobble requests can remove an extra manual step of having to download the album art and reupload it to maloja. I've had to do this many times and I've come up with this idea to automate the process.

I've already added support for this feature to Web Scrobbler: duckfromdiscord/web-scrobbler@2dd8790. I've tested it and it works, and I will open a PR there if this one is merged.

How?

To use this, all a scrobble client has to do is encode an image in base64 and include it in the image parameter to newscrobble.

Additional context

Let me know if there's anything I have to change to get this PR merged. I'm not 100% sure if all edge cases work, but I've tested a couple albums on Bandcamp that work. The edge cases I've tested are where album artists are not the same as track artists, and also tracks that have multiple artists. The way I've mitigated these issues is by using the parsed scrobble for info instead of the info provided directly to newscrobble.

Another way of doing this could be sending a URL from a scrobbler to maloja, but I chose not to do this because of possible malicious URLS (i.e. IP grabbers) and also because images.set_image already accepts base64.

@m7a
Copy link

m7a commented Mar 24, 2024

I just tried out this patch and it works well for me. My only issue getting started with it was to recall that the data format is not only base64, but needs some metadata prefixed of form data:image/type;base64,.

I tested with a locally modified mpdscrobble program. I would love to see the Base64 image support to be merged in upstream Maloja such that it might be possible to extend programs like mpdscrobble to support this feature?

My use case for this is to fill-in the image data in Maloja even without connecting to any external databases from Maloja's side.

@duckfromdiscord
Copy link
Contributor Author

I just tried out this patch and it works well for me. My only issue getting started with it was to recall that the data format is not only base64, but needs some metadata prefixed of form data:image/type;base64,.

@m7a Excited you tried this out and got it to work! Yeah, it uses the same internal function to set images as the web frontend. I did this on purpose because this is how an image URI is formed, and also how Web Scrobbler stores images. I've already written a pull request that will be sent in when this gets merged, to get Web Scrobbler to support this. I spoke to the dev of Web Scrobbler about this too.

I tested with a locally modified mpdscrobble program. I would love to see the Base64 image support to be merged in upstream Maloja such that it might be possible to extend programs like mpdscrobble to support this feature?

It is @krateng's call for this to be merged.

My use case for this is to fill-in the image data in Maloja even without connecting to any external databases from Maloja's side.

This should definitely be possible. The image parameter takes image data URIs, not URL links, so the image data itself is sent with the request. I did this on purpose.

@m7a
Copy link

m7a commented Mar 25, 2024

My only issue getting started with it was to recall that the data format is not only base64, but needs some metadata prefixed of form data:image/type;base64,.

Yeah, it uses the same internal function to set images as the web frontend. I did this on purpose because this is how an image URI is formed, and also how Web Scrobbler stores images. I've already written a pull request that will be sent in when this gets merged, to get Web Scrobbler to support this.

I think it is perfectly fine that it is designed this way, it just wasn't obvious from the API documentation 😄

@krateng
Copy link
Owner

krateng commented May 5, 2024

Hey, sorry for the delay.

I think when an image is sent it should be explicitly passed as album or track image, since we don't know what the client picks and we don't want it to be assigned to the wrong entity.

@duckfromdiscord
Copy link
Contributor Author

duckfromdiscord commented May 6, 2024

What about if the user specifies an album image, but there is no result['track']['album']? Meaning there is no album provided nor one in the database?

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.

3 participants