From 6471b1ea3550427488e17100cd239d723df034d2 Mon Sep 17 00:00:00 2001 From: Thales <> Date: Fri, 17 Jul 2026 12:13:51 +0100 Subject: [PATCH] feat(settings): separation quality (--shifts) setting Adds a "Standard" / "Best (2x slower)" separation quality setting, following the demucs_device runtime-settings pattern exactly (app/core/settings.py get/set + env seed, app/main.py payload + POST handler with 422 on an invalid choice). "Best" appends --shifts 2 to the demucs invocation: separation runs twice on a randomly time-shifted copy of the input and averages the two passes -- measurably cleaner stems, ~2x the separation time. Applies on any device; a CPU user who opts in accepts the wait knowingly. Settings UI: new select next to Compute device on the General tab, wired the same way as the export sample rate / video height selects. --- app/core/settings.py | 32 ++++++++++++++++++++++++++++++++ app/main.py | 8 ++++++++ app/pipeline/separate.py | 15 ++++++++++----- static/js/catalog.js | 19 ++++++++++++++++++- tests/test_network_gate.py | 30 ++++++++++++++++++++++++++++++ tests/test_separate_fallback.py | 13 +++++++++++++ 6 files changed, 111 insertions(+), 6 deletions(-) diff --git a/app/core/settings.py b/app/core/settings.py index 985734b..9e845a4 100644 --- a/app/core/settings.py +++ b/app/core/settings.py @@ -8,6 +8,7 @@ - `video_max_height` — max video resolution for MP4 export / YouTube pulls. - `export_sample_rate` — sample rate for exported mixes/regions (WAV/FLAC/MP3). - `demucs_device` — compute device for separation: auto | cuda | mps | cpu. +- `separation_quality` — demucs shift-averaging: standard | best (2x slower). Defaults fall back to the config.py constants (which honor their env vars), so nothing changes until the user overrides a value. @@ -226,3 +227,34 @@ def set_demucs_device(value: str) -> str: _ensure()["demucs_device"] = choice _save() return choice + + +# ── separation_quality ── +# "standard" (default) runs demucs once. "best" adds --shifts 2: demucs +# re-runs separation on a randomly time-shifted copy of the input and +# averages the two -- measurably cleaner stems, at ~2x the separation time. +# Applies on any device; a CPU user who picks "best" is accepting the wait +# knowingly. STEMDECK_SEPARATION_QUALITY seeds the default so existing +# env-based deployments can force it. +_QUALITY_CHOICES = ("standard", "best") + + +def _default_separation_quality() -> str: + env = os.environ.get("STEMDECK_SEPARATION_QUALITY", "").strip().lower() + return env if env in _QUALITY_CHOICES else "standard" + + +def get_separation_quality() -> str: + with _LOCK: + v = _ensure().get("separation_quality") + return v if isinstance(v, str) and v in _QUALITY_CHOICES else _default_separation_quality() + + +def set_separation_quality(value: str) -> str: + choice = (value or "").strip().lower() + if choice not in _QUALITY_CHOICES: + raise ValueError("separation_quality must be one of: " + ", ".join(_QUALITY_CHOICES)) + with _LOCK: + _ensure()["separation_quality"] = choice + _save() + return choice diff --git a/app/main.py b/app/main.py index 1371307..a274d40 100644 --- a/app/main.py +++ b/app/main.py @@ -37,12 +37,14 @@ get_export_sample_rate, get_max_duration_sec, get_port, + get_separation_quality, get_video_max_height, set_allow_network, set_demucs_device, set_export_sample_rate, set_max_duration_sec, set_port, + set_separation_quality, set_video_max_height, ) from app.pipeline.collect import sweep_failed_jobs, sweep_old_jobs @@ -251,6 +253,7 @@ def _settings_payload() -> dict[str, object]: "max_duration_sec": get_max_duration_sec(), "video_max_height": get_video_max_height(), "export_sample_rate": get_export_sample_rate(), + "separation_quality": get_separation_quality(), "port": get_port(), # The user's choice ("auto" | "cuda" | "mps" | "cpu") drives the UI # select; the resolved value shows what jobs will actually run on; @@ -307,6 +310,11 @@ async def update_settings(request: Request) -> dict[str, object]: # set_demucs_device's messages are safe, user-actionable strings # (invalid choice / device not available on this machine). raise HTTPException(status_code=422, detail=str(e)) from None + if "separation_quality" in body: + try: + set_separation_quality(str(body["separation_quality"])) + except ValueError as e: + raise HTTPException(status_code=422, detail=str(e)) from None return _settings_payload() diff --git a/app/pipeline/separate.py b/app/pipeline/separate.py index 2b72390..b9566f2 100644 --- a/app/pipeline/separate.py +++ b/app/pipeline/separate.py @@ -13,7 +13,7 @@ from app.core.config import DEMUCS_MODEL, TIMEOUT_DEMUCS_STALL from app.core.models import Job, JobCancelled, _set from app.core.registry import set_proc -from app.core.settings import get_demucs_device +from app.core.settings import get_demucs_device, get_separation_quality from app.pipeline.errors import SeparationError, classify_failure logger = logging.getLogger("stemdeck.pipeline") @@ -27,7 +27,7 @@ def _demucs_cmd(device: str, source: Path, job_dir: Path) -> list[str]: """Build the demucs CLI invocation. Module-level seam so tests can swap in a stub executable without touching the process-management machinery.""" - return [ + cmd = [ sys.executable, "-m", "demucs", @@ -35,10 +35,15 @@ def _demucs_cmd(device: str, source: Path, job_dir: Path) -> list[str]: DEMUCS_MODEL, "-d", device, - "-o", - str(job_dir), - str(source), ] + # "best" quality (Settings -> General): demucs re-runs separation on a + # randomly time-shifted copy of the input and averages the two passes -- + # measurably cleaner stems, ~2x the separation time. Applies on any + # device; read fresh per job like get_demucs_device() below. + if get_separation_quality() == "best": + cmd += ["--shifts", "2"] + cmd += ["-o", str(job_dir), str(source)] + return cmd def _run_demucs(job: Job, source: Path, job_dir: Path, device: str) -> tuple[int, list[str]]: diff --git a/static/js/catalog.js b/static/js/catalog.js index ae714ee..99bd94c 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -1857,7 +1857,8 @@ async function wireGeneralSettings(overlay) { const portInput = overlay.querySelector(".set-port"); const deviceSel = overlay.querySelector(".set-demucs-device"); const deviceResolved = overlay.querySelector(".set-demucs-resolved"); - if (!durInput && !heightSel && !sampleRateSel && !portInput && !deviceSel) return; + const qualitySel = overlay.querySelector(".set-separation-quality"); + if (!durInput && !heightSel && !sampleRateSel && !portInput && !deviceSel && !qualitySel) return; // Last server-confirmed device choice, to revert the select when the server // rejects a forced device (e.g. CUDA not available on this machine). @@ -1868,6 +1869,7 @@ async function wireGeneralSettings(overlay) { if (heightSel && d.video_max_height) heightSel.value = String(d.video_max_height); if (sampleRateSel && d.export_sample_rate) sampleRateSel.value = String(d.export_sample_rate); if (portInput && d.port) portInput.value = String(d.port); + if (qualitySel && d.separation_quality) qualitySel.value = d.separation_quality; if (deviceSel) { // Gray out devices this machine can't use (Auto and CPU are always // available). Label disabled options so it's clear WHY they're greyed. @@ -1928,6 +1930,9 @@ async function wireGeneralSettings(overlay) { const port = Math.max(1024, Math.min(65535, parseInt(portInput.value, 10) || 8000)); post({ port }); }); + qualitySel?.addEventListener("change", () => { + post({ separation_quality: qualitySel.value }); + }); // Compute device needs its own POST path: unlike the clamped numeric // settings, the server can REJECT a forced device (422 with a reason, e.g. // "cuda is not available on this machine") -- surface that and revert. @@ -2101,6 +2106,18 @@ function openLibraryEditor() { +
+
+
+
Separation quality
+
Best runs the separator twice with randomized shifts and averages the result — cleaner stems, twice the time.
+
+ +
+
Out of sync tracks
diff --git a/tests/test_network_gate.py b/tests/test_network_gate.py index d84f600..e0e02ca 100644 --- a/tests/test_network_gate.py +++ b/tests/test_network_gate.py @@ -172,6 +172,36 @@ def test_demucs_device_api_round_trip_and_422(monkeypatch, _isolated_settings): assert c.post("/api/settings", json={"demucs_device": "bogus"}).status_code == 422 +# ── separation_quality ── + + +def test_separation_quality_defaults_to_standard(_isolated_settings): + assert settings_mod.get_separation_quality() == "standard" + + +def test_separation_quality_env_seeds_default(monkeypatch, _isolated_settings): + monkeypatch.setenv("STEMDECK_SEPARATION_QUALITY", "best") + assert settings_mod.get_separation_quality() == "best" + + +def test_separation_quality_rejects_unknown_choice(_isolated_settings): + with pytest.raises(ValueError): + settings_mod.set_separation_quality("ultra") + assert settings_mod.get_separation_quality() == "standard" # nothing persisted + + +def test_separation_quality_api_round_trip_and_422(_isolated_settings): + with TestClient(app) as c: + assert c.get("/api/settings").json()["separation_quality"] == "standard" + r = c.post("/api/settings", json={"separation_quality": "best"}) + assert r.status_code == 200 + assert r.json()["separation_quality"] == "best" + assert c.get("/api/settings").json()["separation_quality"] == "best" + r = c.post("/api/settings", json={"separation_quality": "ultra"}) + assert r.status_code == 422 + assert c.get("/api/settings").json()["separation_quality"] == "best" # unchanged + + def test_gate_blocks_non_loopback_when_off(): settings_mod.set_allow_network(False) # TestClient's client host ("testclient") is treated as non-loopback. diff --git a/tests/test_separate_fallback.py b/tests/test_separate_fallback.py index c3fa58c..826809e 100644 --- a/tests/test_separate_fallback.py +++ b/tests/test_separate_fallback.py @@ -70,6 +70,19 @@ def test_gpu_failure_falls_back_to_cpu(job, tmp_path, monkeypatch, caplog): assert "CUDA out of memory" in warning +def test_demucs_cmd_omits_shifts_at_standard_quality(monkeypatch, tmp_path): + monkeypatch.setattr(sep_mod, "get_separation_quality", lambda: "standard") + cmd = sep_mod._demucs_cmd("cpu", tmp_path / "source.wav", tmp_path) + assert "--shifts" not in cmd + + +def test_demucs_cmd_includes_shifts_2_at_best_quality(monkeypatch, tmp_path): + monkeypatch.setattr(sep_mod, "get_separation_quality", lambda: "best") + cmd = sep_mod._demucs_cmd("cpu", tmp_path / "source.wav", tmp_path) + i = cmd.index("--shifts") + assert cmd[i + 1] == "2" + + def test_records_startup_timing_on_first_progress_line(job, tmp_path, monkeypatch): """#288: measurement only -- time from Popen to the first progress line demucs emits, so the real subprocess/model-load startup cost can be