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

Fix CoverartCacheManager #2325

Merged
merged 9 commits into from
Apr 8, 2024

Conversation

hoffie
Copy link
Contributor

@hoffie hoffie commented Apr 6, 2024

Previously, an ERROR was logged for each song without cover art when the Web UI was open.
This commit avoids the error, caches the no-cover-art result and saves a roundtrip to mpd for all no-cover-art songs.

Note: This will start creating an empty file for each song without cover art in order to speed up the listing. This might be surprising at first, but is in line with how the current caching mechanism works.

hoffie and others added 2 commits April 6, 2024 23:18
Previously, an ERROR was logged for each song without cover art when the
Web UI was open.
This commit avoids the error, caches the no-cover-art result and saves a
roundtrip to mpd for all no-cover-art songs.
@pabera
Copy link
Collaborator

pabera commented Apr 7, 2024

Thanks a lot. It's a consequent development. I have refactored the code a little, reducing some code and logical statements.

@pabera
Copy link
Collaborator

pabera commented Apr 7, 2024

Sorry for hijacking your RR so much. Your change is great but the original code was not very well written. I used the opportunity to correct my initial approach :-)

@s-martin s-martin added enhancement future3 Relates to future3 development labels Apr 7, 2024
@pabera
Copy link
Collaborator

pabera commented Apr 8, 2024

Thanks again for the initial commit and the trigger, to work on this feature again. A few things I have changed:

  1. CoverArt retrieval through python-mpd2 created a lot of I/O errors. Moved to mutagen instead
  2. Moved all Cover Art I/O into CoverCacheManager, the Player class now doesn't have to bother with it anymore
  3. Created separated thread for rendering cache files
  4. Added flush_cache function in order to be able to delete all cached files

@pabera pabera self-assigned this Apr 8, 2024
@pabera pabera changed the title Fix CoverartCacheManager for songs with no art (future3) Fix CoverartCacheManager Apr 8, 2024
@pabera pabera added this to the v3.6 milestone Apr 8, 2024
@pabera pabera linked an issue Apr 8, 2024 that may be closed by this pull request
@pabera pabera mentioned this pull request Apr 8, 2024
@pabera pabera linked an issue Apr 8, 2024 that may be closed by this pull request
@pabera pabera merged commit 3865c5a into MiczFlor:future3/develop Apr 8, 2024
22 checks passed
@hoffie
Copy link
Contributor Author

hoffie commented Apr 8, 2024

Sorry for hijacking your RR so much.

No worries!

Moved all Cover Art I/O into CoverCacheManager, the Player class now doesn't have to bother with it anymore
Created separated thread for rendering cache files

This sounds really useful. 👍 The web UI is so nice and snappy, and the cover art stuff seemed to have made it sluggish at times in the past.

Should users be advised to clear their cover art caches on upgrade to get rid of the unnecessary old-style files maybe?

@pabera
Copy link
Collaborator

pabera commented Apr 8, 2024

This sounds really useful. 👍 The web UI is so nice and snappy, and the cover art stuff seemed to have made it sluggish at times in the past.

Thanks! ✌️

Should users be advised to clear their cover art caches on upgrade to get rid of the unnecessary old-style files maybe?

That's a good idea. I will add a how-to note to the release PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement future3 Relates to future3 development
Projects
None yet
3 participants