Repository navigation
Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/ - #2635
Merged
Merged
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2635 +/- ##
==========================================
+ Coverage 31.96% 33.75% +1.78%
==========================================
Files 524 529 +5
Lines 27082 27664 +582
==========================================
+ Hits 8657 9337 +680
+ Misses 18425 18327 -98 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 22, 2026
Review feedback on PR GrandOrgue#2635 (github.com/GrandOrgue/pull/2635, this fork's PR #10) pointed out that GOSoundOnePoleFilter includes sound/buffer/GOSoundBufferMutable.h and exposes it in FilterState:: ProcessBuffer, which the previous "depend on nothing else in the sound engine" wording flatly contradicted. That dependency predates this PR (it was already there in sound/GOSoundOnePoleFilter.h) and is not something a pure file-move PR should be fixing by restructuring the class - buffer/ is itself a dependency-free leaf (a plain data wrapper, verified by its own includes), so depending on it does not actually put a framework or model dependency into dsp-kernels/. Documents that as the one allowed exception instead of overclaiming zero dependencies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 23, 2026
Review feedback on PR GrandOrgue#2635 (github.com/GrandOrgue/pull/2635, this fork's PR #10) pointed out that GOSoundOnePoleFilter includes sound/buffer/GOSoundBufferMutable.h and exposes it in FilterState:: ProcessBuffer, which the previous "depend on nothing else in the sound engine" wording flatly contradicted. That dependency predates this PR (it was already there in sound/GOSoundOnePoleFilter.h) and is not something a pure file-move PR should be fixing by restructuring the class - buffer/ is itself a dependency-free leaf (a plain data wrapper, verified by its own includes), so depending on it does not actually put a framework or model dependency into dsp-kernels/. Documents that as the one allowed exception instead of overclaiming zero dependencies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
force-pushed
the
refactor/sound-dsp-kernels
branch
2 times, most recently
from
September 23, 2026 17:41
da46dae to
02a34b0
Compare
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved GOSoundResample/GOSoundOnePoleFilter into sound/dsp-kernels/, but was branched from upstream/master, which lacks sound/effects/ and sound/mappers/ entirely - so it could not update the two consumers that exist only on this branch: GOSoundShelfFilterProcessor.h and GOTestSoundEnclosureShelfMapper.cpp. This is the remainder of that same include-path fix, scoped to what this branch actually has. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together they are exactly PR 1 as it applies here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved the test's target class into sound/dsp-kernels/ but had no test-file move to make, since this test does not exist on upstream/master yet - it was added later on this branch. This is that missing test-file move, done to match the source move exactly: mirrored path, plus the CMakeLists.txt and GOTestExe.cpp registration updates. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together with the preceding include-fix commit, they complete PR 1 as it applies to this branch. Build clean in build/current (Debug, GO_BUILD_TESTING=ON); ./bin/GOTestExe passes GOTestSoundOnePoleFilter, GOTestSoundShelfFilterProcessor and GOTestSoundEnclosureShelfMapper. The GOTestPerfSoundBufferMutable/ GOTestPerfSoundBufferPlanarMutable failures are pre-existing, unrelated Debug perf-baseline misses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) wrote the sound/ directory breakdown from upstream/master, which has neither directory - both exist only on this branch (GOSoundShelfFilterProcessor, GOSoundEnclosureShelfMapper). Filling in the two entries the cherry-picked text couldn't know about, and sharpening the placement rule to name them directly instead of the vaguer "model-aware layer above processing/" wording that stood in for them. Squash into the PR GrandOrgue#2635 cherry-pick commit once this branch is rebased past GrandOrgue#2635's merge, together with the two preceding commits - the three together are PR 1 as it applies to this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
@larspalo @rousseldenis Could you approve this pr? |
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved GOSoundResample/GOSoundOnePoleFilter into sound/dsp-kernels/, but was branched from upstream/master, which lacks sound/effects/ and sound/mappers/ entirely - so it could not update the two consumers that exist only on this branch: GOSoundShelfFilterProcessor.h and GOTestSoundEnclosureShelfMapper.cpp. This is the remainder of that same include-path fix, scoped to what this branch actually has. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together they are exactly PR 1 as it applies here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved the test's target class into sound/dsp-kernels/ but had no test-file move to make, since this test does not exist on upstream/master yet - it was added later on this branch. This is that missing test-file move, done to match the source move exactly: mirrored path, plus the CMakeLists.txt and GOTestExe.cpp registration updates. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together with the preceding include-fix commit, they complete PR 1 as it applies to this branch. Build clean in build/current (Debug, GO_BUILD_TESTING=ON); ./bin/GOTestExe passes GOTestSoundOnePoleFilter, GOTestSoundShelfFilterProcessor and GOTestSoundEnclosureShelfMapper. The GOTestPerfSoundBufferMutable/ GOTestPerfSoundBufferPlanarMutable failures are pre-existing, unrelated Debug perf-baseline misses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) wrote the sound/ directory breakdown from upstream/master, which has neither directory - both exist only on this branch (GOSoundShelfFilterProcessor, GOSoundEnclosureShelfMapper). Filling in the two entries the cherry-picked text couldn't know about, and sharpening the placement rule to name them directly instead of the vaguer "model-aware layer above processing/" wording that stood in for them. Squash into the PR GrandOrgue#2635 cherry-pick commit once this branch is rebased past GrandOrgue#2635's merge, together with the two preceding commits - the three together are PR 1 as it applies to this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved GOSoundResample/GOSoundOnePoleFilter into sound/dsp-kernels/, but was branched from upstream/master, which lacks sound/effects/ and sound/mappers/ entirely - so it could not update the two consumers that exist only on this branch: GOSoundShelfFilterProcessor.h and GOTestSoundEnclosureShelfMapper.cpp. This is the remainder of that same include-path fix, scoped to what this branch actually has. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together they are exactly PR 1 as it applies here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved the test's target class into sound/dsp-kernels/ but had no test-file move to make, since this test does not exist on upstream/master yet - it was added later on this branch. This is that missing test-file move, done to match the source move exactly: mirrored path, plus the CMakeLists.txt and GOTestExe.cpp registration updates. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together with the preceding include-fix commit, they complete PR 1 as it applies to this branch. Build clean in build/current (Debug, GO_BUILD_TESTING=ON); ./bin/GOTestExe passes GOTestSoundOnePoleFilter, GOTestSoundShelfFilterProcessor and GOTestSoundEnclosureShelfMapper. The GOTestPerfSoundBufferMutable/ GOTestPerfSoundBufferPlanarMutable failures are pre-existing, unrelated Debug perf-baseline misses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Sep 28, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) wrote the sound/ directory breakdown from upstream/master, which has neither directory - both exist only on this branch (GOSoundShelfFilterProcessor, GOSoundEnclosureShelfMapper). Filling in the two entries the cherry-picked text couldn't know about, and sharpening the placement rule to name them directly instead of the vaguer "model-aware layer above processing/" wording that stood in for them. Squash into the PR GrandOrgue#2635 cherry-pick commit once this branch is rebased past GrandOrgue#2635's merge, together with the two preceding commits - the three together are PR 1 as it applies to this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
@oleg68 You need to rebase |
They are stateless DSP primitives already consumed from three different areas (playing/, processing/'s future clients, and reverb/), but lived in sound/playing/ and bare sound/ respectively, misrepresenting them as playback- or engine-scoped rather than shared computational cores. This is a pure file move: give them a directory whose name states the actual dependency rule (depends on nothing, depended on by everything above it) before any more consumers accrete on the old, misleading locations. CLAUDE.md's Main Source Directories section is updated to describe the now-multi-part sound/ layout and the placement rule this move follows. The dsp-kernels/ directory rule is documented once in CLAUDE.md, so the moved headers carry a short doc comment about the class itself rather than restating the directory rule per-header, which would duplicate and drift as new consumers appear. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Oct 1, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved GOSoundResample/GOSoundOnePoleFilter into sound/dsp-kernels/, but was branched from upstream/master, which lacks sound/effects/ and sound/mappers/ entirely - so it could not update the two consumers that exist only on this branch: GOSoundShelfFilterProcessor.h and GOTestSoundEnclosureShelfMapper.cpp. This is the remainder of that same include-path fix, scoped to what this branch actually has. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together they are exactly PR 1 as it applies here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Oct 1, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved the test's target class into sound/dsp-kernels/ but had no test-file move to make, since this test does not exist on upstream/master yet - it was added later on this branch. This is that missing test-file move, done to match the source move exactly: mirrored path, plus the CMakeLists.txt and GOTestExe.cpp registration updates. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together with the preceding include-fix commit, they complete PR 1 as it applies to this branch. Build clean in build/current (Debug, GO_BUILD_TESTING=ON); ./bin/GOTestExe passes GOTestSoundOnePoleFilter, GOTestSoundShelfFilterProcessor and GOTestSoundEnclosureShelfMapper. The GOTestPerfSoundBufferMutable/ GOTestPerfSoundBufferPlanarMutable failures are pre-existing, unrelated Debug perf-baseline misses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Oct 1, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) wrote the sound/ directory breakdown from upstream/master, which has neither directory - both exist only on this branch (GOSoundShelfFilterProcessor, GOSoundEnclosureShelfMapper). Filling in the two entries the cherry-picked text couldn't know about, and sharpening the placement rule to name them directly instead of the vaguer "model-aware layer above processing/" wording that stood in for them. Squash into the PR GrandOrgue#2635 cherry-pick commit once this branch is rebased past GrandOrgue#2635's merge, together with the two preceding commits - the three together are PR 1 as it applies to this branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
force-pushed
the
refactor/sound-dsp-kernels
branch
from
October 1, 2026 13:57
02a34b0 to
7f4dd52
Compare
Contributor
Author
@rousseldenis Done |
Contributor
Author
|
@rousseldenis Could you approve this PR as soon as possible. It is jost simple: only moving files and changing of include directories. There are no code changes at all. |
rousseldenis
approved these changes
Oct 5, 2026
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Oct 7, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved GOSoundResample/GOSoundOnePoleFilter into sound/dsp-kernels/, but was branched from upstream/master, which lacks sound/effects/ and sound/mappers/ entirely - so it could not update the two consumers that exist only on this branch: GOSoundShelfFilterProcessor.h and GOTestSoundEnclosureShelfMapper.cpp. This is the remainder of that same include-path fix, scoped to what this branch actually has. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together they are exactly PR 1 as it applies here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
oleg68
pushed a commit
to oleg68/GrandOrgue-official
that referenced
this pull request
Oct 7, 2026
PR GrandOrgue#2635 (GrandOrgue#2635) moved the test's target class into sound/dsp-kernels/ but had no test-file move to make, since this test does not exist on upstream/master yet - it was added later on this branch. This is that missing test-file move, done to match the source move exactly: mirrored path, plus the CMakeLists.txt and GOTestExe.cpp registration updates. Squash into the PR GrandOrgue#2635 cherry-pick commit ("Moved GOSoundResample and GOSoundOnePoleFilter into sound/dsp-kernels/") once this branch is rebased past GrandOrgue#2635's merge - together with the preceding include-fix commit, they complete PR 1 as it applies to this branch. Build clean in build/current (Debug, GO_BUILD_TESTING=ON); ./bin/GOTestExe passes GOTestSoundOnePoleFilter, GOTestSoundShelfFilterProcessor and GOTestSoundEnclosureShelfMapper. The GOTestPerfSoundBufferMutable/ GOTestPerfSoundBufferPlanarMutable failures are pre-existing, unrelated Debug perf-baseline misses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Oct 7, 2026
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.
Summary
GOSoundResampleandGOSoundOnePoleFilterare stateless DSP primitives already consumed from multiple areas of the sound engine (playing/,reverb/) but lived in inconsistent locations (sound/playing/and baresound/).sound/dsp-kernels/directory that states the actual dependency rule: depends on nothing, depended on by everything above it.CMakeLists.txt, andCLAUDE.md's directory documentation accordingly.Test plan
build/current(Debug,GO_BUILD_TESTING=ON) withLANG=C make -k -j16./bin/GOTestExepasses (GOTestSoundOnePoleFilter... N/A upstream;GOTestSoundStreamand all other suites pass; the twoGOTestPerfSoundBufferMutable/GOTestPerfSoundBufferPlanarMutablefailures are pre-existing, unrelated Debug perf-baseline misses)git diffscoped to renames, include-path updates, build-file entries, and doc comments only🤖 Generated with Claude Code