Skip to content
This repository was archived by the owner on Apr 29, 2026. It is now read-only.

Phase C scaffolding is in place. Summary:#441

Merged
navicore merged 2 commits into
mainfrom
csound-phase-c
Apr 28, 2026
Merged

Phase C scaffolding is in place. Summary:#441
navicore merged 2 commits into
mainfrom
csound-phase-c

Conversation

@navicore

Copy link
Copy Markdown
Owner

live.csd — Csound orchestra. OSCinit 7770 opens the listener, OSClisten
polls in a tight kgoto loop, and matched /kick freq messages schedule
instr 2 (a 0.4 s pitched tone with expon decay envelope). Held-open with
i 1 0 -1.

tone.seq — one-shot driver. Binds, sends one /kick 220.0 to
127.0.0.1:7770, closes, exits. The simplest possible audible test.

live.seq — 8-beat metronome. bpm-ms and beats are word-form constants
the user edits between runs; tail-recursive send-n-kicks threads the
socket through. 120 BPM × 8 = ~4 s of audio.

README.md — install, run-listener, run-driver, troubleshooting, mapping
back to the design doc's checkpoints 3–5.

CI now builds both tone and live binaries via build-examples. Audio
verification stays manual (you brew install csound, run it locally) —
that's checkpoints 3 & 4. Checkpoint 5 (the spinout question) waits on
you actually playing with it and writing up what you learned.

  live.csd — Csound orchestra. OSCinit 7770 opens the listener, OSClisten
  polls in a tight kgoto loop, and matched /kick freq messages schedule
  instr 2 (a 0.4 s pitched tone with expon decay envelope). Held-open with
   i 1 0 -1.

  tone.seq — one-shot driver. Binds, sends one /kick 220.0 to
  127.0.0.1:7770, closes, exits. The simplest possible audible test.

  live.seq — 8-beat metronome. bpm-ms and beats are word-form constants
  the user edits between runs; tail-recursive send-n-kicks threads the
  socket through. 120 BPM × 8 = ~4 s of audio.

  README.md — install, run-listener, run-driver, troubleshooting, mapping
  back to the design doc's checkpoints 3–5.

  CI now builds both tone and live binaries via build-examples. Audio
  verification stays manual (you brew install csound, run it locally) —
  that's checkpoints 3 & 4. Checkpoint 5 (the spinout question) waits on
  you actually playing with it and writing up what you learned.
@claude

claude Bot commented Apr 28, 2026

Copy link
Copy Markdown

Code Review — PR #441: Phase C Scaffolding (Seq → OSC → Csound)

Overview

This PR adds four files that complete the audible portion of the live-coding POC: a Csound orchestra (live.csd), a one-shot driver (tone.seq), an 8-beat metronome (live.seq), and their README. The implementation is clean and the PR description accurately describes what's here.


Observations

live.csd — Csound orchestra

Redundant -odac flag.
-odac appears in both <CsOptions> and every usage example in the README and the file's own header comment. Csound handles duplicate flags gracefully, but it creates a small confusion: readers who follow the README will silently pass it twice. Recommend either:

  • Remove -odac from <CsOptions> and keep it CLI-only (more explicit, easier to swap backends), or
  • Keep it in <CsOptions> and drop it from the README / usage comment (zero-arg invocation: csound live.csd).

Busy-loop polling in instr 1.
The kgoto loop construct polls OSClisten at full k-rate (~689 Hz at ksmps=64). This is the idiomatic Csound OSC pattern, but it pins a CPU core indefinitely. For a short-lived POC this is fine; worth a one-line comment acknowledging the trade-off so a future reader doesn't mistake it for a bug.

expon end-value.
expon 0.5, p3, 0.001 is correct — both values are non-zero and same-sign, so Csound won't error. No issue, just noting it's intentional.

tone.seq — one-shot driver

>aux/aux> vs. dup for socket preservation.
tone.seq stores the socket in the aux register to recover it after udp.send-to consumes it. live.seq's send-kick does the same thing with a stack dup. Both are correct, but using dup (as in live.seq) is more idiomatic for stack-based languages and avoids the implicit side-channel of the aux register. Consider rewriting tone.seq to use the dup pattern for consistency:

0 udp.bind drop drop          # ( socket )
dup                           # ( socket socket )
"/kick" 220.0 osc.msg-f       # ( socket socket msg )
swap                          # ( socket msg socket )
"127.0.0.1" swap 7770 swap    # ( socket msg "127.0.0.1" 7770 socket )
udp.send-to drop              # ( socket )
udp.close drop
0

live.seq — 8-beat metronome

Timing comment inconsistency.
The module-level comment says "~4 seconds" (correct: 8 × 500 ms = 4 000 ms, since send-kick sleeps after the last beat too). README §4 says "~3.5 seconds", which is off by half a beat. Small, but easy to fix.

Hardcoded port / host in two files.
7770 and "127.0.0.1" appear independently in both tone.seq and live.seq. The README troubleshooting section correctly warns users to update both places. For a POC this is fine, but a shared constant in osc.seq (or a separate config.seq) would make the edit-build loop smoother once users start experimenting.

README.md

Well-structured. The checkpoint mapping at the bottom is especially useful for orienting new readers against the design doc.

One small nit: the troubleshooting entry for "Address already in use" recommends changing the port in tone.seq / live.seq but doesn't mention live.csd's OSCinit 7770 line, which also needs to match.


Summary

Severity Item
Minor Redundant -odac in CsOptions vs. README/usage comment
Minor "~3.5 seconds" in README §4 should be "~4 seconds"
Style tone.seq uses >aux/aux>; live.seq uses dup — pick one pattern
Nit Port-change troubleshooting omits live.csd's OSCinit line
Info Busy-loop polling is expected; a comment acknowledging the trade-off would help readers

No blocking issues. The scaffolding correctly delivers checkpoints 3 & 4, the tests run in CI, and the manual-only audio verification is the right call. Good to merge once the minor doc inconsistencies are tidied up.

@claude

claude Bot commented Apr 28, 2026

Copy link
Copy Markdown

Code Review — Phase C (live-coding-csound POC)

Overview

Phase C completes the Seq→OSC→Csound chain with three new files (live.csd, tone.seq, live.seq) plus a README and a thorough design-doc writeup. The encoder layer (osc.seq) was established in a prior phase and is not modified here. CI integration is correct — build-examples auto-discovers .seq files with a main word, so no justfile change was needed.

Overall the work is solid. The notes below are mostly minor polish items, not blockers.


Findings

-odac is specified twice for most users

live.csd's <CsOptions> block already embeds -odac:

<CsOptions>
-odac
</CsOptions>

The README then instructs users to run:

csound -odac examples/projects/live-coding-csound/live.csd

That passes -odac twice. Csound tolerates duplicates silently, so it's not a bug, but it's confusing. Suggestion: remove -odac from <CsOptions> and keep it only in the README command line. This also gives users flexibility to swap -odac for -o output.wav without editing the .csd.


Inconsistent socket-juggling style between tone.seq and live.seq

tone.seq stashes the socket in aux before the udp.send-to call:

dup >aux                                 # ; aux: [socket]
"127.0.0.1" swap
7770 swap
udp.send-to drop
aux> udp.close drop

send-kick in live.seq does the same job with pure stack shuffling:

dup
"/kick" 220.0 osc.msg-f
swap
"127.0.0.1" swap
7770 swap
udp.send-to drop

Both are correct; both are the language's natural idiom. The inconsistency between the two files in the same POC is worth resolving for readability. The >aux approach is arguably cleaner when the socket handle is being threaded past multiple arguments — applying it to send-kick would trim a swap and make the intent clearer.


Pitch is hardcoded in send-kick but not listed as a knob

The README says:

The constants below are the live-codable knobs: change them and re-run

…and lists bpm-ms and beats. But 220.0 in send-kick is equally live-codable and equally undocumented as such. A one-liner kick-hz constant alongside bpm-ms/beats would complete the picture:

: kick-hz ( -- Float ) 220.0 ;

Minor, but consistent with the stated "live-coding loop" use-case in the README.


udp.bind errors are silently dropped

Both tone.seq and live.seq call 0 udp.bind drop drop, discarding both the assigned port and the success flag. For a POC this is fine — bind to port 0 on loopback is essentially infallible. Worth a one-line comment on why error-checking is skipped (# port=0 bind on loopback; failure here is a hard crash anyway) if this pattern gets copied into production use.


What's working well

  • Stack comments are excellent throughout. Every word has accurate ( before -- after ) annotations and the inline # ( ... ) state comments make the stack evolution easy to follow.
  • POC outcomes section in the design doc is unusually thorough — the language-gap catalogue (items 1–6) is exactly what makes a POC valuable beyond its immediate deliverable.
  • CI boundary is right. Encoder/loopback tests run automatically; audible verification is manual with clear instructions. The README troubleshooting section covers the common failure modes.
  • Tail recursion in send-n-kicks is idiomatic given the language's explicit rejection of locals. The recursive pattern is clean and the base-case guard (over 0 i.<=) handles the zero-beats edge case correctly.
  • live.csd k-rate comment (~689 Hz, far above the rate at which we send messages) is a good data point for future readers who might wonder whether OSClisten can miss events.

Summary

No correctness issues. The -odac duplication is the sharpest edge — it's worth a one-line fix in live.csd before anyone tries to record audio to a file. The kick-hz constant and style consistency between the two .seq files are nice-to-haves. The POC writeup is detailed and the spinout-recommendation deferral is the right call.

@navicore navicore merged commit 0e99882 into main Apr 28, 2026
2 of 3 checks passed
@navicore navicore deleted the csound-phase-c branch April 28, 2026 01:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant