Merge pull request #6 from goetzc/fix/high-cpu-usage

Fix high CPU usage: optimize artwork fetches and idle wake-ups

src/corefoundationasyncio/eventloop.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Copied from https://github.com/alberthier/corefoundationasyncio/blob/5061b9b7daa8bcd40d54d58432d84dcc0a339ca6/corefoundationasyncio.py
 # This module is copied, rather than simply installed, because the PyPI version of this module depends on *all* of PyObjC even though it only needs pyobjc-framework-Cocoa.
-# There's an open PR to fix this: httpsV//github.com/alberthier/corefoundationasyncio/pull/3
+# There's an open PR to fix this: https://github.com/alberthier/corefoundationasyncio/pull/3
 
 
 import asyncio
@@ -10,8 +10,8 @@ import threading
 
 from CoreFoundation import (
     CFRunLoopGetCurrent,
-    CFRunLoopTimerCreate, CFRunLoopAddTimer, CFRunLoopRemoveTimer, CFAbsoluteTimeGetCurrent,
+    CFRunLoopTimerCreate, CFRunLoopAddTimer, CFRunLoopRemoveTimer, CFRunLoopTimerInvalidate, CFAbsoluteTimeGetCurrent,
-    CFFileDescriptorCreate, CFFileDescriptorIsValid, CFFileDescriptorEnableCallBacks, CFFileDescriptorDisableCallBacks,
+    CFFileDescriptorCreate, CFFileDescriptorIsValid, CFFileDescriptorEnableCallBacks, CFFileDescriptorDisableCallBacks, CFFileDescriptorInvalidate,
     CFFileDescriptorCreateRunLoopSource, CFRunLoopAddSource, CFRunLoopRemoveSource,
     kCFAllocatorDefault, kCFRunLoopDefaultMode, kCFRunLoopCommonModes,
     kCFFileDescriptorReadCallBack, kCFFileDescriptorWriteCallBack
@@ -106,8 +106,15 @@ class CoreFoundationEventLoop(asyncio.SelectorEventLoop):
         if handle.cancelled():
             return
         def ontimeout(cf_timer, info):
-            if not handle.cancelled():
+            try:
-                handle._run()
+                if not handle.cancelled():
+                    handle._run()
+            finally:
+                # Explicitly invalidate/remove one-shot timers. Relying on
+                # implicit cleanup can leak CoreFoundation timer objects under
+                # long-running loads.
+                CFRunLoopRemoveTimer(self._runloop, cf_timer, kCFRunLoopCommonModes)
+                CFRunLoopTimerInvalidate(cf_timer)
         when = handle.when() if is_timer else self.time()
         cf_timer = CFRunLoopTimerCreate(kCFAllocatorDefault, when, 0, 0, 0, ontimeout, None)
         CFRunLoopAddTimer(self._runloop, cf_timer, kCFRunLoopCommonModes)
@@ -116,7 +123,10 @@ class CoreFoundationEventLoop(asyncio.SelectorEventLoop):
             handle._scheduled = True
 
     def _timer_handle_cancelled(self, handle):
-        CFRunLoopRemoveTimer(self._runloop, handle.cf_runloop_timer, kCFRunLoopCommonModes)
+        if handle.cf_runloop_timer is not None:
+            CFRunLoopRemoveTimer(self._runloop, handle.cf_runloop_timer, kCFRunLoopCommonModes)
+            CFRunLoopTimerInvalidate(handle.cf_runloop_timer)
+            handle.cf_runloop_timer = None
 
     def time(self):
         return CFAbsoluteTimeGetCurrent()
@@ -161,6 +171,7 @@ class CoreFoundationEventLoop(asyncio.SelectorEventLoop):
             CFFileDescriptorDisableCallBacks(entry.cf_fd, event)
         else:
             CFRunLoopRemoveSource(self._runloop, entry.cf_source, kCFRunLoopDefaultMode)
+            CFFileDescriptorInvalidate(entry.cf_fd)
         return True
 
     def _add_reader(self, fd, callback, *args):

src/mpd_now_playable/cli.py

@@ -1,4 +1,5 @@
 import asyncio
+import sys
 from collections.abc import Iterable
 
 from rich import print
@@ -22,7 +23,23 @@ async def listen(
 	await listener.loop(receivers)
 
 
+def print_help() -> None:
+	print("Usage: mpd-now-playable [OPTIONS]")
+	print("")
+	print("Options:")
+	print("  -h, --help     Show this help message and exit.")
+	print("  -v, --version  Show version and exit.")
+
+
 def main() -> None:
+	args = set(sys.argv[1:])
+	if "-h" in args or "--help" in args:
+		print_help()
+		return
+	if "-v" in args or "--version" in args:
+		print(f"mpd-now-playable v{__version__}")
+		return
+
 	print(f"mpd-now-playable v{__version__}")
 	config = loadConfig()
 	print(config)
@@ -34,7 +51,7 @@ def main() -> None:
 	asyncio.run(
 		listen(config, listener, receivers),
 		loop_factory=factory.make_loop,
-		debug=True,
+		debug=False,
 	)
 
 

src/mpd_now_playable/mpd/artwork_cache.py

@@ -30,19 +30,28 @@ class MpdArtworkCache:
 	mpd: MpdStateHandler
 	album_cache: Cache[Artwork]
 	track_cache: Cache[Artwork]
+	pending_tracks: set[str]
 
 	def __init__(self, mpd: MpdStateHandler, cache_url: URL = MEMORY):
 		self.mpd = mpd
 		self.album_cache = make_cache(ArtworkSchema, cache_url, "album")
 		self.track_cache = make_cache(ArtworkSchema, cache_url, "track")
+		self.pending_tracks = set()
 
 	async def get_cached_artwork(self, song: CurrentSongResponse) -> bytes | None:
-		art = await self.track_cache.get(calc_track_key(song))
+		track_key = calc_track_key(song)
-		if art:
+		art = await self.track_cache.get(track_key)
-			return art.data
+		if art is not None:
+			# NoArtwork is a valid cached value too: returning None here avoids
+			# repeatedly re-querying MPD for files that have no embedded art.
+			if art:
+				return art.data
+			return None
 
 		# If we don't have track artwork cached, go find some.
-		run_background_task(self.cache_artwork(song))
+		if track_key not in self.pending_tracks:
+			self.pending_tracks.add(track_key)
+			run_background_task(self.cache_artwork(song))
 
 		# Even if we don't have cached track art, we can try looking for cached album art.
 		art = await self.album_cache.get(calc_album_key(song))
@@ -52,10 +61,16 @@ class MpdArtworkCache:
 		return None
 
 	async def cache_artwork(self, song: CurrentSongResponse) -> None:
-		art = to_artwork(await self.mpd.get_art(song["file"]))
+		track_key = calc_track_key(song)
 		try:
-			await self.album_cache.add(calc_album_key(song), art, ttl=CACHE_TTL)
+			art = to_artwork(await self.mpd.get_art(song["file"]))
-		except ValueError:
+			try:
-			pass
+				await self.album_cache.add(calc_album_key(song), art, ttl=CACHE_TTL)
-		await self.track_cache.set(calc_track_key(song), art, ttl=CACHE_TTL)
+			except ValueError:
-		await self.mpd.refresh()
+				pass
+			await self.track_cache.set(track_key, art, ttl=CACHE_TTL)
+			# Refresh receivers only when we discovered actual artwork.
+			if art:
+				await self.mpd.refresh()
+		finally:
+			self.pending_tracks.discard(track_key)

src/mpd_now_playable/mpd/convert/to_playback.py

@@ -17,10 +17,13 @@ def to_queue(mpd: MpdState) -> Queue:
 
 def to_mixramp(mpd: MpdState) -> MixRamp:
 	delay = mpd.status.get("mixrampdelay", 0)
+	db = mpd.status.get("mixrampdb", 0)
 	if delay == "nan":
 		delay = 0
+	if db == "nan":
+		db = 0
 	return MixRamp(
-		db=float(mpd.status.get("mixrampdb", 0)),
+		db=float(db),
 		delay=float(delay),
 	)
 

src/mpd_now_playable/mpd/listener.py

@@ -3,7 +3,6 @@ from collections.abc import Iterable
 
 from mpd.asyncio import MPDClient
 from mpd.base import CommandError
-from rich import print as rprint
 from yarl import URL
 
 from ..config.model import MpdConfig
@@ -18,17 +17,23 @@ from .types import MpdState
 
 
 class MpdStateListener(Player):
+	# Subsystems relevant to now-playing metadata and remote controls.
+	# Listening to all MPD subsystems can cause noisy wakeups (e.g. database
+	# updates), which drives unnecessary status/currentsong polling.
+	WATCHED_SUBSYSTEMS = ("player", "mixer", "options", "playlist", "partition")
 	config: MpdConfig
 	client: MPDClient
 	receivers: Iterable[Receiver]
 	art_cache: MpdArtworkCache
 	idle_count = 0
+	last_playback: Playback | None
 
 	def __init__(self, cache: URL | None = None) -> None:
 		self.client = MPDClient()
 		self.art_cache = (
 			MpdArtworkCache(self, cache) if cache else MpdArtworkCache(self)
 		)
+		self.last_playback = None
 
 	async def start(self, conf: MpdConfig) -> None:
 		self.config = conf
@@ -53,9 +58,12 @@ class MpdStateListener(Player):
 		# Notify our receivers of the initial state MPD is in when this script loads up.
 		await self.update_receivers()
 		# And then wait for stuff to change in MPD. :)
-		async for subsystems in self.client.idle():
+		async for subsystems in self.client.idle(self.WATCHED_SUBSYSTEMS):
 			# If no subsystems actually changed, we don't need to update the receivers.
 			if not subsystems:
+				# MPD/python-mpd2 can occasionally wake idle() without reporting a
+				# changed subsystem; avoid a hot loop if that happens repeatedly.
+				await asyncio.sleep(0.1)
 				continue
 			self.idle_count += 1
 			await self.update_receivers()
@@ -79,7 +87,9 @@ class MpdStateListener(Player):
 
 		state = MpdState(status, current, art)
 		pb = to_playback(self.config, state)
-		rprint(pb)
+		if pb == self.last_playback:
+			return
+		self.last_playback = pb
 		await self.update(pb)
 
 	async def update(self, playback: Playback) -> None:

src/mpd_now_playable/receivers/cocoa/convert/playback_to_media_item.py

@@ -19,7 +19,8 @@ def playback_to_media_item(playback: Playback) -> NSMutableDictionary:
 	if song := playback.active_song:
 		nowplaying_info = song_to_media_item(song)
 	nowplaying_info[MPNowPlayingInfoPropertyPlaybackQueueCount] = playback.queue.length
-	nowplaying_info[MPNowPlayingInfoPropertyPlaybackQueueIndex] = playback.queue.current
+	if playback.queue.current is not None:
+		nowplaying_info[MPNowPlayingInfoPropertyPlaybackQueueIndex] = playback.queue.current
 	return nowplaying_info