Merge pull request #1126 from SusieeTheLinuxUser/fix/clamp-clip-fx-gain-send-level

Mix: clamp clip-FX gain and send-level automation (fixes #884)

crates/engine/src/cmd/clip_more.rs

@@ -196,8 +196,14 @@ fn unloop(e: &mut Engine, p: &Value) -> Result<Value> {
 fn effects_set(e: &mut Engine, p: &Value) -> Result<Value> {
     let ids = clip_ids_param(e, p);
     let params = p.get("params").and_then(Value::as_object).ok_or_else(|| bad("clip.effects_set", "`params` must be an object of numbers"))?;
-    let vals: Vec<(String, f64)> =
-        params.iter().filter_map(|(k, v)| v.as_f64().filter(|x| x.is_finite()).map(|x| (k.chars().take(64).collect(), x))).collect();
+    let vals: Vec<(String, f64)> = params
+        .iter()
+        .filter_map(|(k, v)| v.as_f64().filter(|x| x.is_finite()).map(|x| (k.chars().take(64).collect(), x)))
+        .map(|(k, v)| {
+            let v = if k == "gain" { v.clamp(f64::from(soundcraft_dsp::MIN_DB), f64::from(soundcraft_mix::CLIP_FX_MAX_GAIN_DB)) } else { v };
+            (k, v)
+        })
+        .collect();
     let s = e.session_mut();
     for c in &ids {
         for (k, v) in &vals {
@@ -316,6 +322,13 @@ mod tests {
         (Engine::new(s), t, ids)
     }
 
+    #[test]
+    fn effects_set_caps_gain() {
+        let (mut e, _t, ids) = engine_with(&[(0, 100, 0)]);
+        e.execute("clip.effects_set", &json!({"clip": ids[0].0, "params": {"gain": 500.0}})).unwrap();
+        assert_eq!(e.session().edit.values.get(&format!("clip_fx.{}.gain", ids[0].0)).copied(), Some(24.0));
+    }
+
     #[test]
     fn unloop_removes_copies() {
         let (mut e, t, ids) = engine_with(&[(0, 100, 0), (100, 100, 0), (200, 100, 0), (300, 100, 50)]);

crates/mix/src/automation.rs

@@ -65,7 +65,7 @@ pub(crate) fn send_level(t: &Track, i: usize, base: f32, pos: Samples) -> f32 {
         && let Some(l) = t.lane(&AutoParam::SendLevel(u8::try_from(i).unwrap_or(0)))
         && !l.points.is_empty()
     {
-        return l.value_at(pos, base);
+        return l.value_at(pos, base).clamp(soundcraft_dsp::MIN_DB, crate::SEND_LEVEL_MAX_DB);
     }
     base
 }

crates/mix/src/clips.rs

@@ -101,7 +101,7 @@ impl ClipFx {
                     c.set_param(p, v);
                 }
             } else if k == "gain" {
-                self.gain_db = v;
+                self.gain_db = if v.is_nan() { 0.0 } else { v.clamp(soundcraft_dsp::MIN_DB, crate::CLIP_FX_MAX_GAIN_DB) };
             }
         }
     }

crates/mix/src/lib.rs

@@ -89,6 +89,11 @@ use strip::{DelayLine, MAX_LATENCY};
 use threads::run_strips;
 pub use threads::{mark_audio_thread, on_audio_thread};
 
+/// Top of the clip effects gain slider; a stored or wired gain is clamped to it (#884).
+pub const CLIP_FX_MAX_GAIN_DB: f32 = 24.0;
+/// Top of a send fader, the same +12 dB `AutoParam::SendLevel` ranges up to (#884).
+pub const SEND_LEVEL_MAX_DB: f32 = 12.0;
+
 /// Default (stereo) main mix width. The actual width is [`main_channels`].
 pub const MAIN_CHANNELS: usize = 2;
 

crates/mix/src/tests.rs

@@ -224,6 +224,15 @@ fn sends_feed_aux_via_bus() {
     assert!(out[0][1000] > 0.1, "aux should carry the send: {}", out[0][1000]);
 }
 
+#[test]
+fn send_level_automation_is_capped_at_the_fader_top() {
+    // #884: a send lane point past +12 dB (pushed without `set_point`'s clamp) reads as +12.
+    let (mut s, t) = session_with_dc(0.5, 4800);
+    let tr = s.track_mut(t).unwrap();
+    tr.lane_mut(&AutoParam::SendLevel(0)).points.push(soundcraft_model::AutomationPoint { at: 0, value: 500.0 });
+    assert_eq!(send_level(s.track(t).unwrap(), 0, 0.0, 100), SEND_LEVEL_MAX_DB);
+}
+
 #[test]
 fn render_bus_is_the_bus_not_the_main_mix() {
     let (mut s, t) = session_with_dc(0.5, 4800);

crates/model/src/session.rs

@@ -626,6 +626,8 @@ impl Session {
             }
             for l in &mut t.automation {
                 l.points.retain(|p| p.value.is_finite());
+                let (lo, hi, _) = l.param.range();
+                l.points.iter_mut().for_each(|p| p.value = p.value.clamp(lo, hi));
                 l.points.iter_mut().for_each(|p| bound(&mut p.at));
                 l.points.sort_by_key(|p| p.at);
             }
@@ -905,6 +907,22 @@ mod tests {
         assert!(s.next_id > t.0);
     }
 
+    #[test]
+    fn loaded_automation_is_clamped_to_its_range() {
+        // #884: a session file can carry points outside a lane's range; load clamps them.
+        let mut s = Session::default();
+        let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, None);
+        let tr = s.track_mut(t).unwrap();
+        for p in [crate::AutoParam::Volume, crate::AutoParam::SendLevel(0)] {
+            tr.lane_mut(&p).points.push(crate::AutomationPoint { at: 0, value: 500.0 });
+        }
+        let s = Session::from_json(&serde_json::to_string(&s).unwrap()).unwrap();
+        let tr = s.track(t).unwrap();
+        for p in [crate::AutoParam::Volume, crate::AutoParam::SendLevel(0)] {
+            assert_eq!(tr.lane(&p).unwrap().points[0].value, 12.0, "{p:?}");
+        }
+    }
+
     #[test]
     fn a_loaded_auto_name_scheme_leaves_room_for_its_number() {
         let mut scheme = crate::AutoName { name: "n".repeat(300), suffix: "s".repeat(300), ..Default::default() };