Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions backend/src/gst/pipeline/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ impl PipelineManager {
/// Set a property on an element.
///
/// `ramp_ms` is consulted only for routes that support smooth interpolation
/// (currently audio `volume`-element `volume`/`mute`). Other properties are
/// set immediately regardless. `None` selects the per-route default ramp.
/// (currently audio `volume`-element `volume` and `mute`). Other properties
/// are set immediately regardless. `None` selects the per-route default
/// ramp (short anti-zipper for `volume`, short anti-click for `mute`); a
/// caller can request a longer broadcast-style fade by passing an explicit
/// duration (e.g. 500 ms for a route mute on-air/off-air).
pub(super) fn set_property(
&self,
element: &gst::Element,
Expand Down Expand Up @@ -50,7 +53,7 @@ impl PipelineManager {
element,
element_id,
*v,
MUTE_ANTICLICK_RAMP_MS,
ramp_ms.unwrap_or(MUTE_ANTICLICK_RAMP_MS),
) =>
{
return Ok(());
Expand Down Expand Up @@ -183,8 +186,8 @@ impl PipelineManager {
/// Validates that the property can be changed in the current pipeline state.
///
/// `ramp_ms` is consulted only for routes that support smooth interpolation
/// (currently audio `volume`-element `volume`). For other properties it is
/// silently ignored.
/// (currently audio `volume`-element `volume` and `mute`). For other
/// properties it is silently ignored.
pub fn update_element_property(
&self,
element_id: &str,
Expand Down
56 changes: 48 additions & 8 deletions backend/src/gst/volume_ramp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use gstreamer::prelude::*;
use gstreamer_controller::prelude::*;
use gstreamer_controller::{DirectControlBinding, InterpolationControlSource, InterpolationMode};
use std::collections::HashMap;
use std::sync::Mutex;
use std::sync::{Arc, Mutex};
use std::time::Duration;
use tracing::{debug, warn};

Expand Down Expand Up @@ -52,13 +52,20 @@ pub struct VolumeRampManager {
/// Pre-mute volume per element id, captured when entering mute so we can
/// restore it on unmute.
pre_mute: Mutex<HashMap<String, f64>>,
/// Per-element generation counter, bumped on every `apply_mute` call. The
/// delayed `mute=true` toggle scheduled by `apply_mute(true, ramp_ms)`
/// captures the generation at scheduling time and bails out on fire if it
/// has been superseded — so a fast unmute cancels a still-pending mute.
/// Wrapped in `Arc` so the spawned tokio task can own a handle.
mute_gen: Arc<Mutex<HashMap<String, u64>>>,
}

impl VolumeRampManager {
pub fn new() -> Self {
Self {
sources: Mutex::new(HashMap::new()),
pre_mute: Mutex::new(HashMap::new()),
mute_gen: Arc::new(Mutex::new(HashMap::new())),
}
}

Expand Down Expand Up @@ -190,7 +197,9 @@ impl VolumeRampManager {

/// Toggle `mute` with anti-click protection. Ramps `volume` toward zero
/// before `mute=true` is applied (masking the discontinuity click), and
/// restores the pre-mute volume on unmute.
/// restores the pre-mute volume on unmute. `ramp_ms` controls both the
/// pre-mute fade-out and the post-unmute fade-in — short values (≤50ms)
/// behave like a click guard, longer values produce broadcast-style fades.
///
/// Falls back to a direct `set_property` if the pipeline doesn't have a
/// running stream-time yet.
Expand All @@ -199,8 +208,20 @@ impl VolumeRampManager {
element: &gst::Element,
element_id: &str,
target_mute: bool,
anticlick_ms: u32,
ramp_ms: u32,
) -> bool {
// Bump generation up front so any in-flight scheduled `mute=true`
// toggle (from a previous apply_mute(true, …)) sees a stale value
// when it fires and bails out. Both directions invalidate stale
// pending toggles — a second mute(true) replaces the first, and
// mute(false) cancels a pending mute(true).
let current_gen = {
let mut gens = self.mute_gen.lock().unwrap();
let g = gens.entry(element_id.to_string()).or_insert(0);
*g = g.wrapping_add(1);
*g
};

if target_mute {
// Capture pre-mute volume only if not already muted (avoid
// overwriting on repeat-mute).
Expand All @@ -216,7 +237,7 @@ impl VolumeRampManager {
}

// Ramp to silence first.
if !self.apply_volume_ramp(element, element_id, 0.0, anticlick_ms) {
if !self.apply_volume_ramp(element, element_id, 0.0, ramp_ms) {
element.set_property("mute", true);
return true;
}
Expand All @@ -226,16 +247,29 @@ impl VolumeRampManager {
// A small extra margin (5ms) ensures the volume control source
// has reached zero before mute kicks in — otherwise the hard
// zeroing of the volume array would still produce a click.
// The captured generation is checked on fire: if a newer
// apply_mute call has bumped it, this scheduled toggle is stale
// and must not run (e.g. unmute arrived mid-fade).
let element_weak = element.downgrade();
let element_id_owned = element_id.to_string();
let delay = Duration::from_millis(anticlick_ms as u64 + 5);
let delay = Duration::from_millis(ramp_ms as u64 + 5);
let mute_gen = self.mute_gen.clone();
tokio::spawn(async move {
tokio::time::sleep(delay).await;
let still_current =
mute_gen.lock().unwrap().get(&element_id_owned).copied() == Some(current_gen);
if !still_current {
debug!(
"volume_ramp[{}]: scheduled mute=true superseded, skipping",
element_id_owned
);
return;
}
if let Some(elem) = element_weak.upgrade() {
elem.set_property("mute", true);
debug!(
"volume_ramp[{}]: mute=true applied after anti-click ramp",
element_id_owned
"volume_ramp[{}]: mute=true applied after {}ms fade-out",
element_id_owned, ramp_ms
);
}
});
Expand All @@ -256,7 +290,7 @@ impl VolumeRampManager {
// and a normal apply_volume_ramp would produce a flat ramp
// (instant unmute = click). Forcing start=0 guarantees a real
// 0→target fade-in.
if !self.apply_volume_ramp_from(element, element_id, 0.0, target, anticlick_ms) {
if !self.apply_volume_ramp_from(element, element_id, 0.0, target, ramp_ms) {
element.set_property("volume", target);
}
}
Expand All @@ -265,9 +299,15 @@ impl VolumeRampManager {

/// Drop all cached control sources. Bindings are owned by the elements
/// and are released when the pipeline drops; this just releases our side.
/// Also bumps every element's generation so any in-flight scheduled
/// `mute=true` toggle from a now-stopped pipeline becomes a no-op.
pub fn clear(&self) {
self.sources.lock().unwrap().clear();
self.pre_mute.lock().unwrap().clear();
let mut gens = self.mute_gen.lock().unwrap();
for v in gens.values_mut() {
*v = v.wrapping_add(1);
}
}
}

Expand Down
59 changes: 59 additions & 0 deletions backend/tests/volume_ramp_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,65 @@ async fn clear_drops_cache_and_subsequent_ramps_work() {
);
}

/// Long mute fades honor the caller-supplied ramp_ms instead of the legacy
/// hardcoded 10 ms anti-click. A 200 ms mute should still have the volume
/// well above zero at +50 ms (the old behavior would have hit zero in ~10 ms).
#[tokio::test(flavor = "multi_thread")]
async fn long_mute_ramp_takes_full_duration() {
let p = TestPipe::new();
let mgr = VolumeRampManager::new();

assert!(mgr.apply_volume_ramp(&p.volume, "v", 1.0, 50));
settle(80).await;

// Long fade-out before mute.
assert!(mgr.apply_mute(&p.volume, "v", true, 200));

// ~50 ms in: well below 1.0 (fade is dB-linear) but not yet silent.
tokio::time::sleep(Duration::from_millis(50)).await;
let mid = p.vol();
assert!(
!p.mute(),
"mute=true must not land before the fade-out has finished"
);
assert!(
mid > 0.01,
"at +50 ms of a 200 ms fade, volume must still be audible (got {})",
mid
);

// After the full duration plus the 5 ms grace, mute=true is applied.
tokio::time::sleep(Duration::from_millis(180)).await;
assert!(p.mute(), "mute=true should land after the 200 ms fade-out");
}

/// Cancel-guard: a mid-fade unmute must cancel the pending `mute=true`
/// toggle scheduled by the long fade-out, otherwise that toggle would land
/// after the unmute and silently kill the route.
#[tokio::test(flavor = "multi_thread")]
async fn unmute_during_long_mute_fade_cancels_pending_toggle() {
let p = TestPipe::new();
let mgr = VolumeRampManager::new();

assert!(mgr.apply_volume_ramp(&p.volume, "v", 1.0, 50));
settle(80).await;

// Start a long fade-out — schedules mute=true at +200 ms.
assert!(mgr.apply_mute(&p.volume, "v", true, 200));

// Unmute well before the scheduled mute=true would fire.
tokio::time::sleep(Duration::from_millis(40)).await;
assert!(mgr.apply_mute(&p.volume, "v", false, 50));

// Wait past the original 200 ms+grace window. The previously-scheduled
// mute=true must have observed the bumped generation and bailed out.
tokio::time::sleep(Duration::from_millis(250)).await;
assert!(
!p.mute(),
"stale scheduled mute=true must be cancelled by an intervening unmute"
);
}

/// `is_volume_element` correctly distinguishes the volume element from
/// other audio elements that happen to expose a `volume` property.
#[tokio::test(flavor = "multi_thread")]
Expand Down
4 changes: 2 additions & 2 deletions openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"license": {
"name": "MIT OR Apache-2.0"
},
"version": "0.4.12"
"version": "0.4.13-dev"
},
"paths": {
"/api/auth/status": {
Expand Down Expand Up @@ -9885,7 +9885,7 @@
"null"
],
"format": "int32",
"description": "Optional ramp duration in milliseconds. Currently only honored for\naudio `volume`-element `volume` updates — when set, the value is\ninterpolated per-sample over the given duration to avoid zipper\nnoise on fader drags or to match an auto-transition duration.\nWhen omitted, a short default ramp is used for `volume`; other\nproperties are set immediately.",
"description": "Optional ramp duration in milliseconds. Currently honored for audio\n`volume`-element `volume` and `mute` updates — when set, `volume` is\ninterpolated per-sample over the given duration (anti-zipper / fade)\nand `mute=true` is preceded by a fade-out of the same length while\n`mute=false` is followed by a 0→pre_mute fade-in. Useful for\nbroadcast-style on-air / off-air route transitions (e.g. 500 ms).\nWhen omitted, a short default ramp is used for `volume`/`mute`; other\nproperties are set immediately.",
"minimum": 0
},
"value": {
Expand Down
12 changes: 7 additions & 5 deletions types/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,13 @@ pub struct UpdatePropertyRequest {
/// The new value for the property
#[cfg_attr(feature = "validation", garde(skip))]
pub value: PropertyValue,
/// Optional ramp duration in milliseconds. Currently only honored for
/// audio `volume`-element `volume` updates — when set, the value is
/// interpolated per-sample over the given duration to avoid zipper
/// noise on fader drags or to match an auto-transition duration.
/// When omitted, a short default ramp is used for `volume`; other
/// Optional ramp duration in milliseconds. Currently honored for audio
/// `volume`-element `volume` and `mute` updates — when set, `volume` is
/// interpolated per-sample over the given duration (anti-zipper / fade)
/// and `mute=true` is preceded by a fade-out of the same length while
/// `mute=false` is followed by a 0→pre_mute fade-in. Useful for
/// broadcast-style on-air / off-air route transitions (e.g. 500 ms).
/// When omitted, a short default ramp is used for `volume`/`mute`; other
/// properties are set immediately.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[cfg_attr(feature = "validation", garde(range(max = 60000)))]
Expand Down
Loading