From f778874fdd0059058a7e8fa9916aa242bf5f3273 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:29:58 -0600 Subject: [PATCH] fix(audio): #508 ALSA-exclusive + streaming regression; flatpak ReserveDevice1 own-name Store the CMAF duration at command-accept so the seek bar and progress can't freeze at 0:00, backfill it on the Resume rebuild, and clear + surface the zombie stream-create-failure path. Degrade every non-priority device-reservation contention error (only a higher-priority holder stays fatal) so a sandboxed D-Bus RequestName denial can't block bit-perfect playback. Honor the stream_buffer_seconds floor in the dynamic CMAF path and wire the network throttle (record_segment_bandwidth / record_underrun / prefetch cap) to tame the #591 underrun loop. Grant --own-name=org.freedesktop.ReserveDevice1.* in the repo flatpak manifest so WirePlumber releases the card cleanly. cc --- .../qbz-audio/src/device_reservation/linux.rs | 45 ++++++--- crates/qbz-player/src/player/mod.rs | 94 +++++++++++++++++-- .../qbz-player/src/player/streaming_source.rs | 16 ++++ crates/qbz/src/playback.rs | 21 +++++ packaging/flatpak/com.blitzfc.qbz.yml | 6 ++ 5 files changed, 163 insertions(+), 19 deletions(-) diff --git a/crates/qbz-audio/src/device_reservation/linux.rs b/crates/qbz-audio/src/device_reservation/linux.rs index d2e2cb448..58f2a1b57 100644 --- a/crates/qbz-audio/src/device_reservation/linux.rs +++ b/crates/qbz-audio/src/device_reservation/linux.rs @@ -16,7 +16,10 @@ //! property; if our priority is higher, call `RequestRelease(our_priority)` //! on the holder. If that returns `true`, retry `RequestName` with //! `DO_NOT_QUEUE | REPLACE_EXISTING`. -//! 4. Anything else (zbus error, refusal, equal-or-lower priority) -> err. +//! 4. Refusal or equal-or-greater priority -> `HigherPriorityHolder` (the +//! only fatal outcome). Any other failure (bus unavailable, name denied, +//! holder unreachable, lost re-acquire race) -> degraded no-op guard; +//! the PCM open + EBUSY backoff ladder arbitrate the device (#508/#534). //! //! On `Drop`, an active guard releases the bus name. A *degraded* guard //! (returned when the session bus is unavailable) is a no-op on `Drop`. @@ -74,15 +77,15 @@ impl DeviceReservation { /// /// Returns: /// - `Ok(active_guard)` once we own the bus name. - /// - `Ok(degraded_guard)` if the session bus is unreachable. The caller - /// should treat playback as "no reservation, but proceed normally". - /// - `Err(InvalidDevice)` for unparseable device strings. - /// - `Err(HigherPriorityHolder { .. })` if another app refuses to - /// release, or holds at equal-or-greater priority. - /// - `Err(DbusError(_))` for protocol-level zbus failures we can't - /// downgrade (e.g. malformed bus name, reply marshaling failure). - /// - `Err(AlsaError(_))` for ALSA enumeration failures while resolving - /// symbolic card names like `hw:CARD=DacMagic`. + /// - `Ok(degraded_guard)` for every non-cooperative failure: unparseable + /// device strings, an unreachable session bus, a denied `RequestName` + /// (confined sandbox bus, #534), or contention resolution failing for + /// any reason other than the holder refusing. The caller treats these + /// as "no reservation, but proceed normally" — the PCM open and its + /// EBUSY backoff ladder arbitrate the device (#508). + /// - `Err(HigherPriorityHolder { .. })` — the only fatal outcome: a + /// cooperative holder refused to release, or holds at + /// equal-or-greater priority. /// /// # Tight coupling rule (load-bearing — do not violate) /// @@ -198,7 +201,27 @@ impl DeviceReservation { // RequestNameReply is exhaustively matched and the contention // logic handles it identically to Exists. RequestNameReply::Exists | RequestNameReply::InQueue => { - resolve_contention(&connection, &bus_name, &object_path, app_device_name) + match resolve_contention(&connection, &bus_name, &object_path, app_device_name) { + Ok(res) => Ok(res), + // A cooperative higher-priority holder (DAW / pro-audio app) + // refused to release: honor it — the one deliberate fatal case. + Err(e @ ReservationError::HigherPriorityHolder { .. }) => Err(e), + // Anything else (holder proxy unreachable, RequestRelease call + // failed, lost re-acquire race): degrade — the PCM open plus its + // EBUSY backoff ladder is the real arbiter, exactly like the + // request_name-failure path above (#508/#534). + Err(e) => { + log::warn!( + "[reservation] contention resolution for {} failed: {}. \ + Proceeding without D-Bus reservation (degraded).", + bus_name, + e + ); + Ok(Self { + state: ReservationState::Degraded, + }) + } + } } } } diff --git a/crates/qbz-player/src/player/mod.rs b/crates/qbz-player/src/player/mod.rs index 52c10b34c..8e5853da5 100644 --- a/crates/qbz-player/src/player/mod.rs +++ b/crates/qbz-player/src/player/mod.rs @@ -2150,6 +2150,14 @@ impl Player { *current_audio_data = None; // Clear regular audio data thread_state.set_loaded_audio(true); + // Track metadata is known-good at command time. Store the + // duration NOW — current_position() clamps to it, so leaving + // it 0 until after the buffer wait freezes the bar at 0:00, + // and a Resume that recovers this source after a failed setup + // would pin the bar at 0:00 forever (#508 follow-up, 2.0.1 + // Flatpak report 2026-07-15). + thread_state.duration.store(duration_secs, Ordering::SeqCst); + let StreamRecreateDecision { needs_new_stream, format_changed, @@ -2297,7 +2305,17 @@ impl Player { log::error!( "No audio output device available for streaming" ); - thread_state.set_stream_error(true); + // Same cleanup as the failure arms below: the + // streaming source was already stored at + // command-accept, and a Resume that finds it + // would replay a track that never started. + *current_streaming_source = None; + *current_audio_data = None; + thread_state.set_loaded_audio(false); + thread_state.is_playing.store(false, Ordering::SeqCst); + thread_state.record_stream_error( + "No audio output device available for streaming", + ); return; }; create_output_stream_with_config( @@ -2325,7 +2343,18 @@ impl Player { sample_rate, e ); - thread_state.set_stream_error(true); + // Backend AND legacy CPAL both failed — a real + // error the user must see. Clear the zombie + // streaming state (same cleanup as the sibling + // failure arms below); leaving it set lets a + // later Resume replay the completed download + // with duration still 0, pinning the bar at + // 0:00 forever (#508/#592). + *current_streaming_source = None; + *current_audio_data = None; + thread_state.set_loaded_audio(false); + thread_state.is_playing.store(false, Ordering::SeqCst); + thread_state.record_stream_error(e); return; } } @@ -3092,6 +3121,21 @@ impl Player { } }; + // A Resume that rebuilds from completed streaming data + // can run after a PlayStreaming that failed BEFORE + // storing the duration. Backfill from the decoded + // source so the position clamp (current_position) + // doesn't pin the bar at 0:00 (#508). Same derivation + // the Play handler uses. Must read total_duration() + // BEFORE skip_duration consumes `source` below. + if thread_state.duration.load(Ordering::SeqCst) == 0 { + if let Some(d) = source.total_duration() { + thread_state + .duration + .store(d.as_secs(), Ordering::SeqCst); + } + } + let resume_pos = thread_state.position.load(Ordering::SeqCst); let skipped_source: Box + Send> = if resume_pos > 0 { @@ -4590,16 +4634,21 @@ impl Player { // Progress logging every 5 segments or on the last segment. if seg_idx % 5 == 0 || seg_idx == n_segments { let elapsed = start.elapsed().as_secs_f64(); + let mbps = if elapsed > 0.0 { + total_written as f64 / (1024.0 * 1024.0) / elapsed + } else { + 0.0 + }; + // Feed the adaptive prefetch throttle with the cumulative MB/s + // the log already reports; also the offline detector's + // positive liveness signal (#467, #591). + qbz_audio::network_throttle::state().record_segment_bandwidth(mbps); log::info!( "[CMAF-STREAM] Segment {}/{} ({:.1} MB, {:.1} MB/s)", seg_idx, n_segments - 1, total_written as f64 / (1024.0 * 1024.0), - if elapsed > 0.0 { - total_written as f64 / (1024.0 * 1024.0) / elapsed - } else { - 0.0 - } + mbps ); } } @@ -5028,7 +5077,36 @@ impl Player { self.state.set_stream_quality(sample_rate, bit_depth); // Use StreamingConfig::from_speed_mbps for dynamic buffer sizing - let config = StreamingConfig::from_speed_mbps(speed_mbps); + let mut config = StreamingConfig::from_speed_mbps(speed_mbps); + + // Floor the initial buffer at the user's `stream_buffer_seconds` worth + // of REAL audio bytes (#591). content_length / duration is the track's + // true average byterate (both derive from the CMAF segment table), + // unlike `speed_mbps`, which is estimated from the tiny init fetch and + // is latency-dominated — it lands on the slowest ladder rung for every + // connection. Clamped to 256KB (format-detection minimum) .. 8MB (the + // process-wide ladder cap has no desktop caller, so this is the + // effective ceiling protecting low-memory hosts). + let user_secs = self + .audio_settings + .lock() + .map(|s| s.stream_buffer_seconds) + .unwrap_or(2); + let bps = content_length / duration_secs.max(1); + let floor = (user_secs as u64).saturating_mul(bps) as usize; + let ladder_bytes = config.initial_buffer_bytes; + config.initial_buffer_bytes = ladder_bytes + .max(floor) + .clamp(256 * 1024, 8 * 1024 * 1024); + if config.initial_buffer_bytes != ladder_bytes { + log::info!( + "Dynamic buffer: user floor {}s x {} B/s → {}KB initial buffer (ladder gave {}KB)", + user_secs, + bps, + config.initial_buffer_bytes / 1024, + ladder_bytes / 1024 + ); + } let (source, writer) = BufferedMediaSource::new(config, Some(content_length)); let source = Arc::new(source); diff --git a/crates/qbz-player/src/player/streaming_source.rs b/crates/qbz-player/src/player/streaming_source.rs index 5d763d809..74d20a14d 100644 --- a/crates/qbz-player/src/player/streaming_source.rs +++ b/crates/qbz-player/src/player/streaming_source.rs @@ -580,6 +580,11 @@ pub struct IncrementalStreamingSource { finished: bool, /// Number of packets decoded (for stats) packets_decoded: u64, + /// True while inside a WouldBlock stall episode (playback caught up + /// with the download). Set on the first WouldBlock after at least one + /// decoded packet, cleared on the next successful decode — so each + /// episode records exactly one underrun with the network throttle. + stalled: bool, /// Reference to the buffered source (for cache retrieval after playback) buffered_source: Arc, } @@ -643,6 +648,7 @@ impl IncrementalStreamingSource { track_id, finished: false, packets_decoded: 0, + stalled: false, buffered_source, }) } @@ -708,6 +714,13 @@ impl IncrementalStreamingSource { { // Not enough data buffered yet - wait briefly and retry // This happens when playback catches up with download + if !self.stalled && self.packets_decoded > 0 { + // Mid-playback stall, not initial buffering (≥1 packet + // already decoded): put the prefetch throttle in panic + // mode so the live stream gets the pipe to itself (#591). + self.stalled = true; + qbz_audio::network_throttle::state().record_underrun(); + } std::thread::sleep(Duration::from_millis(5)); continue; } @@ -741,6 +754,9 @@ impl IncrementalStreamingSource { self.sample_queue .extend(sample_buf.samples().iter().copied()); self.packets_decoded += 1; + // Successful decode ends any stall episode; the next + // WouldBlock streak records a fresh underrun. + self.stalled = false; } Err(SymphoniaError::DecodeError(e)) => { log::warn!("Decode error (skipping packet): {}", e); diff --git a/crates/qbz/src/playback.rs b/crates/qbz/src/playback.rs index 1dabcad9a..cdcfd933a 100644 --- a/crates/qbz/src/playback.rs +++ b/crates/qbz/src/playback.rs @@ -125,6 +125,27 @@ async fn kick_prefetch(runtime: &Runtime) { if crate::offline_mode::engine().is_offline() { return; } + // Adaptive throttle (#591): when the live stream is starving — panic mode + // after a decoder underrun, or bandwidth headroom below the surviving + // ratio — prefetch must get out of the pipe entirely. Cap 0 means "no + // prefetch right now"; the semaphore still bounds concurrency otherwise. + { + use qbz_audio::network_throttle::{self, PlaybackQualityTag}; + let tag = match playback_quality() { + Quality::UltraHiRes => PlaybackQualityTag::UltraHiRes, + Quality::HiRes => PlaybackQualityTag::HiRes, + Quality::Lossless => PlaybackQualityTag::Lossless, + Quality::Mp3 => PlaybackQualityTag::Lossy, + }; + let cap = network_throttle::state().current_prefetch_cap( + network_throttle::playback_mbps_for_quality(tag), + MAX_CONCURRENT_PREFETCH, + ); + if cap == 0 { + log::debug!("[qbz-slint] prefetch: skipped (network throttle cap 0)"); + return; + } + } let upcoming = runtime.core().peek_upcoming(PREFETCH_LOOKAHEAD).await; if upcoming.is_empty() { return; diff --git a/packaging/flatpak/com.blitzfc.qbz.yml b/packaging/flatpak/com.blitzfc.qbz.yml index 694a4f191..75e1f3753 100644 --- a/packaging/flatpak/com.blitzfc.qbz.yml +++ b/packaging/flatpak/com.blitzfc.qbz.yml @@ -33,6 +33,12 @@ finish-args: # includes --device=dri (GPU), so the separate dri entry is dropped. - --device=all + # org.freedesktop.ReserveDevice1 device-reservation names: lets QBZ ask + # WirePlumber/PipeWire to release the sound card cleanly before opening + # it exclusively (freedesktop audio-device reservation protocol). Owning + # a name implies talking to its current holder (same well-known name). #508 + - --own-name=org.freedesktop.ReserveDevice1.* + # Native PipeWire access - --filesystem=xdg-run/pipewire-0:ro