feat(media-controls): SMTC with a real HWND, and a native cover path
Windows media keys never worked because SMTC never started. souvlaki's Windows
backend does `config.hwnd.expect(..)`, which PANICS rather than degrading, and
`MediaControls::new` runs inside a tokio task, so a missing handle aborted the
task instead of returning the Err the caller was written for. Phase A bowed out
before reaching it and logged "SMTC disabled: no window handle wired yet".
`spawn` now takes a `NativeWindow`, and `cxx/win_shell.cpp` supplies the
top-level HWND through `QGuiApplication::topLevelWindows()`. The guard stays:
a missing integration is a missing feature, a panic in a tokio task is a dead
session.
Windows reads the handle from a bounded retry on the GUI thread. Both things it
waits for -- the tray bridge's Qt thread and a shown window -- are "not yet",
never "not ever": `tray_bridge::ui` is a no-op until `QbzTray::boot` registers
the thread, and `QbzSession.boot()` runs first (Main.qml:254 vs :380).
NON-WINDOWS IS UNCHANGED, deliberately. An earlier draft routed every platform
through that hop and it was a Linux/macOS regression three ways: initialization
could be dropped outright before boot, MPRIS would start only when the Qt queue
drained (losing the first metadata push), and `main_window_hwnd` calls
`QWindow::winId()`, which CREATES the platform window as a side effect on
platforms with no reason to.
Cover art needed its own fix. souvlaki 0.8.3 branches on
`url.starts_with("file://")`, then trims exactly that prefix and hands the rest
to `GetFileFromPathAsync`. So `file:///C:/x.jpg` arrives as `/C:/x.jpg` -- a
slash before a drive letter, which never opens -- while a bare `C:\x.jpg` no
longer matches the prefix and takes the remote-URI branch instead. The value
that works is the hybrid `file://C:\x.jpg`. UNC is handled too: `file://nas/m/a`
is a real Windows path and was being passed through as the relative `nas/m/a`.
The percent-decode is full and applied once: `art_url` arrives from
`url::Url::from_file_path` (space as %20) and from `fs_url::file_url` (only
% # ? escaped), and one full decode reads both -- `%2520` gives back a literal
`%20` rather than a space. `fs_url::local_path` is deliberately not used here;
its three-escape decode would leave the other producer's %20 intact.
Eight tests cover the matrix. Reviewed adversarially before commit, which is
where the inverted cover conversion and the non-Windows regression were caught;
the souvlaki branch was then read at the source to confirm.
cc
This commit is contained in:
10 files changed
+397
-42
No files matched your search
@@ -8,6 +8,10 @@ description = "Frontend-agnostic system media controls: MPRIS (mpris-server) on
|
||||
|
||||
[dependencies]
|
||||
log = { workspace = true }
|
||||
# fs_url::local_path -- the ONE file-URL helper (W2). SMTC needs a native
|
||||
# Windows path where MPRIS needs a file:// URL, and hand-rolling that
|
||||
# conversion is exactly the bug class that helper exists to prevent.
|
||||
qbz-models = { path = "../qbz-models" }
|
||||
# Notifications (all platforms): the async entry point spawns the HTTP download
|
||||
# + image resize on `spawn_blocking`, so a tokio runtime must be present.
|
||||
tokio = { workspace = true }
|
||||
|
||||
@@ -33,22 +33,42 @@ mod platform;
|
||||
/// **macOS:** souvlaki's command callbacks fire on the app run loop, so this is
|
||||
/// safe to call from any thread, but the run loop (Slint's winit loop) must be
|
||||
/// running for inbound events to arrive.
|
||||
/// The platform window identity the OS integration needs.
|
||||
///
|
||||
/// Windows SMTC is registered against a window: souvlaki's backend calls
|
||||
/// `config.hwnd.expect(..)`, so a missing handle is a PANIC, not a graceful
|
||||
/// degrade. Linux and macOS key off the D-Bus name and the app bundle
|
||||
/// respectively and ignore this entirely, which is why `default()` is the
|
||||
/// right thing to pass there and from headless callers.
|
||||
#[derive(Clone, Copy, Debug, Default)]
|
||||
pub struct NativeWindow {
|
||||
pub hwnd: Option<*mut core::ffi::c_void>,
|
||||
}
|
||||
|
||||
// SAFETY: an HWND is an opaque process-wide identifier, not a pointer that is
|
||||
// dereferenced here. It is read once by souvlaki on the thread that receives
|
||||
// it. Windows itself allows a window handle to be named from any thread; what
|
||||
// is thread-affine is the window's message loop, which this never touches.
|
||||
unsafe impl Send for NativeWindow {}
|
||||
|
||||
pub fn spawn(
|
||||
on_event: impl Fn(MediaEvent) + Send + Sync + 'static,
|
||||
native: NativeWindow,
|
||||
) -> Option<Box<dyn MediaIntegration>> {
|
||||
let cb: Arc<dyn Fn(MediaEvent) + Send + Sync> = Arc::new(on_event);
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
let _ = native;
|
||||
return linux::spawn(cb).map(|h| Box::new(h) as Box<dyn MediaIntegration>);
|
||||
}
|
||||
#[cfg(any(target_os = "macos", target_os = "windows"))]
|
||||
{
|
||||
return platform::spawn(cb).map(|h| Box::new(h) as Box<dyn MediaIntegration>);
|
||||
return platform::spawn(cb, native).map(|h| Box::new(h) as Box<dyn MediaIntegration>);
|
||||
}
|
||||
#[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))]
|
||||
{
|
||||
let _ = cb;
|
||||
let _ = (cb, native);
|
||||
None
|
||||
}
|
||||
}
|
||||
@@ -9,16 +9,12 @@ use std::sync::{Arc, Mutex};
|
||||
use std::time::Duration;
|
||||
|
||||
use souvlaki::{MediaControls, MediaMetadata, MediaPlayback, MediaPosition};
|
||||
// Only `spawn_native` and `map_event` touch these, and both are compiled out
|
||||
// on Windows (souvlaki's SMTC backend panics without an HWND — see `spawn`).
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
use souvlaki::{MediaControlEvent, PlatformConfig, SeekDirection};
|
||||
|
||||
use crate::types::{MediaEvent, MediaIntegration, PlaybackStatus, TrackMeta};
|
||||
use crate::NativeWindow;
|
||||
|
||||
/// Default step for a magnitude-less MPRIS-style `Seek` (5 seconds, micros).
|
||||
// Only `map_event` reads it, and that is compiled out on Windows.
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
const SEEK_STEP_MICROS: i64 = 5_000_000;
|
||||
|
||||
type EventCb = Arc<dyn Fn(MediaEvent) + Send + Sync>;
|
||||
@@ -31,12 +27,19 @@ impl MediaIntegration for PlatformHandle {
|
||||
fn set_metadata(&self, meta: &TrackMeta) {
|
||||
if let Ok(mut guard) = self.controls.lock() {
|
||||
if let Some(c) = guard.as_mut() {
|
||||
// SMTC wants a NATIVE path where MPRIS wants a URL; see
|
||||
// `cover_for_souvlaki`. Bound to a local so the &str borrowed
|
||||
// into MediaMetadata outlives the call.
|
||||
let cover = meta
|
||||
.art_url
|
||||
.as_deref()
|
||||
.map(|u| cover_for_souvlaki(u, cfg!(target_os = "windows")));
|
||||
let md = MediaMetadata {
|
||||
title: Some(meta.title.as_str()),
|
||||
artist: Some(meta.artist.as_str()),
|
||||
album: Some(meta.album.as_str()),
|
||||
duration: meta.duration,
|
||||
cover_url: meta.art_url.as_deref(),
|
||||
cover_url: cover.as_deref(),
|
||||
};
|
||||
let _ = c.set_metadata(md);
|
||||
}
|
||||
@@ -63,8 +66,6 @@ impl MediaIntegration for PlatformHandle {
|
||||
}
|
||||
}
|
||||
|
||||
// Only `spawn_native` installs it, and that is compiled out on Windows.
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
fn map_event(e: MediaControlEvent) -> Option<MediaEvent> {
|
||||
Some(match e {
|
||||
MediaControlEvent::Play => MediaEvent::Play,
|
||||
@@ -90,34 +91,96 @@ fn map_event(e: MediaControlEvent) -> Option<MediaEvent> {
|
||||
})
|
||||
}
|
||||
|
||||
pub fn spawn(on_event: EventCb) -> Option<PlatformHandle> {
|
||||
// souvlaki's Windows backend does NOT degrade gracefully without an HWND:
|
||||
// `config.hwnd.expect(...)` at souvlaki-0.8.3/src/platform/windows/mod.rs:57-59
|
||||
// PANICS, and `MediaControls::new` runs inside a tokio task, so the first
|
||||
// session would abort the task rather than return the Err the match below
|
||||
// is written for. (The old comment on `hwnd: None` claimed the opposite;
|
||||
// it was read from the comment, not the mechanism.) No HWND is wired yet
|
||||
// — that is plan A-2's "SMTC real" task — so bow out here on Windows.
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
let _ = on_event;
|
||||
log::warn!("[media-controls] SMTC disabled: no window handle wired yet (Phase A)");
|
||||
return None;
|
||||
}
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
{
|
||||
return spawn_native(on_event);
|
||||
/// Convert a cover-art URL into the exact string souvlaki's Windows arm can
|
||||
/// open.
|
||||
///
|
||||
/// souvlaki 0.8.3 (`src/platform/windows/mod.rs:192-203`) branches like this:
|
||||
///
|
||||
/// ```text
|
||||
/// if url.starts_with("file://") {
|
||||
/// let path = url.trim_start_matches("file://");
|
||||
/// StorageFile::GetFileFromPathAsync(path) // wants C:\x or \\nas\x
|
||||
/// } else {
|
||||
/// RandomAccessStreamReference::CreateFromUri(url)
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// So `file://` is the DISCRIMINATOR and must survive, while everything after
|
||||
/// it has to be a native Windows path. Two ways to get this wrong, and the
|
||||
/// first draft of this function managed one of them:
|
||||
///
|
||||
/// - Hand over `file:///C:/x.jpg` and souvlaki trims seven characters, leaving
|
||||
/// `/C:/x.jpg` -- a leading slash before a drive letter, which never opens.
|
||||
/// - Hand over a bare `C:\x.jpg` and it no longer starts with `file://`, so it
|
||||
/// takes the REMOTE branch and `Uri::CreateUri` fails on it.
|
||||
///
|
||||
/// The answer is the hybrid `file://C:\x.jpg`: prefix kept, remainder native.
|
||||
///
|
||||
/// UNC is a real Windows path too. `file://nas/music/a.jpg` names
|
||||
/// `\\nas\music\a.jpg`, and leaving it alone would hand souvlaki the
|
||||
/// relative `nas/music/a.jpg`.
|
||||
///
|
||||
/// The percent-decode is FULL and applied ONCE. `art_url` reaches this from
|
||||
/// two producers -- `url::Url::from_file_path`, which escapes a space as
|
||||
/// `%20`, and `fs_url::file_url`, which escapes only `%`, `#` and `?` -- and a
|
||||
/// single full decode is correct for both. `fs_url::local_path` is NOT used
|
||||
/// here for that reason: its deliberate three-escape decode would leave a
|
||||
/// `%20` from the other producer intact.
|
||||
pub(crate) fn cover_for_souvlaki(url: &str, windows: bool) -> String {
|
||||
if !windows {
|
||||
return url.to_string();
|
||||
}
|
||||
let Some(rest) = url.strip_prefix("file://") else {
|
||||
// http(s) and anything else: souvlaki's Uri branch is the right one.
|
||||
return url.to_string();
|
||||
};
|
||||
|
||||
let decoded = urlencoding::decode(rest)
|
||||
.map(|c| c.into_owned())
|
||||
.unwrap_or_else(|_| rest.to_string());
|
||||
let b = decoded.as_bytes();
|
||||
|
||||
let native = if b.len() >= 3 && b[0] == b'/' && b[1].is_ascii_alphabetic() && b[2] == b':' {
|
||||
// `file:///C:/x` -> the empty authority's slash sits in front of the
|
||||
// drive letter; drop it.
|
||||
decoded[1..].to_string()
|
||||
} else if !decoded.starts_with('/') && !decoded.is_empty() {
|
||||
// A non-empty authority is a UNC server: `file://nas/share/x`.
|
||||
format!("//{decoded}")
|
||||
} else {
|
||||
// A POSIX absolute path. Not a Windows path at all, and nothing here
|
||||
// can make it one -- hand it back untouched rather than inventing
|
||||
// `\home\...`.
|
||||
return url.to_string();
|
||||
};
|
||||
|
||||
format!("file://{}", native.replace('/', "\\"))
|
||||
}
|
||||
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
fn spawn_native(on_event: EventCb) -> Option<PlatformHandle> {
|
||||
pub fn spawn(on_event: EventCb, native: NativeWindow) -> Option<PlatformHandle> {
|
||||
// souvlaki's Windows backend does NOT degrade gracefully without an HWND:
|
||||
// `config.hwnd.expect(...)` at souvlaki-0.8.3/src/platform/windows/mod.rs:57-59
|
||||
// PANICS. The guard therefore stays even now that a handle IS wired:
|
||||
// a missing integration is a missing feature, a panic inside a tokio task
|
||||
// is a dead session. The caller reads the HWND on the GUI thread after the
|
||||
// window is shown, so None here means it asked too early.
|
||||
#[cfg(target_os = "windows")]
|
||||
if native.hwnd.is_none() {
|
||||
log::warn!(
|
||||
"[media-controls] SMTC disabled: no top-level window yet (init ran before it was shown)"
|
||||
);
|
||||
return None;
|
||||
}
|
||||
|
||||
spawn_native(on_event, native)
|
||||
}
|
||||
|
||||
fn spawn_native(on_event: EventCb, native: NativeWindow) -> Option<PlatformHandle> {
|
||||
let config = PlatformConfig {
|
||||
dbus_name: "com.blitzfc.qbz",
|
||||
display_name: "QBZ",
|
||||
// macOS: unused. Windows: unreachable — see the early return in
|
||||
// `spawn` above.
|
||||
hwnd: None,
|
||||
// macOS ignores it; Windows registers SMTC against this window.
|
||||
hwnd: native.hwnd,
|
||||
};
|
||||
|
||||
let mut controls = match MediaControls::new(config) {
|
||||
@@ -142,3 +205,90 @@ fn spawn_native(on_event: EventCb) -> Option<PlatformHandle> {
|
||||
controls: Arc::new(Mutex::new(Some(controls))),
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::cover_for_souvlaki;
|
||||
|
||||
#[test]
|
||||
fn windows_keeps_the_file_prefix_and_makes_the_rest_native() {
|
||||
// The prefix is souvlaki's discriminator: drop it and the string takes
|
||||
// the remote-URI branch. Keep it, and what it trims must be a real
|
||||
// Windows path.
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///C:/Users/v/c.jpg", true),
|
||||
"file://C:\\Users\\v\\c.jpg"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_windows_is_byte_identical() {
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///home/v/c.jpg", false),
|
||||
"file:///home/v/c.jpg"
|
||||
);
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///C:/Users/v/c.jpg", false),
|
||||
"file:///C:/Users/v/c.jpg"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remote_urls_are_never_touched() {
|
||||
assert_eq!(cover_for_souvlaki("https://x/y.jpg", true), "https://x/y.jpg");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_unc_url_becomes_a_unc_path() {
|
||||
// `file://nas/music/a.jpg` is a real Windows path. Left alone, souvlaki
|
||||
// would trim it to the RELATIVE `nas/music/a.jpg`.
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file://nas/music/a.jpg", true),
|
||||
"file://\\\\nas\\music\\a.jpg"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_posix_path_is_left_alone_even_on_windows() {
|
||||
// No drive, no authority: nothing here can turn it into a Windows path.
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///home/v/c.jpg", true),
|
||||
"file:///home/v/c.jpg"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn both_producers_percent_decode_the_same_way() {
|
||||
// fs_url::file_url leaves a space LITERAL...
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///C:/My Music/a b.jpg", true),
|
||||
"file://C:\\My Music\\a b.jpg"
|
||||
);
|
||||
// ...while url::Url::from_file_path escapes it as %20. One full decode
|
||||
// is right for both.
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///C:/My%20Music/a.jpg", true),
|
||||
"file://C:\\My Music\\a.jpg"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_escapes_that_matter_are_undone() {
|
||||
// A # in a filename reaches SMTC as %23 otherwise, and the cover
|
||||
// silently never loads.
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///C:/m/a%23b%3Fc.jpg", true),
|
||||
"file://C:\\m\\a#b?c.jpg"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_literal_percent_twenty_is_not_double_decoded() {
|
||||
// A filename that really contains "%20" is escaped to %2520 by either
|
||||
// producer; ONE decode must give back "%20", not a space.
|
||||
assert_eq!(
|
||||
cover_for_souvlaki("file:///C:/m/a%2520b.jpg", true),
|
||||
"file://C:\\m\\a%20b.jpg"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user