From 967354f512aa7d506f6d744a6addfecd3ec5a755 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 22 Sep 2026 11:09:00 +0200 Subject: [PATCH] perf(studio): reuse the audio mix across optimistic edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `optimistic::commit` bumps `generation` on EVERY mutation (one per `oninput` event), and `use_hot_reload` polls it every 250 ms, so a slider drag on a scenario that has audio calls `audio::prepare` ~4x/second. `prepare` spawns a brand-new `std::thread` that runs `mix_audio_tracks` over the whole scenario — documented in audio.rs:158 as "seconds on a long scenario" — decoding and resampling every track from scratch even though a font-size edit cannot have changed the audio. `MIX_TOKEN` only discards the RESULT; nothing cancels the work. Dozens of concurrent decode threads pile up, each holding a full interleaved f32 PCM buffer (~23 MB for 60 s stereo), fighting the 2-6 prefetch render workers for cores during exactly the interaction that most needs to stay smooth. Refs #220 --- crates/rustmotion-studio/src/editor/audio.rs | 20 +++++- .../rustmotion-studio/src/editor/playback.rs | 18 +++-- crates/rustmotion-studio/tests/audit_ws_g.rs | 71 +++++++++++++++---- 3 files changed, 88 insertions(+), 21 deletions(-) diff --git a/crates/rustmotion-studio/src/editor/audio.rs b/crates/rustmotion-studio/src/editor/audio.rs index 5b015479..59fbed59 100644 --- a/crates/rustmotion-studio/src/editor/audio.rs +++ b/crates/rustmotion-studio/src/editor/audio.rs @@ -13,12 +13,14 @@ //! by a command channel; position and state are published as atomics the UI //! reads without ever touching that thread. +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; use std::sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}; use std::sync::mpsc::{channel, RecvTimeoutError, Sender}; use std::sync::{Arc, OnceLock}; use std::time::Duration; -use rustmotion::schema::ResolvedScenario; +use rustmotion::schema::{AudioTrack, ResolvedScenario}; enum Cmd { /// Interleaved stereo f32 for the whole scenario, and its sample rate. @@ -145,6 +147,22 @@ fn sample_rate(rate: u32) -> rodio::SampleRate { rodio::SampleRate::new(rate).unwrap_or(rodio::SampleRate::new(44_100).unwrap()) } +/// Fingerprint of everything a mix depends on: every track's placement and +/// volume envelope, plus the total duration silence pads out to. Serialised +/// rather than hashed field by field so a field added to `AudioTrack` cannot +/// silently drop out of the comparison. Two scenarios with the same +/// fingerprint would remix to the same PCM, so [`use_hot_reload`](super::playback::use_hot_reload) +/// calls [`prepare`] only when it changes — an edit to anything else (layout, +/// text, color) must not re-decode and resample every track from scratch. +pub fn audio_fingerprint(audio: &[AudioTrack], total_duration: f64) -> u64 { + let mut hasher = DefaultHasher::new(); + serde_json::to_string(audio) + .unwrap_or_default() + .hash(&mut hasher); + total_duration.to_bits().hash(&mut hasher); + hasher.finish() +} + /// Mix the scenario's audio in the background and hand it to the audio thread. /// Cheap to call on every model reload: a scenario with no tracks just clears. pub fn prepare(scenario: Arc, total_duration: f64) { diff --git a/crates/rustmotion-studio/src/editor/playback.rs b/crates/rustmotion-studio/src/editor/playback.rs index 078fb93e..ed54ccbd 100644 --- a/crates/rustmotion-studio/src/editor/playback.rs +++ b/crates/rustmotion-studio/src/editor/playback.rs @@ -92,15 +92,18 @@ pub fn use_playback_clock(shared: Shared, mut current: Signal, playing: Sig } /// Bump `rev` whenever the watcher swaps in a reloaded model, so the `` -/// refetches the (now changed) current frame — and re-mix the preview audio for -/// whatever scenario is now loaded. The mix has to follow the model: opening -/// another file, or editing this one's tracks, otherwise keeps playing the -/// previous scenario's sound. +/// refetches the (now changed) current frame — and re-mix the preview audio +/// when the audio itself changed. Every optimistic edit (a slider drag is +/// ~4/s) bumps `generation`, but re-mixing decodes and resamples every track +/// from scratch, so gating on [`audio::audio_fingerprint`](super::audio::audio_fingerprint) +/// rather than on `generation` keeps a font-size or color edit from spawning +/// a fresh mixer thread it has no use for. pub fn use_hot_reload(shared: Shared, mut rev: Signal) { use_future(move || { let shared = shared.clone(); async move { let mut last_gen: Option = None; + let mut last_audio_fp: Option = None; loop { let (g, scenario, total) = { let m = shared.lock().unwrap_or_else(|e| e.into_inner()); @@ -112,7 +115,12 @@ pub fn use_hot_reload(shared: Shared, mut rev: Signal) { } last_gen = Some(g); let fps = scenario.video.fps.max(1); - super::audio::prepare(scenario, total as f64 / fps as f64); + let total_duration = total as f64 / fps as f64; + let fp = super::audio::audio_fingerprint(&scenario.audio, total_duration); + if last_audio_fp != Some(fp) { + last_audio_fp = Some(fp); + super::audio::prepare(scenario, total_duration); + } } tokio::time::sleep(Duration::from_millis(250)).await; } diff --git a/crates/rustmotion-studio/tests/audit_ws_g.rs b/crates/rustmotion-studio/tests/audit_ws_g.rs index 89b8325a..2e23d8f2 100644 --- a/crates/rustmotion-studio/tests/audit_ws_g.rs +++ b/crates/rustmotion-studio/tests/audit_ws_g.rs @@ -1,26 +1,28 @@ -//! Regression tests for the studio's file-write pipeline — a debounced write -//! that must rebase onto the current disk instead of replaying a stale -//! in-memory snapshot, coalesced edits inside one debounce window that must -//! all survive (not just the last), and undo/redo cancelling a still-pending -//! write before it can clobber the just-restored state — plus a render-thread -//! panic that must surface as an error instead of being cached as an empty -//! JPEG. +//! Regression tests for the studio's file-write pipeline and a few other +//! previously-untestable decision points: a debounced write that must rebase +//! onto the current disk instead of replaying a stale in-memory snapshot, +//! coalesced edits inside one debounce window that must all survive (not +//! just the last), undo/redo cancelling a still-pending write before it can +//! clobber the just-restored state, a render-thread panic that must surface +//! as an error instead of a cached empty JPEG, and the preview audio mixer +//! only re-running when the audio itself changed. //! //! `rustmotion-studio` has no `[dev-dependencies]` and cannot gain one in //! this change, so every test below drives the crate's existing public -//! surface (`scenario::*`, `editor::frames`) against real temp files with -//! plain synchronous `#[test]`s — no Dioxus runtime, no async executor. That -//! public surface is itself the fix for the crate having no integration -//! tests: the defects lived in a debounce timer and Dioxus event handlers -//! that cannot be driven from a test, so each one was reduced to a pure -//! decision over plain data (`resolve_flush`, the pending-write queue, -//! `render_frame_deep` returning `Result`) and the handler calls that instead -//! of deciding inline. +//! surface (`scenario::*`, `editor::audio`, `editor::frames`) against real +//! temp files with plain synchronous `#[test]`s — no Dioxus runtime, no +//! async executor. That public surface is itself the fix for the crate +//! having no integration tests: the defects lived in a debounce timer and +//! Dioxus event handlers that cannot be driven from a test, so each one was +//! reduced to a pure decision over plain data (`resolve_flush`, the +//! pending-write queue, `audio_fingerprint`) and the handler calls that +//! instead of deciding inline. use std::fs; use std::path::PathBuf; use std::sync::{Arc, Mutex}; +use rustmotion_studio::editor::audio::audio_fingerprint; use rustmotion_studio::editor::frames::render_frame_deep; use rustmotion_studio::scenario::{ apply_optimistic, empty_scenario, pending_write_slot, queue_mutation, record_edit, @@ -266,3 +268,42 @@ fn a_normal_render_returns_nonempty_jpeg_bytes() { let jpeg = render_frame_deep(&scenario, &tasks, 0, 1.0).expect("a normal render succeeds"); assert!(!jpeg.is_empty()); } + +// ── The preview audio mixer only re-runs when the audio actually changed ─── + +#[test] +fn audio_fingerprint_ignores_unrelated_edits_and_reacts_to_real_audio_changes() { + let scenario_a = rustmotion::loader::load_scenario_from_source( + None, + Some(r##"{ "video": { "width": 64, "height": 64 }, "audio": [ { "src": "a.mp3" } ], "scenes": [ { "duration": 1.0 } ] }"##), + ) + .unwrap(); + let scenario_b_same_audio = rustmotion::loader::load_scenario_from_source( + None, + Some(r##"{ "video": { "width": 64, "height": 64 }, "audio": [ { "src": "a.mp3" } ], "scenes": [ { "duration": 1.0, "children": [ { "type": "text", "content": "Hi" } ] } ] }"##), + ) + .unwrap(); + let scenario_c_different_audio = rustmotion::loader::load_scenario_from_source( + None, + Some(r##"{ "video": { "width": 64, "height": 64 }, "audio": [ { "src": "a.mp3", "volume": 0.5 } ], "scenes": [ { "duration": 1.0 } ] }"##), + ) + .unwrap(); + + let fp_a = audio_fingerprint(&scenario_a.audio, 1.0); + let fp_b = audio_fingerprint(&scenario_b_same_audio.audio, 1.0); + let fp_c = audio_fingerprint(&scenario_c_different_audio.audio, 1.0); + + assert_eq!( + fp_a, fp_b, + "an edit unrelated to audio (layout, text) must not change the fingerprint" + ); + assert_ne!( + fp_a, fp_c, + "a real audio change (volume) must change the fingerprint" + ); + assert_ne!( + audio_fingerprint(&scenario_a.audio, 1.0), + audio_fingerprint(&scenario_a.audio, 2.0), + "a total-duration change (silence padding) must also change the fingerprint" + ); +}