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

Cover Art Fetcher: cache fetched images per track #11084

Closed
ronso0 opened this issue Nov 21, 2022 · 3 comments · Fixed by #12301
Closed

Cover Art Fetcher: cache fetched images per track #11084

ronso0 opened this issue Nov 21, 2022 · 3 comments · Fixed by #12301

Comments

@ronso0
Copy link
Member

ronso0 commented Nov 21, 2022

Feature Description

#4851
Selecting a TagFetcher result instantly fetches the cover image. Thus, selecting multiple results to find the best-matching cover (actual picture, size, quality) would needlessly re-download images that were already viewed.

Let's cache the images per track. We can either use a strict memory limit or just clear the cache once another track is loaded.

@daschuer
Copy link
Member

I think we should do both, use a separate cache only for the dialog and clear it if we go to another track.

@ronso0 ronso0 added this to the 2.4.0 milestone Nov 14, 2023
@ronso0 ronso0 linked a pull request Nov 14, 2023 that will close this issue
3 tasks
@ywwg
Copy link
Member

ywwg commented Jan 6, 2024

@ronso0 is this really an item for 2.4? Adding a new caching layer sounds risky and complex at this late date.

@ronso0
Copy link
Member Author

ronso0 commented Jan 6, 2024

I attached it to 2.4 because it makes the new cover fetcher complete. By now it will delay the release, so it can go to 2.4.1.
The changes of #12301 are not risky, please take a look at the essence in src/library/dlgtagfetcher.cpp 178b224#diff-679b7c04399af1221df243a3d7555a986537c6a297d7d88f2ca4a03d0b325803
But we need to test how it performs of course.

@ronso0 ronso0 modified the milestones: 2.4.0, 2.4.1 Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants