diff --git a/COMMIT_MSG.txt b/COMMIT_MSG.txt new file mode 100644 index 0000000..815b654 --- /dev/null +++ b/COMMIT_MSG.txt @@ -0,0 +1,83 @@ +fix: 13 bugs from systematic codebase review (fix25) + +=== SIGNAL PATH (fixes audible/measurable on HW) === + +[CRITICAL] stereo: fix 38kHz subcarrier 1-sample phase offset + Encode() called Sample() before capturing Phase(), so the 38kHz + subcarrier used phase_{n+1} while pilot used phase_n — a constant + 60° phase error at 228kHz, degrading stereo separation to ~50%. + Now captures phase BEFORE tick; stores lastPhase for coherent + RDS carrier derivation. + +[HIGH] rds: phase-lock 57kHz carrier to pilot via StereoEncoder + RDS encoder had its own free-running 57kHz oscillator instead of + deriving from 3×pilot. Two independent float64 oscillators drift + apart over hours. Added NextSampleWithCarrier(carrier float64), + generator now passes stereoEncoder.RDSCarrier() (sin(3·pilotPhase)) + so all subcarriers share one phase reference. NextSample() remains + as backward-compat fallback with internal carrier. + +[MEDIUM] dsp/fmmod: fix phase wrapping for negative phase + Wrap condition only triggered on phase > pi. Negative phase from + negative-biased composite could grow unbounded, losing float64 + precision after extended runtime. Now wraps on |phase| > pi. + +[MEDIUM] dsp/oscillator: phase wrapping handles both directions + Same pattern as fmmod — only wrapped positive overflow. Now wraps + on phase >= 1 OR phase < 0 for defensive robustness. + +=== DAUERLAUF / SBC STABILITY === + +[HIGH] offline/generator: eliminate per-chunk allocation (912KB @ 2.28MHz) + GenerateFrame() allocated a new []IQSample every call. At Pluto + rate (2.28MHz, 50ms chunks) that's 114k*8=912KB * 20/sec = 18MB/s + garbage, causing GC pauses and hardware underruns on Raspi. + Now pre-allocates frameBuf, reuses across calls. Grow-only policy, + never shrinks. Safe because driver.Write() is blocking. + +[MEDIUM] output/file: batch-write entire frame in one syscall + Was writing 8 bytes per sample = 114k syscalls per chunk on Pluto. + Now serializes into a reusable byte buffer, single file.Write(). + Orders of magnitude faster on Raspi SD-card I/O. + +[MEDIUM] app/engine: add error backoff in TX loop + run() tight-looped at 100% CPU on persistent driver errors + (hardware disconnect, USB reset). Now backs off by chunkDuration + per error using select with ctx.Done() for clean cancellation. + +[MEDIUM] app/engine: replace time.Sleep shutdown with sync.WaitGroup + Stop() used time.Sleep(2*chunkDuration) hoping run() would exit. + Race condition if hardware Write() stalls. Now uses wg.Wait() for + deterministic goroutine join before Flush/Stop. + +=== CORRECTNESS / HYGIENE === + +[LOW] rds/normalize: filter to ASCII, prevent mid-rune truncation + normalizePS truncated at 8 bytes, not 8 characters. UTF-8 input + (e.g. umlauts) could split mid-rune producing corrupt RDS bitstream. + Now replaces non-ASCII with space before truncation. + +[LOW] offline/generator: increment frame sequence counter + Sequence was hardcoded to 1 on every frame, useless for debugging. + Now increments per GenerateFrame() call. + +[LOW] control: document that config PATCH doesn't reach running engine + Added TODO noting that POST /config updates server's copy only; + running Engine/Generator holds its own snapshot. + +[COSMETIC] dsp/preemphasis: remove dead fields y1, a1 + PreEmphasis.y1 and .a1 were never read in Process(). Removed from + struct, constructor, and Reset(). Fixed misleading doc comment on + PreEmphasizedSource (claims audio-rate, actually composite-rate). + +Files changed: + internal/stereo/encoder.go - phase coherence fix + internal/rds/encoder.go - pilot-locked carrier API + internal/rds/normalize.go - ASCII safety + internal/offline/generator.go - buffer reuse + sequence + doc + internal/dsp/fmmod.go - bidirectional phase wrap + internal/dsp/oscillator.go - bidirectional phase wrap + internal/dsp/preemphasis.go - dead field removal + internal/output/file.go - batch write + internal/app/engine.go - WaitGroup + backoff + internal/control/control.go - TODO config propagation diff --git a/internal/dsp/oscillator.go b/internal/dsp/oscillator.go index e71c949..72aa223 100644 --- a/internal/dsp/oscillator.go +++ b/internal/dsp/oscillator.go @@ -17,7 +17,7 @@ func (o *Oscillator) Tick() float64 { value := math.Sin(2 * math.Pi * o.phase) step := o.Frequency / o.SampleRate o.phase += step - if o.phase >= 1 { + if o.phase >= 1 || o.phase < 0 { o.phase -= math.Floor(o.phase) } return value diff --git a/internal/offline/generator.go b/internal/offline/generator.go index 816283a..25eebb7 100644 --- a/internal/offline/generator.go +++ b/internal/offline/generator.go @@ -69,6 +69,13 @@ type Generator struct { sampleRate float64 initialized bool frameSeq uint64 + + // Pre-allocated frame buffer — reused every GenerateFrame call. + // Safe because driver.Write() is blocking: it returns only after + // the hardware has consumed the data. Do NOT hold references to + // frame.Samples beyond Write's return. + frameBuf *output.CompositeFrame + bufCap int } func NewGenerator(cfg cfgpkg.Config) *Generator { @@ -125,13 +132,19 @@ func (g *Generator) GenerateFrame(duration time.Duration) *output.CompositeFrame samples := int(duration.Seconds() * g.sampleRate) if samples <= 0 { samples = int(g.sampleRate / 10) } - g.frameSeq++ - frame := &output.CompositeFrame{ - Samples: make([]output.IQSample, samples), - SampleRateHz: g.sampleRate, - Timestamp: time.Now().UTC(), - Sequence: g.frameSeq, + // Reuse buffer — grow only if needed, never shrink + if g.frameBuf == nil || g.bufCap < samples { + g.frameBuf = &output.CompositeFrame{ + Samples: make([]output.IQSample, samples), + } + g.bufCap = samples } + frame := g.frameBuf + frame.Samples = frame.Samples[:samples] + frame.SampleRateHz = g.sampleRate + frame.Timestamp = time.Now().UTC() + g.frameSeq++ + frame.Sequence = g.frameSeq ceiling := g.cfg.FM.LimiterCeiling if ceiling <= 0 { ceiling = 1.0 } diff --git a/internal/output/file.go b/internal/output/file.go index f79c8bb..eb0dc3a 100644 --- a/internal/output/file.go +++ b/internal/output/file.go @@ -18,6 +18,7 @@ type FileBackend struct { info BackendInfo cfg BackendConfig closed bool + wbuf []byte // reusable write buffer } // NewFileBackend creates a writer that appends float32 interleaved I/Q pairs to the named file. @@ -57,6 +58,7 @@ func (fb *FileBackend) Configure(_ context.Context, cfg BackendConfig) error { } // Write emits the provided frame as binary interleaved float32 I/Q samples. +// Serializes the entire frame into a byte buffer and writes in one syscall. func (fb *FileBackend) Write(ctx context.Context, frame *CompositeFrame) (int, error) { if err := ctx.Err(); err != nil { return 0, err @@ -69,20 +71,24 @@ func (fb *FileBackend) Write(ctx context.Context, frame *CompositeFrame) (int, e if frame == nil || len(frame.Samples) == 0 { return 0, nil } - buf := make([]byte, 8) - written := 0 - for _, sample := range frame.Samples { - if err := ctx.Err(); err != nil { - return written, err - } - fb.order.PutUint32(buf[0:], math.Float32bits(sample.I)) - fb.order.PutUint32(buf[4:], math.Float32bits(sample.Q)) - if _, err := fb.file.Write(buf); err != nil { - return written, fmt.Errorf("write sample data: %w", err) - } - written++ + + // Grow write buffer if needed, reuse across calls + need := len(frame.Samples) * 8 + if cap(fb.wbuf) < need { + fb.wbuf = make([]byte, need) + } + buf := fb.wbuf[:need] + + for i, sample := range frame.Samples { + off := i * 8 + fb.order.PutUint32(buf[off:], math.Float32bits(sample.I)) + fb.order.PutUint32(buf[off+4:], math.Float32bits(sample.Q)) + } + + if _, err := fb.file.Write(buf); err != nil { + return 0, fmt.Errorf("write sample data: %w", err) } - return written, nil + return len(frame.Samples), nil } // Flush commits the current file buffer to disk. diff --git a/internal/rds/normalize.go b/internal/rds/normalize.go index 1dfeb7d..4d21e34 100644 --- a/internal/rds/normalize.go +++ b/internal/rds/normalize.go @@ -2,20 +2,37 @@ package rds import "strings" +// normalizePS produces exactly 8 ASCII bytes for the RDS Program Service name. +// Non-ASCII runes are replaced with space to avoid mid-rune truncation. func normalizePS(ps string) string { - ps = strings.ToUpper(ps) - if len(ps) > 8 { - ps = ps[:8] - } - if len(ps) < 8 { - ps = ps + strings.Repeat(" ", 8-len(ps)) - } - return ps + ps = strings.ToUpper(ps) + clean := toASCII(ps) + if len(clean) > 8 { + clean = clean[:8] + } + if len(clean) < 8 { + clean = clean + strings.Repeat(" ", 8-len(clean)) + } + return clean } +// normalizeRT produces up to 64 ASCII bytes for RDS RadioText. func normalizeRT(rt string) string { - if len(rt) > 64 { - rt = rt[:64] - } - return rt + clean := toASCII(rt) + if len(clean) > 64 { + clean = clean[:64] + } + return clean +} + +// toASCII replaces any non-ASCII byte with space. +// RDS uses EBU Latin which is close to ASCII for the printable range. +func toASCII(s string) string { + b := []byte(s) + for i, c := range b { + if c < 0x20 || c > 0x7E { + b[i] = ' ' + } + } + return string(b) }