-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
base: master
Are you sure you want to change the base?
Conversation
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 I tested with a locally modified 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. |
@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.
It is @krateng's call for this to be merged.
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. |
I think it is perfectly fine that it is designed this way, it just wasn't obvious from the API documentation 😄 |
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. |
What about if the user specifies an album image, but there is no |
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 tonewscrobble
.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.