Repository navigation
Set the count-in length, and accent a song in an odd meter - #594
Merged
Merged
Conversation
The count-in was one bar, fixed. At 120 BPM in 4/4 that is two seconds between pressing play and the song arriving, which is not enough time to get a hand back to an instrument. The reporter asked for more clicks. The count-in is now a length select on the transport: off, or 1 to 4 bars. Off is the zero case rather than a separate toggle, because once "how many bars" is a question, a toggle beside a length is two widgets for one setting. That replaces the press-to-arm button, so "armed" now reads as a non-zero value and the tint moves to the select's wrapper. Bars rather than a literal click count, which is what the request named. The count-in exists to land the song on a downbeat, and both implementations derive their clicks as countBars * beatsPerBar -- computeCountIn in static/js/metronome.js and count_in_beats in app/pipeline/click_render.py, which are held to each other beat-for-beat by tests. A raw click count would break that alignment in every meter. In 4/4 the bar lengths already give 4, 8, 12 and 16 clicks, which is the span the request was about. Exports follow the transport setting, so a file counts a player in for as long as playback does. The export query cap moves from 2 to 4 to match; it had been below what the UI can now ask for, which would have 422'd an export while playback counted in fine. The lead-in is count_in * beats_per_bar * beat_interval seconds of extra render and beats_per_bar is itself capped at 32, so the ceiling still bounds the allocation the way `end` is bounded by _validate_trim_range. The stored preference changes shape from a countIn boolean to countInBars. The old key is still read when the new one is absent, so an existing user keeps their count-in armed across the upgrade instead of silently losing it. Closes #587
The click offered 2/4, 3/4, 4/4 and 6/8 and nothing else, so a song in 5/4 or 7/8 could not be accented correctly. Auto was not a general answer either: it follows the bar marks the detector found, and on an odd-meter track those are the marks most likely to be wrong or missing. When they are missing Auto is disabled outright, leaving four presets and Off. Adds 5/4, 7/8, 9/8 and 12/8 as presets, and a "Custom..." option that reveals a number input for anything else. Nothing underneath needed changing, which is the point: beats_per_bar was already validated ge=1, le=32 in app/api/jobs.py, the beat-grid editor already exposed a free number input over the same range, computeCountIn and count_in_beats were already meter-agnostic, and the click render cache key already includes the bar signature so a custom value cannot serve a stale render. The <select> was the only thing narrowing the range. A typed value is clamped into 1..32 rather than rejected, and an unparseable one falls back to 4 rather than to Auto -- the user explicitly asked for a custom meter, so dropping them back to detection would be surprising. The free-entry box is kept outside .metro-select-wrap because that wrapper draws the select's caret with an ::after pinned to its right edge, which a nested sibling would sit under. The denominator stays cosmetic. "7/8" means seven beats per bar, not a subdivision change; there is no note-value concept anywhere in the app, and introducing one would reach click rate, count-in tempo and the render cache key. That is a larger change than this, and not what the gap was. Closes #593
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two changes to the click panel, one commit each.
Closes #587
Closes #593
Count-in length
The count-in was one bar, fixed. The reporter asked for more clicks: at 120 BPM in 4/4 there are two seconds between pressing play and the song arriving, which is not long enough to get a hand back to an instrument.
It is now a length select: off, or 1 to 4 bars. Off is the zero case rather than a separate toggle, since once "how many bars" is a question a toggle beside a length is two widgets for one setting. That replaces the press-to-arm button, so "armed" reads as a non-zero value and the tint moves to the select wrapper.
Bars, not a literal click count, which is what the request named. Both implementations derive clicks as
countBars * beatsPerBar--computeCountIninstatic/js/metronome.jsandcount_in_beatsinapp/pipeline/click_render.py, which are held to each other beat-for-beat by tests. A raw click count would break the downbeat alignment in every meter. In 4/4 the bar lengths already give 4, 8, 12 and 16 clicks, which is the span the request was about.Exports follow the transport setting. The export query cap moves from 2 to 4 to match -- it sat below what the UI can now ask for, so picking 3 or 4 would have 422d the export while playback counted in fine.
The stored preference changes from a
countInboolean tocountInBars. The old key is still read when the new one is absent, so an existing user keeps their count-in armed across the upgrade rather than silently losing it.Odd meters
The click offered 2/4, 3/4, 4/4 and 6/8 and nothing else, so 5/4 or 7/8 could not be accented correctly. Auto was not a general answer: it follows detected bar marks, which on an odd-meter track are the marks most likely to be wrong or missing, and when they are missing Auto is disabled outright.
Adds 5/4, 7/8, 9/8 and 12/8, plus a "Custom..." option revealing a number input for anything else.
Nothing underneath needed changing, which is the point.
beats_per_barwas already validatedge=1, le=32, the beat-grid editor already exposed a free number input over the same range, the count-in maths was already meter-agnostic, and the render cache key already includes the bar signature. The<select>was the only thing narrowing the range.A typed value is clamped into 1..32 rather than rejected. An unparseable one falls back to 4 rather than Auto, since the user explicitly asked for a custom meter.
The denominator stays cosmetic. "7/8" means seven beats per bar, not a subdivision change. There is no note-value concept in the app, and adding one would reach click rate, count-in tempo and the cache key. Out of scope here, and noted on #593.
Verification
All run locally on macOS arm64:
ruff check/ruff format --checkpytest tests/ -qnode --check static/js/*.jstests/js/*.test.mjsnpx playwright testbandit -r app/ -llbash -n run.sh.claude/rules/i18n.md)The two failures are
test_all_stems_zip_oggandtest_ogg_is_still_streamed. They fail on unmodifiedmainon this machine too -- the local Homebrew ffmpeg ships only the experimental nativevorbisencoder, notlibvorbis. Verified by checking outmainand re-running them.New coverage: 4 count-in lengths through the export API, lead-in arithmetic across lengths, 1-4 bar shapes and a 7/8 count-in in the JS parity test, and two e2e tests driving the real controls in a browser -- including that a typed 99 clamps to 32.
Both commits were verified green independently, so the intermediate commit is not broken.
Note the i18n rule says 8 language tables; there are now 10 (
esandkowere added since). New keys went into all of them.