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.

Sign up for GitHub

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

Jump to bottom

fetchart: Use Cover Art Archive thumbnails #3637

Merged
merged 1 commit into from
Jun 27, 2020
Merged

fetchart: Use Cover Art Archive thumbnails #3637

merged 1 commit into from
Jun 27, 2020

Conversation

mtrolley
Copy link
Contributor

The Cover Art Archive API offers pre-resized thumbnails of cover
art. If the maxwidth option of fetchart matches one of the
supported Cover Art Archive thumbnail sizes fetch it directly
instead of fetching the full size image then resizing it.

@mtrolley
Copy link
Contributor Author

Cover Art Archive API reference: https://musicbrainz.org/doc/Cover_Art_Archive/API#.2Frelease.2F.7Bmbid.7D.2F.28.7Bid.7D.7Cfront.7Cback.29-.28250.7C500.7C1200.29

Tested manually by setting the fetchart maxwidth option to:

  • each of the supported values from Cover Art Archive (250, 500, 1200) and verify the fetched image is identical to the Cover Art Archive image
  • a value that did not match one of those values and verified the final cover art file was resized to the proper size
  • no value and verified that the full size art was retrieved

sampsyo
sampsyo reviewed Jun 27, 2020
View reviewed changes
Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome; seems great! Could you please add a quick entry to the changelog describing what has changed?

@mtrolley mtrolley requested a review from sampsyo June 27, 2020 16:23
sampsyo
sampsyo reviewed Jun 27, 2020
View reviewed changes
Copy link
Member

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one thought to avoid some unpredictable mutation.

@mtrolley
fetchart: Use Cover Art Archive thumbnails
2131f6f 
The Cover Art Archive API offers pre-resized thumbnails of cover
art. If the `maxwidth` option of `fetchart` matches one of the
supported Cover Art Archive thumbnail sizes fetch it directly
instead of fetching the full size image then resizing it.
@mtrolley mtrolley requested a review from sampsyo June 27, 2020 16:35
sampsyo added a commit that referenced this pull request Jun 27, 2020
@sampsyo
Thanks & bug link for #3637
db181d3
@sampsyo sampsyo merged commit 0cd5293 into beetbox:master Jun 27, 2020
@sampsyo
Copy link
Member

sampsyo commented Jun 27, 2020

Looks excellent; thanks again!

@mtrolley mtrolley deleted the topic/coverart-thumb branch June 27, 2020 17:15
@mtrolley
Copy link
Contributor Author

@sampsyo - Unfortunately I discovered a problem with this after it was merged, which I logged here: #3638

I'll work on getting it fixed ASAP. Let me know if you'd rather I revert this change until then.

@mtrolley mtrolley mentioned this pull request Jun 27, 2020
Merged
sampsyo added a commit that referenced this pull request Jun 29, 2020
@sampsyo
Thanks & bug link for #3637
aed81da
@snejus snejus mentioned this pull request Dec 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sampsyo sampsyo Awaiting requested review from sampsyo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mtrolley @sampsyo