From 3865c5ab8af929ba05147754a60a29ce1f1bfe22 Mon Sep 17 00:00:00 2001 From: Christian Hoffmann Date: Mon, 8 Apr 2024 19:00:06 +0000 Subject: [PATCH] Fix CoverartCacheManager (#2325) * Fix CoverartCacheManager for songs with no art 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. * refactor: Reducing code and simplifying some logical statements * fix: flake8 error * refactor: reducing complexity for cache filename * refactor: introduce queuing for saving cache files * fix: remove slugify * feat: Use mutagen instead of MPD to retrieve cover art, include cache flush, and thread * fix: flake8 error * Update src/jukebox/components/playermpd/__init__.py Co-authored-by: Christian Hoffmann --------- Co-authored-by: pabera <1260686+pabera@users.noreply.github.com> --- requirements.txt | 2 +- src/jukebox/components/playermpd/__init__.py | 46 +++------- .../playermpd/coverart_cache_manager.py | 90 ++++++++++++++++--- src/jukebox/components/rpc_command_alias.py | 4 + .../albums/album-list/album-list-item.js | 4 +- 5 files changed, 97 insertions(+), 49 deletions(-) diff --git a/requirements.txt b/requirements.txt index c172a7636..8ddfc881a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,11 +10,11 @@ wheel # Jukebox Core # For USB inputs (reader, buttons) and bluetooth buttons evdev +mutagen pyalsaaudio pulsectl python-mpd2 ruamel.yaml -python-slugify # For playlistgenerator requests # For the publisher event reactor loop: diff --git a/src/jukebox/components/playermpd/__init__.py b/src/jukebox/components/playermpd/__init__.py index 4ae9458ec..dcbef2ea8 100644 --- a/src/jukebox/components/playermpd/__init__.py +++ b/src/jukebox/components/playermpd/__init__.py @@ -87,7 +87,7 @@ import logging import time import functools -from slugify import slugify +from pathlib import Path import components.player import jukebox.cfghandler import jukebox.utils as utils @@ -521,40 +521,10 @@ def play_card(self, folder: str, recursive: bool = False): @plugs.tag def get_single_coverart(self, song_url): - """ - Saves the album art image to a cache and returns the filename. - """ - base_filename = slugify(song_url) - - try: - metadata_list = self.mpd_client.listallinfo(song_url) - metadata = {} - if metadata_list: - metadata = metadata_list[0] - - if 'albumartist' in metadata and 'album' in metadata: - base_filename = slugify(f"{metadata['albumartist']}-{metadata['album']}") - - cache_filename = self.coverart_cache_manager.find_file_by_hash(base_filename) - - if cache_filename: - return cache_filename - - # Cache file does not exist - # Fetch cover art binary - album_art_data = self.mpd_client.readpicture(song_url) + mp3_file_path = Path(components.player.get_music_library_path(), song_url).expanduser() + cache_filename = self.coverart_cache_manager.get_cache_filename(mp3_file_path) - # Save to cache - cache_filename = self.coverart_cache_manager.save_to_cache(base_filename, album_art_data) - - return cache_filename - - except mpd.base.CommandError as e: - logger.error(f"{e.__class__.__qualname__}: {e} at uri {song_url}") - except Exception as e: - logger.error(f"{e.__class__.__qualname__}: {e} at uri {song_url}") - - return "" + return cache_filename @plugs.tag def get_album_coverart(self, albumartist: str, album: str): @@ -562,6 +532,14 @@ def get_album_coverart(self, albumartist: str, album: str): return self.get_single_coverart(song_list[0]['file']) + @plugs.tag + def flush_coverart_cache(self): + """ + Deletes the Cover Art Cache + """ + + return self.coverart_cache_manager.flush_cache() + @plugs.tag def get_folder_content(self, folder: str): """ diff --git a/src/jukebox/components/playermpd/coverart_cache_manager.py b/src/jukebox/components/playermpd/coverart_cache_manager.py index a7ae12eef..bb2346497 100644 --- a/src/jukebox/components/playermpd/coverart_cache_manager.py +++ b/src/jukebox/components/playermpd/coverart_cache_manager.py @@ -1,26 +1,90 @@ -import os +from mutagen.mp3 import MP3 +from mutagen.id3 import ID3, APIC +from pathlib import Path +import hashlib +import logging +from queue import Queue +from threading import Thread import jukebox.cfghandler +COVER_PREFIX = 'cover' +NO_COVER_ART_EXTENSION = 'no-art' +NO_CACHE = '' +CACHE_PENDING = 'CACHE_PENDING' + +logger = logging.getLogger('jb.CoverartCacheManager') cfg = jukebox.cfghandler.get_handler('jukebox') class CoverartCacheManager: def __init__(self): coverart_cache_path = cfg.setndefault('webapp', 'coverart_cache_path', value='../../src/webapp/build/cover-cache') - self.cache_folder_path = os.path.expanduser(coverart_cache_path) + self.cache_folder_path = Path(coverart_cache_path).expanduser() + self.write_queue = Queue() + self.worker_thread = Thread(target=self.process_write_requests) + self.worker_thread.daemon = True # Ensure the thread closes with the program + self.worker_thread.start() + + def generate_cache_key(self, base_filename: str) -> str: + return f"{COVER_PREFIX}-{hashlib.sha256(base_filename.encode()).hexdigest()}" + + def get_cache_filename(self, mp3_file_path: str) -> str: + base_filename = Path(mp3_file_path).stem + cache_key = self.generate_cache_key(base_filename) + + for path in self.cache_folder_path.iterdir(): + if path.stem == cache_key: + if path.suffix == f".{NO_COVER_ART_EXTENSION}": + return NO_CACHE + return path.name + + self.save_to_cache(mp3_file_path) + return CACHE_PENDING + + def save_to_cache(self, mp3_file_path: str): + self.write_queue.put(mp3_file_path) - def find_file_by_hash(self, hash_value): - for filename in os.listdir(self.cache_folder_path): - if filename.startswith(hash_value): - return filename - return None + def _save_to_cache(self, mp3_file_path: str): + base_filename = Path(mp3_file_path).stem + cache_key = self.generate_cache_key(base_filename) + file_extension, data = self._extract_album_art(mp3_file_path) - def save_to_cache(self, base_filename, album_art_data): - mime_type = album_art_data['type'] - file_extension = 'jpg' if mime_type == 'image/jpeg' else mime_type.split('/')[-1] - cache_filename = f"{base_filename}.{file_extension}" + cache_filename = f"{cache_key}.{file_extension}" + full_path = self.cache_folder_path / cache_filename # Works due to Pathlib - with open(os.path.join(self.cache_folder_path, cache_filename), 'wb') as file: - file.write(album_art_data['binary']) + with full_path.open('wb') as file: + file.write(data) + logger.debug(f"Created file: {cache_filename}") return cache_filename + + def _extract_album_art(self, mp3_file_path: str) -> tuple: + try: + audio_file = MP3(mp3_file_path, ID3=ID3) + except Exception as e: + logger.error(f"Error reading MP3 file {mp3_file_path}: {e}") + return (NO_COVER_ART_EXTENSION, b'') + + for tag in audio_file.tags.values(): + if isinstance(tag, APIC): + mime_type = tag.mime + file_extension = 'jpg' if mime_type == 'image/jpeg' else mime_type.split('/')[-1] + return (file_extension, tag.data) + + return (NO_COVER_ART_EXTENSION, b'') + + def process_write_requests(self): + while True: + mp3_file_path = self.write_queue.get() + try: + self._save_to_cache(mp3_file_path) + except Exception as e: + logger.error(f"Error processing write request: {e}") + self.write_queue.task_done() + + def flush_cache(self): + for path in self.cache_folder_path.iterdir(): + if path.is_file(): + path.unlink() + logger.debug(f"Deleted cached file: {path.name}") + logger.info("Cache flushed successfully.") diff --git a/src/jukebox/components/rpc_command_alias.py b/src/jukebox/components/rpc_command_alias.py index f6e238559..5a7820733 100644 --- a/src/jukebox/components/rpc_command_alias.py +++ b/src/jukebox/components/rpc_command_alias.py @@ -75,6 +75,10 @@ 'method': 'repeat', 'note': 'Repeat', 'ignore_card_removal_action': True}, + 'flush_coverart_cache': { + 'package': 'player', + 'plugin': 'ctrl', + 'method': 'flush_coverart_cache'}, # VOLUME 'set_volume': { diff --git a/src/webapp/src/components/Library/lists/albums/album-list/album-list-item.js b/src/webapp/src/components/Library/lists/albums/album-list/album-list-item.js index 75882dd0d..2c6d99180 100644 --- a/src/webapp/src/components/Library/lists/albums/album-list/album-list-item.js +++ b/src/webapp/src/components/Library/lists/albums/album-list/album-list-item.js @@ -29,7 +29,9 @@ const AlbumListItem = ({ albumartist, album, isButton = true }) => { album: album }); if (result) { - setCoverImage(`/cover-cache/${result}`); + if(result !== 'CACHE_PENDING') { + setCoverImage(`/cover-cache/${result}`); + } }; }