Merge pull request #40 from imonlinux/let_mpv_do_the_work

Simplify mpv audio backend selection and log effective backend

linux_voice_assistant/mpv_player.py

@@ -1,115 +1,50 @@
 """
 Media player using mpv in a subprocess.
-Includes logic to detect the audio server being used (Pirewire, PulseAudio, Alsa).
+
-This refactored version simplifies the audio backend selection logic,
+This wrapper focuses on:
-improves readability with docstrings, and standardizes method signatures.
+- Simple playback control (play / pause / resume / stop)
+- Volume control and ducking
+- Notifying a done_callback when playback finishes
+- Letting mpv choose the best audio backend by default
+
+If a specific audio device is provided, it is passed directly to mpv as
+`audio-device`. Otherwise, mpv's own automatic backend/device selection is used.
 """
 from __future__ import annotations
 
 import asyncio
 import logging
 import os
-from threading import Lock, Timer
+from threading import Lock
-from typing import Callable, Iterable, List, Optional, Sequence, Tuple, Union
+from typing import Callable, List, Optional, Sequence, Union
 
+# Note: python-mpv must be installed; imported at runtime.
 from mpv import MPV
 
 _LOGGER = logging.getLogger(__name__)
 
-# -----------------------------------------------------------------------------
-
-def _set_player_opt(player: MPV, key: str, value) -> bool:
-    """Safely set an mpv option, logging any errors."""
-    try:
-        player[key] = value
-        return True
-    except Exception:
-        _LOGGER.warning("Failed to set mpv option %r=%r", key, value, exc_info=True)
-        return False
-
-def _select_backend(player: MPV, device: Optional[str]) -> None:
-    """
-    Selects the best available audio output backend for mpv.
-    
-    1. If a specific 'device' is given (e.g., 'alsa/plughw:CARD=...'), it is used directly.
-    2. If 'device' is None, it queries mpv for available devices and picks the first
-       available backend in the preferred order (pipewire > pulse > alsa).
-    3. If all else fails, it falls back to 'auto'.
-    """
-    
-    # 1. Explicit Path: User provided a specific device. Use it directly.
-    if device:
-        _LOGGER.debug(f"User specified audio device: {device}")
-        try:
-            # If it's a full path like 'alsa/plughw:...'
-            if '/' in device:
-                ao, _ = device.split('/', 1)
-                player["ao"] = ao
-                player["audio-device"] = device
-                _LOGGER.info(f"mpv backend set (explicit): ao={ao}, audio-device={device}")
-                return
-            else:
-                # Ambiguous name like 'default'. Fall through to auto-detection.
-                _LOGGER.debug(f"Ambiguous device name '{device}', falling back to auto-detection.")
-        except Exception as e:
-            _LOGGER.warning(f"Failed to parse explicit audio device {device}: {e}. Falling back to auto-detection.")
-
-    # 2. Automatic Path: No device specified or fallback from ambiguous name.
-    _LOGGER.debug("Starting audio backend auto-detection.")
-    
-    available_device_names = set()
-    try:
-        available_devices = player.audio_device_list
-        if not available_devices:
-            _LOGGER.warning("mpv returned an empty list of audio devices. Will try defaults.")
-        else:
-            available_device_names = {dev['name'] for dev in available_devices}
-            _LOGGER.debug(f"Available audio devices found: {available_device_names}")
-    except Exception as e:
-        _LOGGER.error(f"Failed to query mpv for audio devices: {e}. Will try defaults.")
-
-    # Our preferred order of drivers
-    preferred_drivers = ["pipewire", "pulse", "alsa"]
-    
-    if available_device_names:
-        for driver in preferred_drivers:
-            # Check if ANY device in the list starts with this driver's prefix
-            prefix = f"{driver}/"
-            # Also check for the simple driver name, e.g., 'alsa'
-            if any(name.startswith(prefix) for name in available_device_names) or (driver in available_device_names):
-                try:
-                    # --- THIS IS THE FIX ---
-                    # For pulse and pipewire, set audio-device to "default"
-                    # For alsa, it's safer to use "alsa" as the device
-                    device_name = "default" if driver in ["pipewire", "pulse"] else driver
-                    
-                    player["ao"] = driver
-                    player["audio-device"] = device_name
-                    _LOGGER.info(f"Auto-detected and set active backend: ao={driver}, audio-device={device_name}")
-                    return
-                except Exception as e:
-                    _LOGGER.warning(f"Failed to set auto-detected driver {driver}: {e}")
-
-    # 3. Fallback Path: No devices found, or no prefix matched.
-    _LOGGER.warning("Could not find a preferred driver in mpv's list. Telling mpv to use 'auto'.")
-    player["ao"] = "auto"
-    player["audio-device"] = "auto"
-
 
 class MpvMediaPlayer:
     """A media player class that wraps the python-mpv library."""
+
     def __init__(
         self,
-        loop: asyncio.AbstractEventLoop,
+        loop: Optional[asyncio.AbstractEventLoop],
         device: Optional[str] = None,
-        initial_volume: float = 1.0
+        initial_volume: float = 1.0,
     ) -> None:
+        """
+        :param loop: The asyncio loop used to schedule done_callback.
+                     May be None in one-off utility contexts (e.g. listing devices).
+        :param device: Optional mpv audio device name (e.g. "pulse/alsa_output.pci-0000_00_1f.3.analog-stereo").
+        :param initial_volume: Initial volume as a float 0.0–1.0.
+        """
         self.loop = loop
+
         self.player = MPV(
             video=False,
             terminal=False,
             log_handler=self._mpv_log,
-            # Set options directly in the constructor
             audio_samplerate=44100,
             audio_channels="stereo",
             keep_open="no",
@@ -117,24 +52,79 @@ class MpvMediaPlayer:
             msg_level=os.environ.get("LVA_MPV_MSG_LEVEL", "all=warn"),
         )
 
-        _select_backend(self.player, device) # <-- This is the updated function
+        # Optional: allow forcing ao via environment for power users/debugging.
+        ao_env = os.environ.get("LVA_AO")
+        if ao_env:
+            try:
+                self.player["ao"] = ao_env
+                _LOGGER.info("Forcing mpv ao=%r from LVA_AO", ao_env)
+            except Exception:
+                _LOGGER.exception("Failed to set mpv ao=%r", ao_env)
+
+        # If the caller provided a specific device, honor it directly.
+        if device:
+            try:
+                self.player["audio-device"] = device
+                _LOGGER.info("Using mpv audio-device=%r", device)
+            except Exception:
+                _LOGGER.exception("Failed to set mpv audio-device %r", device)
+
+        # Log final backend/device selection and available devices (best-effort)
+        try:
+            ao_effective = self.player["ao"]
+            # python-mpv may expose an unset option as [] – normal for "auto"
+            if isinstance(ao_effective, list) and not ao_effective:
+                ao_display = "<unset/auto>"
+            else:
+                ao_display = ao_effective
+
+            audio_device_effective = self.player["audio-device"]
+
+            try:
+                dev_list = self.player.audio_device_list or []
+                dev_summary = [
+                    f"{dev.get('name')} ({dev.get('description')})"
+                    for dev in dev_list
+                ]
+            except Exception:
+                dev_summary = ["<unavailable>"]
+
+            _LOGGER.debug(
+                "mpv audio config: ao=%r, audio-device=%r, devices=%s",
+                ao_display,
+                audio_device_effective,
+                dev_summary,
+            )
+        except Exception:
+            _LOGGER.exception("Failed to query mpv audio properties")
+
+        # Volume is 0–100 in mpv, we accept 0.0–1.0 here.
         self.set_volume(int(initial_volume * 100))
 
         self.is_playing: bool = False
-        self._playlist: List[str] = []
         self._done_callback: Optional[Callable[[], None]] = None
         self._done_callback_lock = Lock()
         self._pre_duck_volume: Optional[int] = None
 
+        # When mpv becomes idle, we treat it as end-of-playback.
         self.player.observe_property("idle-active", self._on_idle_active)
 
+    # -------------------------------------------------------------------------
+    # Public API
+    # -------------------------------------------------------------------------
+
     def play(
         self,
         url: Union[str, Sequence[str], bytes],
         done_callback: Optional[Callable[[], None]] = None,
     ) -> None:
-        """Plays a URL or a sequence of URLs."""
+        """Plays a URL or sequence of URLs.
-        self.stop() # Ensure player is in a clean state
+
+        :param url: A single URL (str/bytes) or a sequence of URLs.
+        :param done_callback: Called once when playback finishes or is stopped.
+        """
+        # Ensure player is in a clean state
+        self.stop()
 
         with self._done_callback_lock:
             self._done_callback = done_callback
@@ -150,35 +140,45 @@ class MpvMediaPlayer:
             _LOGGER.error("play() expected str, bytes, or sequence, got %r", type(url))
             self._run_done_callback()
             return
-        
+
         if not playlist:
             self._run_done_callback()
             return
-            
+
         # Load the full playlist into mpv
         self.player.playlist_clear()
         for item in playlist:
             self.player.playlist_append(item)
 
         self.is_playing = True
-        self.player.playlist_pos = 0 # Start playing from the first item
+        self.player.playlist_pos = 0  # Start playing from the first item
-        
+
     def pause(self) -> None:
         """Pauses playback."""
-        self.player.pause = True
+        try:
+            self.player.pause = True
+        except Exception:
+            _LOGGER.exception("pause() failed")
 
     def resume(self) -> None:
         """Resumes playback."""
-        self.player.pause = False
+        try:
+            self.player.pause = False
+        except Exception:
+            _LOGGER.exception("resume() failed")
 
     def stop(self) -> None:
         """Stops playback and clears the playlist."""
         if self.is_playing:
             self.is_playing = False
-            self.player.playlist_clear()
+            try:
-            self.player.command("stop")
+                self.player.playlist_clear()
-            self._run_done_callback()
+                self.player.command("stop")
-            
+            except Exception:
+                _LOGGER.exception("stop() failed")
+            finally:
+                self._run_done_callback()
+
     def set_volume(self, volume: int) -> None:
         """Sets the player volume from 0 to 100."""
         try:
@@ -202,9 +202,15 @@ class MpvMediaPlayer:
             return
         try:
             self.set_volume(self._pre_duck_volume)
+        except Exception:
+            _LOGGER.exception("unduck() failed")
         finally:
             self._pre_duck_volume = None
 
+    # -------------------------------------------------------------------------
+    # Internal callbacks
+    # -------------------------------------------------------------------------
+
     def _on_idle_active(self, _name: str, active: bool) -> None:
         """Callback triggered when mpv enters or leaves the idle state."""
         if active and self.is_playing:
@@ -213,16 +219,26 @@ class MpvMediaPlayer:
             self._run_done_callback()
 
     def _run_done_callback(self) -> None:
-        """Safely runs the done_callback on the main asyncio loop."""
+        """Safely runs the done_callback on the main asyncio loop (if any)."""
         with self._done_callback_lock:
             cb = self._done_callback
             self._done_callback = None
-        if cb:
+
+        if not cb:
+            return
+
+        # If we have an asyncio loop, schedule callback there.
+        if self.loop is not None:
             try:
-                # Use call_soon_threadsafe to run the callback on the main loop
                 self.loop.call_soon_threadsafe(cb)
             except Exception:
-                _LOGGER.exception("Error scheduling done_callback")
+                _LOGGER.exception("Error scheduling done_callback on loop")
+        else:
+            # Fallback: call directly (used in contexts where no loop is passed).
+            try:
+                cb()
+            except Exception:
+                _LOGGER.exception("Error running done_callback directly")
 
     def _mpv_log(self, level: str, prefix: str, text: str) -> None:
         """Routes mpv's internal logs to our logger."""