Merge pull request #2656 from Mathew-D/v5

fix(media): MPRIS change to async

src/dbus/mpris/mpris_service.cpp

@@ -54,20 +54,6 @@ namespace {
     return loop_status == "None" || loop_status == "Track" || loop_status == "Playlist";
   }
 
-  template <typename T>
-  T get_property_or(sdbus::IProxy& proxy, std::string_view interface_name, std::string_view property_name, T fallback) {
-    try {
-      const sdbus::Variant value = proxy.getProperty(property_name).onInterface(interface_name);
-      return value.get<T>();
-    } catch (const sdbus::Error&) {
-      return fallback;
-    }
-  }
-
-  std::map<std::string, sdbus::Variant> get_metadata_or(sdbus::IProxy& proxy) {
-    return get_property_or(proxy, k_mpris_player_interface, "Metadata", std::map<std::string, sdbus::Variant>{});
-  }
-
   std::string get_string_from_variant(const std::map<std::string, sdbus::Variant>& values, std::string_view key) {
     const auto it = values.find(std::string{key});
     if (it == values.end()) {
@@ -403,18 +389,47 @@ std::optional<MprisPlayerInfo> MprisService::activePlayer() const {
 
 void MprisService::refreshPlayerPosition(const std::string& busName, bool notifyChange) {
   const auto proxyIt = m_playerProxies.find(busName);
-  const auto playerIt = m_players.find(busName);
-  if (proxyIt == m_playerProxies.end() || playerIt == m_players.end()) {
+  if (proxyIt == m_playerProxies.end() || !m_players.contains(busName)) {
     return;
   }
 
+  const std::weak_ptr<void> aliveGuard = m_aliveGuard;
   try {
+    proxyIt->second->callMethodAsync("Get")
+        .onInterface(k_properties_interface)
+        .withArguments(std::string{k_mpris_player_interface}, std::string{"Position"})
+        .uponReplyInvoke(
+            [this, aliveGuard, busName, notifyChange](std::optional<sdbus::Error> err, sdbus::Variant value) {
+              if (aliveGuard.expired()) {
+                return;
+              }
+              if (err.has_value()) {
+                kLog.warn("position refresh failed name={} err={}", busName, err->what());
+                return;
+              }
+              const auto rawPositionUs = value.get<int64_t>();
+              DeferredCall::callLater([this, aliveGuard, busName, notifyChange, rawPositionUs]() {
+                if (aliveGuard.expired()) {
+                  return;
+                }
+                applyPositionSample(busName, rawPositionUs, notifyChange);
+              });
+            });
+  } catch (const sdbus::Error& e) {
+    kLog.warn("position refresh dispatch failed name={} err={}", busName, e.what());
+  }
+}
+
+void MprisService::applyPositionSample(const std::string& busName, int64_t rawPositionUs, bool notifyChange) {
+  const auto playerIt = m_players.find(busName);
+  if (playerIt == m_players.end()) {
+    return;
+  }
+
   const auto now = std::chrono::steady_clock::now();
   const auto seekCommandIt = m_lastSeekCommandAt.find(busName);
   const bool recentLocalSeek =
       seekCommandIt != m_lastSeekCommandAt.end() && now - seekCommandIt->second <= k_seek_pause_grace_window;
-    const auto rawPositionUs =
-        proxyIt->second->getProperty("Position").onInterface(k_mpris_player_interface).get<int64_t>();
   auto offsetIt = m_positionOffsetsUs.find(busName);
   std::int64_t offsetUs = offsetIt != m_positionOffsetsUs.end() ? offsetIt->second : 0;
   std::int64_t normalizedUs = std::max<std::int64_t>(0, rawPositionUs - offsetUs);
@@ -461,8 +476,13 @@ void MprisService::refreshPlayerPosition(const std::string& busName, bool notify
     }
     if (playerIt->second.playbackStatus != "Stopped") {
       auto& timerId = m_positionResyncTimers[busName];
-        timerId = TimerManager::instance().start(timerId, k_position_retry_interval,
-                                                 [this, busName]() { refreshPlayerPosition(busName, true); });
+      const std::weak_ptr<void> timerAliveGuard = m_aliveGuard;
+      timerId = TimerManager::instance().start(timerId, k_position_retry_interval, [this, timerAliveGuard, busName]() {
+        if (timerAliveGuard.expired()) {
+          return;
+        }
+        refreshPlayerPosition(busName, true);
+      });
     }
     return;
   }
@@ -532,8 +552,14 @@ void MprisService::refreshPlayerPosition(const std::string& busName, bool notify
       m_pendingPositionCandidateAt[busName] = now;
       if (playerIt->second.playbackStatus == "Playing") {
         auto& timerId = m_positionResyncTimers[busName];
+        const std::weak_ptr<void> timerAliveGuard = m_aliveGuard;
         timerId = TimerManager::instance().start(timerId, k_position_candidate_retry_interval,
-                                                   [this, busName]() { refreshPlayerPosition(busName, true); });
+                                                 [this, timerAliveGuard, busName]() {
+                                                   if (timerAliveGuard.expired()) {
+                                                     return;
+                                                   }
+                                                   refreshPlayerPosition(busName, true);
+                                                 });
       }
       return;
     }
@@ -545,8 +571,14 @@ void MprisService::refreshPlayerPosition(const std::string& busName, bool notify
         m_pendingPositionCandidateUs[busName] = normalizedUs;
         m_pendingPositionCandidateAt[busName] = now;
         auto& timerId = m_positionResyncTimers[busName];
+        const std::weak_ptr<void> timerAliveGuard = m_aliveGuard;
         timerId = TimerManager::instance().start(timerId, k_position_candidate_retry_interval,
-                                                   [this, busName]() { refreshPlayerPosition(busName, true); });
+                                                 [this, timerAliveGuard, busName]() {
+                                                   if (timerAliveGuard.expired()) {
+                                                     return;
+                                                   }
+                                                   refreshPlayerPosition(busName, true);
+                                                 });
         return;
       }
     }
@@ -559,40 +591,29 @@ void MprisService::refreshPlayerPosition(const std::string& busName, bool notify
         m_hasAuthoritativePositionSample.contains(busName) && m_hasAuthoritativePositionSample.at(busName);
     if (playerIt->second.playbackStatus == "Playing" && !hasAuthoritativeSample) {
       auto& timerId = m_positionResyncTimers[busName];
-        timerId = TimerManager::instance().start(timerId, k_position_retry_interval,
-                                                 [this, busName]() { refreshPlayerPosition(busName, true); });
+      const std::weak_ptr<void> timerAliveGuard = m_aliveGuard;
+      timerId = TimerManager::instance().start(timerId, k_position_retry_interval, [this, timerAliveGuard, busName]() {
+        if (timerAliveGuard.expired()) {
+          return;
+        }
+        refreshPlayerPosition(busName, true);
+      });
     }
     return;
   }
 
   if (playerIt->second.positionUs != normalizedUs) {
     playerIt->second.positionUs = normalizedUs;
-      m_lastPositionSampleAt[busName] = now;
-      m_hasAuthoritativePositionSample[busName] = true;
-      m_pendingPositionCandidateUs.erase(busName);
-      m_pendingPositionCandidateMatches.erase(busName);
-      m_pendingPositionCandidateAt.erase(busName);
     if (notifyChange && m_changeCallback) {
       m_changeCallback();
     }
-    } else {
+  }
+
   m_lastPositionSampleAt[busName] = now;
   m_hasAuthoritativePositionSample[busName] = true;
   m_pendingPositionCandidateUs.erase(busName);
   m_pendingPositionCandidateMatches.erase(busName);
   m_pendingPositionCandidateAt.erase(busName);
-    }
-
-    const bool hasAuthoritativeSample =
-        m_hasAuthoritativePositionSample.contains(busName) && m_hasAuthoritativePositionSample.at(busName);
-    if (playerIt->second.playbackStatus == "Playing" && !hasAuthoritativeSample) {
-      auto& timerId = m_positionResyncTimers[busName];
-      timerId = TimerManager::instance().start(timerId, k_position_retry_interval,
-                                               [this, busName]() { refreshPlayerPosition(busName, true); });
-    }
-  } catch (const sdbus::Error& e) {
-    kLog.warn("position refresh failed name={} err={}", busName, e.what());
-  }
 }
 
 MprisPlayerInfo MprisService::projectedPlayerInfo(const MprisPlayerInfo& player) const {
@@ -673,6 +694,51 @@ void MprisService::registerIpc(IpcService& ipc) {
       "media <next|previous|toggle>", "Control active media playback");
 }
 
+std::function<void(std::optional<sdbus::Error>)> MprisService::makeAsyncReplyHandler(std::string op,
+                                                                                     std::string busName) {
+  const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+  return [this, aliveGuard, op = std::move(op), busName = std::move(busName)](std::optional<sdbus::Error> err) {
+    if (aliveGuard.expired()) {
+      return;
+    }
+    if (err.has_value()) {
+      kLog.warn("{} failed name={} err={}", op, busName, err->what());
+      return;
+    }
+
+    DeferredCall::callLater([this, aliveGuard, busName]() {
+      if (aliveGuard.expired()) {
+        return;
+      }
+      addOrRefreshPlayer(busName);
+    });
+  };
+}
+
+std::function<void(std::optional<sdbus::Error>)>
+MprisService::makeAsyncReplyHandler(std::string op, std::string busName, std::string_view method) {
+  const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+  return [this, aliveGuard, op = std::move(op), busName = std::move(busName),
+          method = std::string(method)](std::optional<sdbus::Error> err) {
+    if (aliveGuard.expired()) {
+      return;
+    }
+    if (err.has_value()) {
+      kLog.warn("{} failed name={} method={} err={}", op, busName, method, err->what());
+      return;
+    }
+
+    kLog.debug("{} name={} method={}", op, busName, method);
+
+    DeferredCall::callLater([this, aliveGuard, busName]() {
+      if (aliveGuard.expired()) {
+        return;
+      }
+      addOrRefreshPlayer(busName);
+    });
+  };
+}
+
 bool MprisService::playPause(const std::string& busName) {
   const auto it = m_players.find(busName);
   if (it == m_players.end()) {
@@ -742,12 +808,14 @@ bool MprisService::seek(const std::string& busName, int64_t offsetUs) {
   }
 
   try {
-    proxyIt->second->callMethod("Seek").onInterface(k_mpris_player_interface).withArguments(offsetUs);
+    proxyIt->second->callMethodAsync("Seek")
+        .onInterface(k_mpris_player_interface)
+        .withArguments(offsetUs)
+        .uponReplyInvoke(makeAsyncReplyHandler("seek", busName));
     m_lastSeekCommandAt[busName] = std::chrono::steady_clock::now();
-    addOrRefreshPlayer(busName);
     return true;
   } catch (const sdbus::Error& e) {
-    kLog.warn("seek failed name={} err={}", busName, e.what());
+    kLog.warn("seek dispatch failed name={} err={}", busName, e.what());
     return false;
   }
 }
@@ -771,16 +839,12 @@ bool MprisService::setPosition(const std::string& busName, int64_t positionUs) {
     return false;
   }
 
-  auto fallback_seek = [&]() {
-    int64_t currentPositionUs = it->second.positionUs;
-    try {
-      const sdbus::Variant positionValue =
-          proxyIt->second->getProperty("Position").onInterface(k_mpris_player_interface);
-      currentPositionUs = positionValue.get<int64_t>();
-    } catch (const sdbus::Error& e) {
-      kLog.warn("position refresh failed name={} err={}, using cached value", busName, e.what());
-    }
+  // Use projected position to reduce stale-cache drift for relative-seek fallback.
+  // Capture values by value, not iterator references.
+  const int64_t currentPositionUs = projectedPositionUs(it->second);
+  const bool preferRelativeSeek = it->second.trackId.empty() || busName.find("spotify") != std::string::npos;
 
+  auto fallback_seek = [this, busName, currentPositionUs, positionUs]() {
     const int64_t offsetUs = positionUs - currentPositionUs;
     if (offsetUs == 0) {
       return true;
@@ -788,7 +852,6 @@ bool MprisService::setPosition(const std::string& busName, int64_t positionUs) {
     return seek(busName, offsetUs);
   };
 
-  const bool preferRelativeSeek = it->second.trackId.empty() || busName.find("spotify") != std::string::npos;
   if (preferRelativeSeek) {
     // Some players don't expose track_id consistently; emulate absolute position with Seek.
     kLog.debug("mpris set-position using relative Seek fallback for {}", busName);
@@ -796,14 +859,14 @@ bool MprisService::setPosition(const std::string& busName, int64_t positionUs) {
   }
 
   try {
-    proxyIt->second->callMethod("SetPosition")
+    proxyIt->second->callMethodAsync("SetPosition")
         .onInterface(k_mpris_player_interface)
-        .withArguments(sdbus::ObjectPath{it->second.trackId}, positionUs);
+        .withArguments(sdbus::ObjectPath{it->second.trackId}, positionUs)
+        .uponReplyInvoke(makeAsyncReplyHandler("set-position", busName));
     m_lastSeekCommandAt[busName] = std::chrono::steady_clock::now();
-    addOrRefreshPlayer(busName);
     return true;
   } catch (const sdbus::Error& e) {
-    kLog.warn("set-position failed name={} err={}, falling back to Seek", busName, e.what());
+    kLog.warn("set-position dispatch failed name={} err={}, falling back to Seek", busName, e.what());
     return fallback_seek();
   }
 }
@@ -817,6 +880,10 @@ bool MprisService::setPositionActive(int64_t positionUs) {
 }
 
 bool MprisService::setVolume(const std::string& busName, double volume) {
+  if (!std::isfinite(volume) || volume < 0.0) {
+    return false;
+  }
+
   const auto it = m_players.find(busName);
   if (it == m_players.end()) {
     return false;
@@ -828,11 +895,13 @@ bool MprisService::setVolume(const std::string& busName, double volume) {
   }
 
   try {
-    proxyIt->second->setProperty("Volume").onInterface(k_mpris_player_interface).toValue(volume);
-    addOrRefreshPlayer(busName);
+    proxyIt->second->callMethodAsync("Set")
+        .onInterface(k_properties_interface)
+        .withArguments(std::string{k_mpris_player_interface}, std::string{"Volume"}, sdbus::Variant{volume})
+        .uponReplyInvoke(makeAsyncReplyHandler("set-volume", busName));
     return true;
   } catch (const sdbus::Error& e) {
-    kLog.warn("set-volume failed name={} err={}", busName, e.what());
+    kLog.warn("set-volume dispatch failed name={} err={}", busName, e.what());
     return false;
   }
 }
@@ -857,11 +926,13 @@ bool MprisService::setShuffle(const std::string& busName, bool shuffle) {
   }
 
   try {
-    proxyIt->second->setProperty("Shuffle").onInterface(k_mpris_player_interface).toValue(shuffle);
-    addOrRefreshPlayer(busName);
+    proxyIt->second->callMethodAsync("Set")
+        .onInterface(k_properties_interface)
+        .withArguments(std::string{k_mpris_player_interface}, std::string{"Shuffle"}, sdbus::Variant{shuffle})
+        .uponReplyInvoke(makeAsyncReplyHandler("set-shuffle", busName));
     return true;
   } catch (const sdbus::Error& e) {
-    kLog.warn("set-shuffle failed name={} err={}", busName, e.what());
+    kLog.warn("set-shuffle dispatch failed name={} err={}", busName, e.what());
     return false;
   }
 }
@@ -875,6 +946,10 @@ bool MprisService::setShuffleActive(bool shuffle) {
 }
 
 bool MprisService::setLoopStatus(const std::string& busName, std::string loopStatus) {
+  if (!is_valid_loop_status(loopStatus)) {
+    return false;
+  }
+
   const auto it = m_players.find(busName);
   if (it == m_players.end()) {
     return false;
@@ -886,11 +961,14 @@ bool MprisService::setLoopStatus(const std::string& busName, std::string loopSta
   }
 
   try {
-    proxyIt->second->setProperty("LoopStatus").onInterface(k_mpris_player_interface).toValue(std::move(loopStatus));
-    addOrRefreshPlayer(busName);
+    proxyIt->second->callMethodAsync("Set")
+        .onInterface(k_properties_interface)
+        .withArguments(std::string{k_mpris_player_interface}, std::string{"LoopStatus"},
+                       sdbus::Variant{std::move(loopStatus)})
+        .uponReplyInvoke(makeAsyncReplyHandler("set-loop-status", busName));
     return true;
   } catch (const sdbus::Error& e) {
-    kLog.warn("set-loop-status failed name={} err={}", busName, e.what());
+    kLog.warn("set-loop-status dispatch failed name={} err={}", busName, e.what());
     return false;
   }
 }
@@ -1304,23 +1382,56 @@ void MprisService::registerBusSignals() {
 }
 
 void MprisService::discoverPlayers() {
-  std::vector<std::string> names;
   try {
-    m_dbusProxy->callMethod("ListNames").onInterface(k_dbus_interface).storeResultsTo(names);
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+    m_dbusProxy->callMethodAsync("ListNames")
+        .onInterface(k_dbus_interface)
+        .uponReplyInvoke([this, aliveGuard](std::optional<sdbus::Error> err, std::vector<std::string> names) {
+          if (aliveGuard.expired()) {
+            return;
+          }
+          if (err.has_value()) {
+            kLog.warn("discover players failed err={}", err->what());
+            scheduleRecoveryDiscovery();
+            return;
+          }
+
+          m_pendingDiscoveryBusNames.clear();
+          for (const auto& name : names) {
+            if (is_mpris_bus_name(name)) {
+              m_pendingDiscoveryBusNames.push_back(name);
+            }
+          }
+          scheduleDiscoveryDrain();
+        });
   } catch (const sdbus::Error& e) {
     kLog.warn("discover players failed err={}", e.what());
     scheduleRecoveryDiscovery();
     return;
   }
+}
 
-  for (const auto& name : names) {
-    if (is_mpris_bus_name(name)) {
-      // kLog.debug("discover found mpris bus={}", name);
-      addOrRefreshPlayer(name);
-    }
+void MprisService::scheduleDiscoveryDrain() {
+  if (m_discoveryDrainScheduled || m_pendingDiscoveryBusNames.empty()) {
+    return;
   }
 
-  // kLog.debug("discover players listed={} cached_after={}", names.size(), m_players.size());
+  m_discoveryDrainScheduled = true;
+  const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+  DeferredCall::callLater([this, aliveGuard]() {
+    m_discoveryDrainScheduled = false;
+    if (aliveGuard.expired() || m_pendingDiscoveryBusNames.empty()) {
+      return;
+    }
+
+    const std::string busName = std::move(m_pendingDiscoveryBusNames.front());
+    m_pendingDiscoveryBusNames.pop_front();
+    addOrRefreshPlayer(busName);
+
+    if (!m_pendingDiscoveryBusNames.empty()) {
+      scheduleDiscoveryDrain();
+    }
+  });
 }
 
 void MprisService::scheduleStartupRediscovery() {
@@ -1328,7 +1439,11 @@ void MprisService::scheduleStartupRediscovery() {
     return;
   }
 
-  DeferredCall::callLater([this]() {
+  const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+  DeferredCall::callLater([this, aliveGuard]() {
+    if (aliveGuard.expired()) {
+      return;
+    }
     discoverPlayers();
     --m_startupRediscoveryPassesRemaining;
     if (m_startupRediscoveryPassesRemaining > 0) {
@@ -1343,15 +1458,17 @@ void MprisService::scheduleRecoveryDiscovery() {
   }
 
   m_recoveryDiscoveryScheduled = true;
-  DeferredCall::callLater([this]() {
+  const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+  DeferredCall::callLater([this, aliveGuard]() {
+    if (aliveGuard.expired()) {
+      return;
+    }
     m_recoveryDiscoveryScheduled = false;
     discoverPlayers();
   });
 }
 
 void MprisService::addOrRefreshPlayer(const std::string& busName) {
-  const auto previousActive = activePlayer();
-
   auto [proxyIt, inserted] = m_playerProxies.emplace(
       busName, sdbus::createProxy(m_bus.connection(), sdbus::ServiceName{busName}, k_mpris_path));
 
@@ -1446,19 +1563,76 @@ void MprisService::addOrRefreshPlayer(const std::string& busName) {
     });
   }
 
-  try {
   const bool hadPositionSignal = m_pendingPositionSignalRefresh[busName];
   m_pendingPositionSignalRefresh[busName] = false;
-    const MprisPlayerInfo info = readPlayerInfo(*proxyIt->second, busName);
-    const auto now = std::chrono::steady_clock::now();
-    // kLog.debug(
-    //     "queried player name={} identity=\"{}\" status=\"{}\" title=\"{}\" artist=\"{}\" track_id=\"{}\"
-    //     art_url=\"{}\"", info.busName, info.identity, info.playbackStatus, info.title, primary_artist(info.artists),
-    //     info.trackId, info.artUrl);
-    if (info.artUrl.empty()) {
-      const auto metadata = get_metadata_or(*proxyIt->second);
-      // kLog.debug("queried player missing art url name={} metadata_keys=[{}]", info.busName, joinKeys(metadata));
+
+  const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+  try {
+    proxyIt->second->callMethodAsync("GetAll")
+        .onInterface(k_properties_interface)
+        .withArguments(std::string{k_mpris_root_interface})
+        .uponReplyInvoke([this, aliveGuard, busName, hadPositionSignal](
+                             std::optional<sdbus::Error> rootErr, std::map<std::string, sdbus::Variant> rootProps) {
+          if (aliveGuard.expired()) {
+            return;
           }
+
+          auto proxyLookup = m_playerProxies.find(busName);
+          if (proxyLookup == m_playerProxies.end()) {
+            return;
+          }
+
+          try {
+            proxyLookup->second->callMethodAsync("GetAll")
+                .onInterface(k_properties_interface)
+                .withArguments(std::string{k_mpris_player_interface})
+                .uponReplyInvoke([this, aliveGuard, busName, hadPositionSignal, rootErr,
+                                  rootProps = std::move(rootProps)](std::optional<sdbus::Error> playerErr,
+                                                                    std::map<std::string, sdbus::Variant> playerProps) {
+                  if (aliveGuard.expired()) {
+                    return;
+                  }
+
+                  const bool rootFailed = rootErr.has_value();
+                  const bool playerFailed = playerErr.has_value();
+
+                  // If both interfaces failed for a player we've never seen before, we'd produce a phantom
+                  // entry with all-empty fields. Bail out and let recovery rediscover it instead.
+                  if (rootFailed && playerFailed && !m_players.contains(busName)) {
+                    kLog.warn("player hydration failed (both interfaces) name={}", busName);
+                    scheduleRecoveryDiscovery();
+                    return;
+                  }
+
+                  std::map<std::string, sdbus::Variant> effectiveRootProps;
+                  if (!rootFailed) {
+                    effectiveRootProps = rootProps;
+                  }
+
+                  std::map<std::string, sdbus::Variant> effectivePlayerProps;
+                  if (!playerFailed) {
+                    effectivePlayerProps = playerProps;
+                  }
+
+                  const MprisPlayerInfo info =
+                      readPlayerInfoFromProperties(busName, effectiveRootProps, effectivePlayerProps);
+                  applyPlayerSnapshot(busName, info, hadPositionSignal);
+                });
+          } catch (const sdbus::Error& e) {
+            kLog.warn("player query failed name={} err={}", busName, e.what());
+            scheduleRecoveryDiscovery();
+          }
+        });
+  } catch (const sdbus::Error& e) {
+    kLog.warn("player query failed name={} err={}", busName, e.what());
+    scheduleRecoveryDiscovery();
+  }
+}
+
+void MprisService::applyPlayerSnapshot(const std::string& busName, const MprisPlayerInfo& info,
+                                       bool hadPositionSignal) {
+  const auto previousActive = activePlayer();
+  const auto now = std::chrono::steady_clock::now();
   if (info.playbackStatus == "Playing") {
     m_lastActivePlayer = busName;
     m_lastPlayingUpdate[busName] = now;
@@ -1482,9 +1656,6 @@ void MprisService::addOrRefreshPlayer(const std::string& busName) {
     m_pendingPositionCandidateUs.erase(busName);
     m_pendingPositionCandidateMatches.erase(busName);
     m_pendingPositionCandidateAt.erase(busName);
-      // kLog.debug("added player name={} identity=\"{}\" status={} title=\"{}\" artist=\"{}\" art_url=\"{}\"",
-      //            info.busName, info.identity, info.playbackStatus, info.title, primary_artist(info.artists),
-      //            info.artUrl);
     m_players.emplace(busName, std::move(initial));
     emitPlayersChanged();
     syncSignals(previousActive);
@@ -1493,10 +1664,21 @@ void MprisService::addOrRefreshPlayer(const std::string& busName) {
     }
 
     if (info.playbackStatus != "Stopped" || info.positionUs > 0) {
-        DeferredCall::callLater([this, busName]() { refreshPlayerPosition(busName, true); });
+      const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+      DeferredCall::callLater([this, aliveGuard, busName]() {
+        if (aliveGuard.expired()) {
+          return;
+        }
+        refreshPlayerPosition(busName, true);
+      });
       auto& timerId = m_positionResyncTimers[busName];
-        timerId = TimerManager::instance().start(timerId, k_position_retry_interval,
-                                                 [this, busName]() { refreshPlayerPosition(busName, true); });
+      const std::weak_ptr<void> timerAliveGuard = m_aliveGuard;
+      timerId = TimerManager::instance().start(timerId, k_position_retry_interval, [this, timerAliveGuard, busName]() {
+        if (timerAliveGuard.expired()) {
+          return;
+        }
+        refreshPlayerPosition(busName, true);
+      });
     }
     return;
   }
@@ -1632,13 +1814,11 @@ void MprisService::addOrRefreshPlayer(const std::string& busName) {
     }
 
     existing->second = merged;
-      // kLog.debug("updated player name={} status={} title=\"{}\" artist=\"{}\" art_url=\"{}\"", merged.busName,
-      //            merged.playbackStatus, merged.title, primary_artist(merged.artists), merged.artUrl);
 
     const bool trackChanged = previous_info.title != merged.title || previous_info.album != merged.album ||
                               previous_info.artists != merged.artists || previous_info.artUrl != merged.artUrl ||
-                                previous_info.sourceUrl != merged.sourceUrl ||
-                                previous_info.trackId != merged.trackId || previous_info.lengthUs != merged.lengthUs;
+                              previous_info.sourceUrl != merged.sourceUrl || previous_info.trackId != merged.trackId ||
+                              previous_info.lengthUs != merged.lengthUs;
     const bool significantChanged =
         trackChanged || previous_info.identity != merged.identity ||
         previous_info.playbackStatus != merged.playbackStatus || previous_info.loopStatus != merged.loopStatus ||
@@ -1647,10 +1827,21 @@ void MprisService::addOrRefreshPlayer(const std::string& busName) {
         previous_info.canPause != merged.canPause || previous_info.canSeek != merged.canSeek;
 
     if (trackChanged || previous_info.playbackStatus != merged.playbackStatus) {
-        DeferredCall::callLater([this, busName]() { refreshPlayerPosition(busName, true); });
+      const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+      DeferredCall::callLater([this, aliveGuard, busName]() {
+        if (aliveGuard.expired()) {
+          return;
+        }
+        refreshPlayerPosition(busName, true);
+      });
       auto& timerId = m_positionResyncTimers[busName];
-        timerId = TimerManager::instance().start(timerId, k_position_retry_interval,
-                                                 [this, busName]() { refreshPlayerPosition(busName, true); });
+      const std::weak_ptr<void> timerAliveGuard = m_aliveGuard;
+      timerId = TimerManager::instance().start(timerId, k_position_retry_interval, [this, timerAliveGuard, busName]() {
+        if (timerAliveGuard.expired()) {
+          return;
+        }
+        refreshPlayerPosition(busName, true);
+      });
     }
 
     if (trackChanged) {
@@ -1662,10 +1853,6 @@ void MprisService::addOrRefreshPlayer(const std::string& busName) {
       m_changeCallback();
     }
   }
-  } catch (const sdbus::Error& e) {
-    kLog.warn("player query failed name={} err={}", busName, e.what());
-    scheduleRecoveryDiscovery();
-  }
 }
 
 void MprisService::removePlayer(const std::string& busName) {
@@ -1800,13 +1987,15 @@ bool MprisService::callPlayerMethod(const std::string& busName, const char* meth
     return false;
   }
 
+  const std::string method{methodName}; // Capture as owned string, not dangling pointer
+
   try {
-    it->second->callMethod(methodName).onInterface(k_mpris_player_interface);
-    addOrRefreshPlayer(busName);
-    kLog.debug("control name={} method={}", busName, methodName);
+    it->second->callMethodAsync(method.c_str())
+        .onInterface(k_mpris_player_interface)
+        .uponReplyInvoke(makeAsyncReplyHandler("control", busName, method));
     return true;
   } catch (const sdbus::Error& e) {
-    kLog.warn("control failed name={} method={} err={}", busName, methodName, e.what());
+    kLog.warn("control dispatch failed name={} method={} err={}", busName, method, e.what());
     return false;
   }
 }
@@ -2135,23 +2324,10 @@ std::tuple<bool, std::string, std::vector<std::string>> MprisService::onGetPlaye
   return {true, *m_pinnedPlayerPreference, m_preferredPlayers};
 }
 
-MprisPlayerInfo MprisService::readPlayerInfo(sdbus::IProxy& proxy, const std::string& busName) const {
-  std::map<std::string, sdbus::Variant> rootProps;
-  std::map<std::string, sdbus::Variant> playerProps;
-
-  try {
-    for (auto& [k, v] : proxy.getAllProperties().onInterface(k_mpris_root_interface)) {
-      rootProps.emplace(std::string(k), std::move(v));
-    }
-  } catch (const sdbus::Error&) {
-  }
-  try {
-    for (auto& [k, v] : proxy.getAllProperties().onInterface(k_mpris_player_interface)) {
-      playerProps.emplace(std::string(k), std::move(v));
-    }
-  } catch (const sdbus::Error&) {
-  }
-
+MprisPlayerInfo
+MprisService::readPlayerInfoFromProperties(const std::string& busName,
+                                           const std::map<std::string, sdbus::Variant>& rootProps,
+                                           const std::map<std::string, sdbus::Variant>& playerProps) const {
   auto metadata = get_variant_map_from_props(playerProps, "Metadata");
 
   return MprisPlayerInfo{

src/dbus/mpris/mpris_service.h

@@ -2,7 +2,9 @@
 
 #include <chrono>
 #include <cstdint>
+#include <deque>
 #include <functional>
+#include <map>
 #include <memory>
 #include <optional>
 #include <string>
@@ -11,8 +13,10 @@
 #include <vector>
 
 namespace sdbus {
+  class Error;
   class IObject;
   class IProxy;
+  class Variant;
 } // namespace sdbus
 
 class SessionBus;
@@ -98,16 +102,24 @@ private:
   void syncSignals(const std::optional<MprisPlayerInfo>& previousActive);
   void registerBusSignals();
   void discoverPlayers();
+  void scheduleDiscoveryDrain();
   void scheduleStartupRediscovery();
   void scheduleRecoveryDiscovery();
   void addOrRefreshPlayer(const std::string& busName);
+  void applyPlayerSnapshot(const std::string& busName, const MprisPlayerInfo& info, bool hadPositionSignal);
   void refreshPlayerPosition(const std::string& busName, bool notifyChange);
+  void applyPositionSample(const std::string& busName, int64_t rawPositionUs, bool notifyChange);
   void removePlayer(const std::string& busName);
-  [[nodiscard]] MprisPlayerInfo readPlayerInfo(sdbus::IProxy& proxy, const std::string& busName) const;
+  [[nodiscard]] MprisPlayerInfo
+  readPlayerInfoFromProperties(const std::string& busName, const std::map<std::string, sdbus::Variant>& rootProps,
+                               const std::map<std::string, sdbus::Variant>& playerProps) const;
   [[nodiscard]] MprisPlayerInfo projectedPlayerInfo(const MprisPlayerInfo& player) const;
   [[nodiscard]] std::int64_t projectedPositionUs(const MprisPlayerInfo& player) const;
   [[nodiscard]] std::optional<std::string> chooseActivePlayer() const;
   [[nodiscard]] bool isBlacklisted(const MprisPlayerInfo& player) const;
+  std::function<void(std::optional<sdbus::Error>)> makeAsyncReplyHandler(std::string op, std::string busName);
+  std::function<void(std::optional<sdbus::Error>)> makeAsyncReplyHandler(std::string op, std::string busName,
+                                                                         std::string_view method);
   [[nodiscard]] bool callPlayerMethod(const std::string& busName, const char* methodName);
   [[nodiscard]] bool canInvoke(const MprisPlayerInfo& player, const char* methodName) const;
 
@@ -141,6 +153,7 @@ private:
   [[nodiscard]] std::tuple<bool, std::string, std::vector<std::string>> onGetPlayerPreferences() const;
 
   SessionBus& m_bus;
+  std::shared_ptr<void> m_aliveGuard = std::make_shared<int>(0);
   std::unique_ptr<sdbus::IObject> m_controlObject;
   std::unique_ptr<sdbus::IProxy> m_dbusProxy;
   std::unordered_map<std::string, std::unique_ptr<sdbus::IProxy>> m_playerProxies;
@@ -161,6 +174,7 @@ private:
   std::unordered_map<std::string, std::chrono::steady_clock::time_point> m_lastPropertiesUpdate;
   std::unordered_map<std::string, std::chrono::steady_clock::time_point> m_lastPlayingUpdate;
   std::unordered_map<std::string, std::chrono::steady_clock::time_point> m_lastStrongMetadataUpdate;
+  std::deque<std::string> m_pendingDiscoveryBusNames;
   std::string m_lastActivePlayer;
   std::string m_lastEmittedActivePlayer;
   std::optional<std::string> m_pinnedPlayerPreference;
@@ -169,4 +183,5 @@ private:
   std::function<void()> m_changeCallback;
   int m_startupRediscoveryPassesRemaining = 4;
   bool m_recoveryDiscoveryScheduled = false;
+  bool m_discoveryDrainScheduled = false;
 };

src/shell/control_center/media_tab.cpp

@@ -1,5 +1,6 @@
 #include "shell/control_center/media_tab.h"
 
+#include "core/deferred_call.h"
 #include "core/log.h"
 #include "dbus/mpris/mpris_art.h"
 #include "dbus/mpris/mpris_service.h"
@@ -71,7 +72,7 @@ MediaTab::MediaTab(MprisService* mpris, HttpClient* httpClient, PipeWireSpectrum
     : m_mpris(mpris), m_httpClient(httpClient), m_spectrum(spectrum), m_wayland(wayland),
       m_renderContext(renderContext) {}
 
-MediaTab::~MediaTab() = default;
+MediaTab::~MediaTab() { m_aliveGuard.reset(); }
 
 void MediaTab::openPlayerMenu() {
   if (m_playerMenuPopup == nullptr || m_mpris == nullptr || m_playerMenuButton == nullptr) {
@@ -270,17 +271,19 @@ std::unique_ptr<Flex> MediaTab::create() {
     m_pendingSeekBusName = seekBusName;
     m_pendingSeekUs = targetUs;
     m_pendingSeekUntil = now + std::chrono::milliseconds(3000);
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
 
-    bool seekIssued = false;
+    DeferredCall::callLater([this, aliveGuard, seekBusName, targetUs]() {
+      if (aliveGuard.expired() || m_mpris == nullptr) {
+        return;
+      }
       if (!seekBusName.empty()) {
-      seekIssued = m_mpris->setPosition(seekBusName, targetUs);
+        (void)m_mpris->setPosition(seekBusName, targetUs);
       } else {
-      seekIssued = m_mpris->setPositionActive(targetUs);
-    }
-    if (!seekIssued) {
-      // Keep the thumb stable briefly even if transport seek dispatch races.
-      m_pendingSeekUntil = now + std::chrono::milliseconds(750);
+        (void)m_mpris->setPositionActive(targetUs);
       }
+      PanelManager::instance().refresh();
+    });
   });
   m_progressSlider = progress.get();
   mediaStack->addChild(std::move(progress));
@@ -305,14 +308,17 @@ std::unique_ptr<Flex> MediaTab::create() {
   repeat->setPadding(Style::spaceSm * scale, Style::spaceSm * scale);
   repeat->setRadius(Style::radiusLg * scale);
   repeat->setOnClick([this]() {
-    if (m_mpris == nullptr) {
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+    DeferredCall::callLater([this, aliveGuard]() {
+      if (aliveGuard.expired() || m_mpris == nullptr) {
         return;
       }
       const auto current = m_mpris->loopStatusActive().value_or("None");
       const std::string next = current == "None" ? "Playlist" : (current == "Playlist" ? "Track" : "None");
-    m_mpris->setLoopStatusActive(next);
+      (void)m_mpris->setLoopStatusActive(next);
       PanelManager::instance().refresh();
     });
+  });
   m_repeatButton = repeat.get();
   controls->addChild(std::move(repeat));
 
@@ -324,10 +330,14 @@ std::unique_ptr<Flex> MediaTab::create() {
   previous->setPadding(Style::spaceSm * scale, Style::spaceSm * scale);
   previous->setRadius(Style::radiusLg * scale);
   previous->setOnClick([this]() {
-    if (m_mpris != nullptr) {
-      m_mpris->previousActive();
-      PanelManager::instance().refresh();
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+    DeferredCall::callLater([this, aliveGuard]() {
+      if (aliveGuard.expired() || m_mpris == nullptr) {
+        return;
       }
+      (void)m_mpris->previousActive();
+      PanelManager::instance().refresh();
+    });
   });
   m_prevButton = previous.get();
   controls->addChild(std::move(previous));
@@ -340,10 +350,14 @@ std::unique_ptr<Flex> MediaTab::create() {
   playPause->setPadding(Style::spaceSm * scale, Style::spaceSm * scale);
   playPause->setRadius(Style::radiusLg * scale);
   playPause->setOnClick([this]() {
-    if (m_mpris != nullptr) {
-      m_mpris->playPauseActive();
-      PanelManager::instance().refresh();
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+    DeferredCall::callLater([this, aliveGuard]() {
+      if (aliveGuard.expired() || m_mpris == nullptr) {
+        return;
       }
+      (void)m_mpris->playPauseActive();
+      PanelManager::instance().refresh();
+    });
   });
   m_playPauseButton = playPause.get();
   controls->addChild(std::move(playPause));
@@ -356,10 +370,14 @@ std::unique_ptr<Flex> MediaTab::create() {
   next->setPadding(Style::spaceSm * scale, Style::spaceSm * scale);
   next->setRadius(Style::radiusLg * scale);
   next->setOnClick([this]() {
-    if (m_mpris != nullptr) {
-      m_mpris->nextActive();
-      PanelManager::instance().refresh();
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+    DeferredCall::callLater([this, aliveGuard]() {
+      if (aliveGuard.expired() || m_mpris == nullptr) {
+        return;
       }
+      (void)m_mpris->nextActive();
+      PanelManager::instance().refresh();
+    });
   });
   m_nextButton = next.get();
   controls->addChild(std::move(next));
@@ -372,11 +390,15 @@ std::unique_ptr<Flex> MediaTab::create() {
   shuffle->setPadding(Style::spaceSm * scale, Style::spaceSm * scale);
   shuffle->setRadius(Style::radiusLg * scale);
   shuffle->setOnClick([this]() {
-    if (m_mpris != nullptr) {
-      const bool enabled = m_mpris->shuffleActive().value_or(false);
-      m_mpris->setShuffleActive(!enabled);
-      PanelManager::instance().refresh();
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+    DeferredCall::callLater([this, aliveGuard]() {
+      if (aliveGuard.expired() || m_mpris == nullptr) {
+        return;
       }
+      const bool enabled = m_mpris->shuffleActive().value_or(false);
+      (void)m_mpris->setShuffleActive(!enabled);
+      PanelManager::instance().refresh();
+    });
   });
   m_shuffleButton = shuffle.get();
   controls->addChild(std::move(shuffle));
@@ -419,7 +441,9 @@ std::unique_ptr<Flex> MediaTab::create() {
   if (m_wayland != nullptr && m_renderContext != nullptr) {
     m_playerMenuPopup = std::make_unique<ContextMenuPopup>(*m_wayland, *m_renderContext);
     m_playerMenuPopup->setOnActivate([this](const ContextMenuControlEntry& entry) {
-      if (m_mpris == nullptr) {
+      const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+      DeferredCall::callLater([this, aliveGuard, entry]() {
+        if (aliveGuard.expired() || m_mpris == nullptr) {
           return;
         }
         if (entry.id == 0) {
@@ -430,6 +454,8 @@ std::unique_ptr<Flex> MediaTab::create() {
             m_mpris->setPinnedPlayerPreference(m_playerBusNames[idx]);
           }
         }
+        PanelManager::instance().refresh();
+      });
     });
   }
 
@@ -603,7 +629,15 @@ void MediaTab::setActive(bool active) {
     // Pull a fresh snapshot (including Position) when the tab opens so the
     // progress slider starts at the current playback position.
     m_positionSampleAt = {};
+    const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+    DeferredCall::callLater([this, aliveGuard]() {
+      if (aliveGuard.expired() || m_mpris == nullptr) {
+        return;
+      }
       m_mpris->refreshPlayers();
+      PanelManager::instance().requestUpdateOnly();
+      PanelManager::instance().requestRedraw();
+    });
     m_lastMprisRefreshAttempt = std::chrono::steady_clock::now();
   }
 }
@@ -676,11 +710,15 @@ void MediaTab::refresh(Renderer& renderer) {
     if (shouldRetryMpris) {
       m_lastMprisRefreshAttempt = now;
       kLog.debug("media tab retrying mpris discovery players={} active={}", players.size(), active.has_value());
+      const std::weak_ptr<void> aliveGuard = m_aliveGuard;
+      DeferredCall::callLater([this, aliveGuard]() {
+        if (aliveGuard.expired() || m_mpris == nullptr) {
+          return;
+        }
         m_mpris->refreshPlayers();
-      players = m_mpris->listPlayers();
-      active = m_mpris->activePlayer();
-      kLog.debug("media tab refresh after retry players={} active={} active_bus=\"{}\"", players.size(),
-                 active.has_value(), active.has_value() ? active->busName : std::string{});
+        PanelManager::instance().requestUpdateOnly();
+        PanelManager::instance().requestRedraw();
+      });
     }
   }
 

src/shell/control_center/media_tab.h

@@ -43,6 +43,11 @@ private:
 
   void openPlayerMenu();
 
+  // Guard token for deferred callbacks that run on the next main-loop tick.
+  // Callbacks capture a weak_ptr so they can detect destruction without
+  // relying on a raw this pointer staying valid.
+  std::shared_ptr<void> m_aliveGuard = std::make_shared<int>(0);
+
   MprisService* m_mpris = nullptr;
   HttpClient* m_httpClient = nullptr;
   PipeWireSpectrum* m_spectrum = nullptr;