From 19b0083a3ace386aec40571c4bf6d65ff1ea7e60 Mon Sep 17 00:00:00 2001 From: Thales Pereira <31625914+thcp@users.noreply.github.com> Date: Mon, 7 Sep 2026 15:14:45 +0100 Subject: [PATCH 1/4] feat(click): group the bar so odd and compound meters sound like themselves The click marked exactly one beat per bar, on the same rule in both implementations, with only two voices. Correct for 2/4, 3/4 and 4/4. Wrong for every meter that carries internal stresses: 7/8 came out as seven even clicks, which gives a player nothing to lock onto, and 6/8 rendered as six straight eighths when it is felt in two dotted-quarter groups. Adds a grouping to the bar. 7 is played 3+2+2, 6 and 9 and 12 in threes, 5 as 3+2, with an accent at each group start. A meter with no conventional reading, 11 for instance, stays flat rather than being guessed at. Grouping needs a third voice. With only an accent and a plain beat, a 3+2+2 bar has three identical accents and the downbeat stops being findable, so a group start now sounds between the two: 1225 Hz at 0.85, the geometric middle of the existing 1000 and 1500 because the ear hears pitch ratios. Both renderers gained it and both count-ins use it, so counting a player into 7/8 gives them the pulse they are about to play. 2, 3 and 4 are deliberately unchanged. 4/4 does carry a real secondary stress on beat 3, but turning that on by default would change the commonest meter in the app for every existing user. That is their choice to make, and the grouping box makes it available. 6/8 does change, and that is the point of the issue: it was shipping a reading of the meter that no player would count. The user can override the grouping for an explicit meter, "3+2+2" or "2+2+3". One that does not sum to the bar is refused wholesale rather than repaired, on both sides, because a half-understood grouping puts accents on beats nobody asked for. The export mirrors that rather than returning 422: a file is better served with the default grouping than refused. Under Auto no single grouping can apply, since the bar length can change from bar to bar, so each detected bar is grouped by its own length. That is what makes a detected 6/8 passage group in threes with nothing configured, and it needed a bar-position lookup on the live side, where only a downbeat yes/no had existed. The grouping enters the render cache key only when one is in force, so every export made before this keeps hitting the entry it already had. Playback and export are held to the same numbers in tests/js/count-in.test.mjs and tests/test_click_render.py, the existing parity discipline for this pair. Closes #595 --- app/api/stems.py | 32 +++++- app/pipeline/click_render.py | 197 ++++++++++++++++++++++++++++----- static/css/daw.css | 13 +++ static/index.html | 1 + static/js/beatgrid.js | 17 +++ static/js/i18n.js | 20 ++++ static/js/metronome.js | 130 ++++++++++++++++++++-- static/js/player.js | 4 +- static/js/state.js | 9 ++ static/js/transport.js | 59 +++++++++- tests/e2e/click-track.spec.mjs | 31 ++++++ tests/js/count-in.test.mjs | 57 +++++++++- tests/test_click_render.py | 189 ++++++++++++++++++++++++++++++- 13 files changed, 710 insertions(+), 49 deletions(-) diff --git a/app/api/stems.py b/app/api/stems.py index fb178b7e..c5743557 100644 --- a/app/api/stems.py +++ b/app/api/stems.py @@ -216,6 +216,26 @@ class _ClickLane(NamedTuple): count_in: bool +def _parse_groups(raw: str, accent_mode: int) -> list[int] | None: + """Parse a "3+2+2" grouping, or None to use the default. + + Rejected wholesale rather than repaired, and never a 422: a grouping that + does not fit the bar would put accents on beats the user never asked for, + and an export is better served with the default grouping than refused. The + same rule the browser applies in transport.js::_applyGrouping, so a URL a + user cannot produce from the UI degrades to what the UI would have sent. + + Only meaningful with an explicit meter -- under Auto each detected bar is + grouped by its own length, so one grouping cannot be assumed to fit. + """ + if not raw or accent_mode <= 0: + return None + parts = [int(p) for p in re.findall(r"\d+", raw)[:32]] + if not parts or any(p < 1 for p in parts) or sum(parts) != accent_mode: + return None + return parts + + def _click_lane( job_id: str, enabled: bool, @@ -225,6 +245,7 @@ def _click_lane( count_in_bars: int = 0, start: float | None = None, end: float | None = None, + groups: list[int] | None = None, ) -> _ClickLane | None: """Render (or reuse) the click / count-in track for this job as an extra ffmpeg input, or None when both are off or the job has no beat grid. @@ -260,6 +281,7 @@ def _click_lane( include_click=enabled, start=start, end=end, + groups=groups, ) path = _CLICK_CACHE_DIR / f"{key}.wav" @@ -267,7 +289,7 @@ def _click_lane( # lead_in is a pure function of the grid; recompute it even on a cache # hit so the caller can delay the stems without re-reading the WAV. lead_in, _ = count_in_beats( - beats, bars, count_in_bars, multiplier, accent_mode, start=start or 0.0 + beats, bars, count_in_bars, multiplier, accent_mode, start=start or 0.0, groups=groups ) if not path.is_file(): try: @@ -283,6 +305,7 @@ def _click_lane( include_click=enabled, start=start or 0.0, end=end, + groups=groups, ) except Exception: logger.exception("count-in render failed for %s", job_id) @@ -305,6 +328,7 @@ def _click_lane( sample_rate=sample_rate, multiplier=multiplier, accent_mode=accent_mode, + groups=groups, ) except Exception: logger.exception("click render failed for %s", job_id) @@ -804,6 +828,11 @@ async def get_mixdown( click_mult: float = Query(default=1.0, description="Click rate: 0.5, 1 or 2"), click_accent: int = Query(default=-1, ge=-1, le=32, description="-1 auto, 0 off, N per bar"), click_gain: float = Query(default=0.6, ge=0, le=4, description="Click level"), + click_groups: str = Query( + default="", + max_length=64, + description='Beat grouping for an explicit meter, e.g. "3+2+2". Blank uses the default.', + ), count_in: int = Query( # Ceiling matches MAX_COUNT_IN_BARS in static/js/state.js, the longest # count-in the UI can ask for (#587). The lead-in it prepends is @@ -858,6 +887,7 @@ async def get_mixdown( count_in_bars=count_in, start=start, end=end, + groups=_parse_groups(click_groups, click_accent), ) cache_key = _mixdown_cache_key(job_id, ext, names, parsed_gains, start, end, click_lane) cache_path = _MIXDOWN_CACHE_DIR / f"{cache_key}.{ext}" diff --git a/app/pipeline/click_render.py b/app/pipeline/click_render.py index e94709a2..08119a69 100644 --- a/app/pipeline/click_render.py +++ b/app/pipeline/click_render.py @@ -24,11 +24,23 @@ # Mirrors the constants at the top of static/js/metronome.js. Changing either # side without the other makes exports diverge from playback. CLICK_FREQ = 1000.0 +# Group starts sit between the downbeat and a plain beat in both pitch and +# level, so a grouped bar reads as "strong, medium, weak" rather than as three +# identical accents (#595). Geometric middle of the two existing voices: the ear +# hears pitch ratios, so 1225 is the midpoint of 1000 and 1500, not 1250. +GROUP_FREQ = 1225.0 ACCENT_FREQ = 1500.0 CLICK_DECAY = 0.035 CLICK_ATTACK = 0.001 CLICK_PEAK = 0.7 +GROUP_PEAK = 0.85 ACCENT_PEAK = 1.0 + +# Click strength at one beat. The renderer and static/js/metronome.js must agree +# on these three, or a monitored click and an exported one differ. +LEVEL_WEAK = 0 +LEVEL_GROUP = 1 +LEVEL_DOWNBEAT = 2 # exponentialRampToValueAtTime cannot start from zero, so the scheduler ramps # from this floor; matching it keeps the attack shape identical. RAMP_FLOOR = 0.0001 @@ -40,6 +52,62 @@ _VALID_MULTIPLIERS = (0.5, 1.0, 2.0) +def default_grouping(beats_per_bar: int) -> list[int]: + """How a bar of `beats_per_bar` divides, when the user has not said. + + Odd and compound meters carry internal stresses that a flat bar does not + express: 7 is played 3+2+2, 6 is two dotted-quarter groups, 5 is 3+2. The + click has to mark those or it gives a player nothing to lock onto (#595). + + 2, 3 and 4 deliberately return a single group, leaving them exactly as they + sounded before. 4/4 does carry a real secondary stress on beat 3, but + turning that on by default would change the most common meter in the app for + every existing user, which is a decision for them and not a default. + + Mirrored by defaultGrouping() in static/js/metronome.js -- keep both in step. + """ + if beats_per_bar < 1: + return [] + if beats_per_bar in (5, 7): + # 3 first: the long group leads in both, which is the commoner reading + # (Take Five is 3+2, and 7/8 is more often 3+2+2 than 2+2+3). + return [3] + [2] * ((beats_per_bar - 3) // 2) + if beats_per_bar >= 6 and beats_per_bar % 3 == 0: + # Compound: 6, 9 and 12 are felt in dotted-quarter groups of three. + return [3] * (beats_per_bar // 3) + return [beats_per_bar] + + +def normalise_grouping(groups: list[int] | None, beats_per_bar: int) -> list[int]: + """A grouping that is safe to index a bar with, or the default. + + Anything that is not a list of positive ints summing to the bar length is + rejected wholesale rather than repaired: a half-understood grouping would + put accents on beats the user never asked for, and silently playing the + default is the honest failure. + """ + if beats_per_bar < 1: + return [] + if not groups: + return default_grouping(beats_per_bar) + if not all(isinstance(g, int) and g >= 1 for g in groups): + return default_grouping(beats_per_bar) + if sum(groups) != beats_per_bar: + return default_grouping(beats_per_bar) + return list(groups) + + +def group_offsets(groups: list[int]) -> set[int]: + """Beat offsets inside a bar that start a group, excluding the downbeat + (which is already the stronger LEVEL_DOWNBEAT).""" + offsets: set[int] = set() + at = 0 + for g in groups[:-1]: + at += g + offsets.add(at) + return offsets + + def rescale_beats(beats: list[float], multiplier: float) -> list[float]: """Apply the playback rate multiplier. Mirrors `_rescale` in metronome.js: doubling inserts midpoints, halving takes every other beat, and both derive @@ -72,12 +140,16 @@ def source_index(i: int, multiplier: float) -> int | None: return i -def is_downbeat(index: int | None, bars: list[dict], accent_mode: int) -> bool: - """Whether the beat at `index` (original grid) carries an accent.""" - if index is None or accent_mode == ACCENT_OFF: - return False +def _bar_position(index: int, bars: list[dict], accent_mode: int) -> tuple[int, int] | None: + """(offset into the bar, bar length) for a beat, or None when no bar applies. + + Split out of is_downbeat so grouping can ask *where* in the bar a beat falls, + not merely whether it is the first one (#595). + """ + if accent_mode == ACCENT_OFF: + return None if accent_mode > 0: - return index % accent_mode == 0 + return index % accent_mode, accent_mode # Auto: follow the last bar mark at or before this beat. mark = None for b in bars: @@ -87,11 +159,45 @@ def is_downbeat(index: int | None, bars: list[dict], accent_mode: int) -> bool: else: break if mark is None: - return False + return None per_bar = mark.get("beats_per_bar") if not isinstance(per_bar, int) or per_bar < 1: + return None + return (index - mark["beat"]) % per_bar, per_bar + + +def is_downbeat(index: int | None, bars: list[dict], accent_mode: int) -> bool: + """Whether the beat at `index` (original grid) carries an accent.""" + if index is None: return False - return (index - mark["beat"]) % per_bar == 0 + pos = _bar_position(index, bars, accent_mode) + return pos is not None and pos[0] == 0 + + +def beat_level( + index: int | None, bars: list[dict], accent_mode: int, groups: list[int] | None = None +) -> int: + """How strongly the beat at `index` is clicked: downbeat, group start, weak. + + `groups` applies only to an explicit meter. Under Auto the bar length can + change from bar to bar, so a single user-supplied grouping cannot be assumed + to fit every bar; each bar falls back to the default for its own length, + which is what makes a detected 6/8 passage group in threes without the user + configuring anything. + """ + if index is None: + return LEVEL_WEAK + pos = _bar_position(index, bars, accent_mode) + if pos is None: + return LEVEL_WEAK + offset, per_bar = pos + if offset == 0: + return LEVEL_DOWNBEAT + if accent_mode > 0: + bar_groups = normalise_grouping(groups, per_bar) + else: + bar_groups = default_grouping(per_bar) + return LEVEL_GROUP if offset in group_offsets(bar_groups) else LEVEL_WEAK def count_in_beats_per_bar(bars: list[dict], accent_mode: int, start_index: int = 0) -> int: @@ -150,15 +256,20 @@ def count_in_beats( multiplier: float = 1.0, accent_mode: int = ACCENT_AUTO, start: float = 0.0, -) -> tuple[float, list[tuple[float, bool]]]: + groups: list[int] | None = None, +) -> tuple[float, list[tuple[float, int]]]: """Compute the count-in that leads into playback at `start`. Returns `(lead_in, clicks)` where `lead_in` is the seconds of pre-roll to - prepend and `clicks` is `[(offset, accent), ...]` with each offset in + prepend and `clicks` is `[(offset, level), ...]` with each offset in `[0, lead_in)`. One bar of the detected meter counts in by default: `PI po po po` on 4/4, the final click landing one beat before the audio so the song enters on the next downbeat. + The count-in is grouped exactly as the running click is (#595), so counting + a player into 7/8 gives them the 3+2+2 pulse they are about to play rather + than seven flat clicks. + Pure and side-effect free so playback (metronome.js) and export (render_click_wav) can share one definition -- pinned by tests/test_click_render.py, exactly like rescale/source_index/is_downbeat. @@ -180,7 +291,21 @@ def count_in_beats( return 0.0, [] n = count_bars * bpb lead_in = n * interval - clicks = [(j * interval, (j % bpb) == 0) for j in range(n)] + # The count-in is its own run of bars, so its grouping comes from bpb + # directly rather than from the bar marks: there is no detected bar to + # consult in front of the audio. + bar_groups = normalise_grouping(groups, bpb) if accent_mode > 0 else default_grouping(bpb) + starts = group_offsets(bar_groups) + clicks = [] + for j in range(n): + offset = j % bpb + if offset == 0: + level = LEVEL_DOWNBEAT + elif offset in starts: + level = LEVEL_GROUP + else: + level = LEVEL_WEAK + clicks.append((j * interval, level)) return lead_in, clicks @@ -212,6 +337,7 @@ def cache_key( include_click: bool = True, start: float | None = None, end: float | None = None, + groups: list[int] | None = None, ) -> str: """Every input to the render is in the key. Beats are included by digest rather than by job id alone: an edited grid must not hit a cache entry @@ -228,6 +354,11 @@ def cache_key( ).hexdigest() bar_sig = ",".join(f"{b.get('beat')}:{b.get('beats_per_bar')}" for b in bars) raw = f"{job_id}|{grid}|{bar_sig}|{duration:.3f}|{sample_rate}|{multiplier}|{accent_mode}" + # Appended only when a grouping is actually in force, so every export that + # predates grouping keeps the exact key it had and the cache survives the + # change. Without this a re-grouped render would serve the old flat file. + if groups: + raw += f"|g{'+'.join(str(int(g)) for g in groups)}" if count_in_bars > 0: seg = f"{'' if start is None else f'{start:.3f}'}:{'' if end is None else f'{end:.3f}'}" raw += f"|ci{count_in_bars}|clk{int(include_click)}|{seg}" @@ -235,23 +366,28 @@ def cache_key( def _song_click_events( - beats: list[float], bars: list[dict], multiplier: float, accent_mode: int -) -> list[tuple[float, bool]]: - """The (time, accent) pair for every beat of the running click track, in + beats: list[float], + bars: list[dict], + multiplier: float, + accent_mode: int, + groups: list[int] | None = None, +) -> list[tuple[float, int]]: + """The (time, level) pair for every beat of the running click track, in source time. Shared by the plain click render and the count-in render so the click sounds identical whether or not a count-in precedes it.""" grid = rescale_beats([float(b) for b in beats], multiplier) return [ - (t, is_downbeat(source_index(i, multiplier), bars, accent_mode)) for i, t in enumerate(grid) + (t, beat_level(source_index(i, multiplier), bars, accent_mode, groups)) + for i, t in enumerate(grid) ] def _render_events( - dest: Path, events: list[tuple[float, bool]], duration: float, sample_rate: int + dest: Path, events: list[tuple[float, int]], duration: float, sample_rate: int ) -> Path | None: - """Stamp a list of (time, accent) clicks into a mono WAV of `duration` + """Stamp a list of (time, level) clicks into a mono WAV of `duration` seconds. The single place clicks become audio, so playback parity only has - to be maintained against the two voices, not against two render paths.""" + to be maintained against the three voices, not against two render paths.""" total = int(round(duration * sample_rate)) if not events or total <= 0: return None @@ -263,15 +399,18 @@ def _render_events( # again in the int16 conversion below. The output is 16-bit PCM, so the # extra mantissa was never audible (#512). buf = np.zeros(total, dtype=np.float32) - # Only two distinct voices, so render each once and stamp it in. - plain = _voice(CLICK_PEAK, CLICK_FREQ, sample_rate) - accented = _voice(ACCENT_PEAK, ACCENT_FREQ, sample_rate) - - for t, accent in events: + # Only three distinct voices, so render each once and stamp it in. + voices = { + LEVEL_WEAK: _voice(CLICK_PEAK, CLICK_FREQ, sample_rate), + LEVEL_GROUP: _voice(GROUP_PEAK, GROUP_FREQ, sample_rate), + LEVEL_DOWNBEAT: _voice(ACCENT_PEAK, ACCENT_FREQ, sample_rate), + } + + for t, level in events: start = int(round(t * sample_rate)) if start >= total or start < 0: continue - voice = accented if accent else plain + voice = voices.get(int(level), voices[LEVEL_WEAK]) n = min(len(voice), total - start) # Clicks can overlap at very fast tempos; summing matches the graph, # where every click is its own node into the same gain. @@ -299,6 +438,7 @@ def render_click_wav( sample_rate: int = 44100, multiplier: float = 1.0, accent_mode: int = ACCENT_AUTO, + groups: list[int] | None = None, ) -> Path | None: """Write a mono WAV of the click track spanning the whole track. @@ -309,7 +449,7 @@ def render_click_wav( """ if multiplier not in _VALID_MULTIPLIERS: multiplier = 1.0 - events = _song_click_events(beats, bars, multiplier, accent_mode) + events = _song_click_events(beats, bars, multiplier, accent_mode, groups) out = _render_events(dest, events, duration, sample_rate) if out is not None: logger.info("click render: %d beats, %.1f s -> %s", len(events), duration, dest.name) @@ -328,6 +468,7 @@ def render_count_in_wav( include_click: bool = True, start: float = 0.0, end: float | None = None, + groups: list[int] | None = None, ) -> tuple[Path, float] | None: """Render the click WAV for a count-in export, in *output* coordinates. @@ -345,13 +486,13 @@ def render_count_in_wav( seg_len = max(0.0, seg_end - seg_start) lead_in, count_clicks = count_in_beats( - beats, bars, count_in_bars, multiplier, accent_mode, start=seg_start + beats, bars, count_in_bars, multiplier, accent_mode, start=seg_start, groups=groups ) - events: list[tuple[float, bool]] = list(count_clicks) + events: list[tuple[float, int]] = list(count_clicks) if include_click: - for t, accent in _song_click_events(beats, bars, multiplier, accent_mode): + for t, level in _song_click_events(beats, bars, multiplier, accent_mode, groups): if seg_start <= t < seg_end: - events.append((t - seg_start + lead_in, accent)) + events.append((t - seg_start + lead_in, level)) out = _render_events(dest, events, lead_in + seg_len, sample_rate) if out is None: diff --git a/static/css/daw.css b/static/css/daw.css index 9dc03447..992dfdad 100644 --- a/static/css/daw.css +++ b/static/css/daw.css @@ -2847,6 +2847,19 @@ input, textarea { font-family: inherit; } .metro-custom-beats:hover { background: var(--panel-3); } .metro-custom-beats:focus { border-color: var(--accent); } +/* Beat grouping for the current meter, e.g. 3+2+2 (#595). Shown only when the + meter has more than one sensible reading. */ +.metro-group { + width: 74px; height: 34px; padding: 0 8px; + background: var(--panel); border: 1px solid var(--border-strong); + border-radius: 8px; color: var(--fg); + font-family: inherit; font-size: 12px; font-weight: 500; outline: none; + font-variant-numeric: tabular-nums; flex-shrink: 0; text-align: center; +} +.metro-group:hover { background: var(--panel-3); } +.metro-group:focus { border-color: var(--accent); } +.metro-group::placeholder { color: var(--muted); } + /* Detection confidence / unavailability note: its own line under the waveform, where it reads as a caption on the timeline it describes (design 1b). Hidden until it has text, clipped to one line with the full text as a diff --git a/static/index.html b/static/index.html index f45b9466..97964e38 100644 --- a/static/index.html +++ b/static/index.html @@ -778,6 +778,7 @@ +