From 06b1c86c3a58edaec2bb36f5b93ce667549e9b9c Mon Sep 17 00:00:00 2001 From: "5684185+vsariola@users.noreply.github.com" <5684185+vsariola@users.noreply.github.com> Date: Sat, 15 Aug 2026 13:49:40 +0300 Subject: [PATCH] fix(vm/compiler)!: don't define SU_BUFFER_LENGTH when speed is used BREAKING CHANGE: If song uses a speed unit, SU_LENGTH_IN_SAMPLES, SU_BUFFER_LENGTH and SU_SYNCBUFFER_LENGTH won't be defined in the generated header file, as we cannot know their exact values during compile time without rendering the entire song. When using a speed unit, the user has to take responsibility of allocating large enough buffers. Closes #241 --- CHANGELOG.md | 9 +++++++++ cmd/sointu-compile/main.go | 6 +++++- tests/test_renderer.c | 6 ++++++ vm/compiler/compiler.go | 17 ++++++++++------- vm/compiler/templates/amd64-386/player.h | 14 +++++++++++++- 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec8dc7b..41a29b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [Unreleased] +### BREAKING CHANGES +- BREAKING CHANGE: If song uses a speed unit, SU_LENGTH_IN_SAMPLES, + SU_BUFFER_LENGTH and SU_SYNCBUFFER_LENGTH won't be defined in the generated + header file, as we cannot know their exact values during compile time without + rendering the entire song. When using a speed unit, the user has to take + responsibility of allocating large enough buffers. ([#241][i241]) + ## [0.6.0] ### Added - Binary builds for sointu-play from GitHub Actions on all platforms. @@ -441,3 +449,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). [i222]: https://github.com/vsariola/sointu/issues/222 [i226]: https://github.com/vsariola/sointu/issues/226 [i227]: https://github.com/vsariola/sointu/issues/227 +[i241]: https://github.com/vsariola/sointu/issues/241 diff --git a/cmd/sointu-compile/main.go b/cmd/sointu-compile/main.go index 90480d0..21c69c1 100644 --- a/cmd/sointu-compile/main.go +++ b/cmd/sointu-compile/main.go @@ -143,10 +143,14 @@ func main() { var compiledPlayer map[string]string if compile { var err error - compiledPlayer, err = comp.Song(&song) + var warnings []string + compiledPlayer, warnings, err = comp.Song(&song) if err != nil { return fmt.Errorf("compiling player failed: %v", err) } + for _, warning := range warnings { + fmt.Fprintf(os.Stderr, "warning: %v\n", warning) + } if len(*extensionsOut) > 0 { compiledPlayer = filterExtensions(compiledPlayer, strings.Split(*extensionsOut, ",")) } diff --git a/tests/test_renderer.c b/tests/test_renderer.c index d538989..b59f9b5 100644 --- a/tests/test_renderer.c +++ b/tests/test_renderer.c @@ -14,6 +14,12 @@ #include #include TEST_HEADER +#ifndef SU_BUFFER_LENGTH +// SU_BUFFER_LENGTH is not defined, likely because user is using the speed unit. +// But we will make the tests so that they don't use any more than this, so the +// usual size is fine. +#define SU_BUFFER_LENGTH SU_SAMPLES_PER_ROW*SU_LENGTH_IN_ROWS*SU_CHANNEL_COUNT +#endif SUsample buf[SU_BUFFER_LENGTH]; SUsample filebuf[SU_BUFFER_LENGTH]; #ifdef SU_SYNC diff --git a/vm/compiler/compiler.go b/vm/compiler/compiler.go index f6a8faf..22eda26 100644 --- a/vm/compiler/compiler.go +++ b/vm/compiler/compiler.go @@ -75,9 +75,9 @@ func (com *Compiler) Library() (map[string]string, error) { return retmap, nil } -func (com *Compiler) Song(song *sointu.Song) (map[string]string, error) { +func (com *Compiler) Song(song *sointu.Song) (retmap map[string]string, warnings []string, err error) { if com.Arch != "386" && com.Arch != "amd64" && com.Arch != "wasm" { - return nil, fmt.Errorf(`compiling a song player is supported only on 386, amd64 and wasm architectures (targeted architecture was %v)`, com.Arch) + return nil, nil, fmt.Errorf(`compiling a song player is supported only on 386, amd64 and wasm architectures (targeted architecture was %v)`, com.Arch) } var templates []string if com.Arch == "386" || com.Arch == "amd64" { @@ -86,14 +86,17 @@ func (com *Compiler) Song(song *sointu.Song) (map[string]string, error) { templates = []string{"player.wat"} } features := vm.NecessaryFeaturesFor(song.Patch) - retmap := map[string]string{} + if _, ok := features.Opcode("speed"); ok { + warnings = append(warnings, fmt.Sprintf(`song uses the speed unit, so SU_LENGTH_IN_SAMPLES, SU_BUFFER_LENGTH, and SU_SYNCBUFFER_LENGTH cannot be known without rendering the entire song. They won't be defined in the generated header file. You have to take responsibility for allocating large enough audio buffer and syncBuf.`)) + } + retmap = map[string]string{} encodedPatch, err := vm.NewBytecode(song.Patch, features, song.BPM) if err != nil { - return nil, fmt.Errorf(`could not encode patch: %v`, err) + return nil, nil, fmt.Errorf(`could not encode patch: %v`, err) } patterns, sequences, err := ConstructPatterns(song) if err != nil { - return nil, fmt.Errorf(`could not encode song: %v`, err) + return nil, nil, fmt.Errorf(`could not encode song: %v`, err) } for _, templateName := range templates { compilerMacros := *NewCompilerMacros(*com) @@ -133,11 +136,11 @@ func (com *Compiler) Song(song *sointu.Song) (map[string]string, error) { populatedTemplate, extension, err = com.compile(templateName, &data) } if err != nil { - return nil, fmt.Errorf(`could not execute template "%v": %v`, templateName, err) + return nil, nil, fmt.Errorf(`could not execute template "%v": %v`, templateName, err) } retmap[extension] = populatedTemplate } - return retmap, nil + return retmap, warnings, nil } func (com *Compiler) compile(templateName string, data interface{}) (string, string, error) { diff --git a/vm/compiler/templates/amd64-386/player.h b/vm/compiler/templates/amd64-386/player.h index 35de63f..60107b3 100644 --- a/vm/compiler/templates/amd64-386/player.h +++ b/vm/compiler/templates/amd64-386/player.h @@ -3,15 +3,22 @@ #define SU_RENDER_H #define SU_CHANNEL_COUNT 2 +{{- if .HasOp "speed" -}} +// warning: song uses speed unit, so SU_LENGTH_IN_SAMPLES, SU_BUFFER_LENGTH, and +// SU_SYNCBUFFER_LENGTH cannot be known without rendering the entire song. They +// are not defined in this generated header file. You have to take +// responsibility for allocating large enough audio buffer and syncBuf. +{{- else}} #define SU_LENGTH_IN_SAMPLES {{.MaxSamples}} #define SU_BUFFER_LENGTH (SU_LENGTH_IN_SAMPLES*SU_CHANNEL_COUNT) +{{- end}} #define SU_SAMPLE_RATE 44100 #define SU_BPM {{.Song.BPM}} #define SU_ROWS_PER_BEAT {{.Song.RowsPerBeat}} #define SU_ROWS_PER_PATTERN {{.Song.Score.RowsPerPattern}} #define SU_LENGTH_IN_PATTERNS {{.Song.Score.Length}} -#define SU_LENGTH_IN_ROWS (SU_LENGTH_IN_PATTERNS*SU_PATTERN_SIZE) +#define SU_LENGTH_IN_ROWS (SU_LENGTH_IN_PATTERNS*SU_ROWS_PER_PATTERN) #define SU_SAMPLES_PER_ROW (SU_SAMPLE_RATE*60/(SU_BPM*SU_ROWS_PER_BEAT)) {{- if or .RowSync (.HasOp "sync")}} @@ -20,8 +27,13 @@ {{- else}} #define SU_NUMSYNCS {{.Song.Patch.NumSyncs}} {{- end}} +{{- if .HasOp "speed" -}} +// Patch uses the speed unit, cannot safely define SU_SYNCBUFFER_LENGTH here. +// Normally it would be round_up(SU_LENGTH_IN_SAMPLES/256)*SU_NUMSYNCS. +{{- else}} #define SU_SYNCBUFFER_LENGTH ((SU_LENGTH_IN_SAMPLES+255)>>8)*SU_NUMSYNCS {{- end}} +{{- end}} #include #if UINTPTR_MAX == 0xffffffff