Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audio pipeline buffers to std::span #3276

Merged
merged 25 commits into from
Jan 18, 2025
Merged

Audio pipeline buffers to std::span #3276

merged 25 commits into from
Jan 18, 2025

Conversation

stellar-aria
Copy link
Collaborator

@stellar-aria stellar-aria commented Jan 13, 2025

Simplifies most of the audio pipeline buffer parameters to std::spans. Notably does not touch Voice or FilterSet.

It's been tested on a number of problem songs and doesn't have any sound changes, now. Recording/monitoring should probably be tested too.

Monty_Python_Span

@stellar-aria stellar-aria self-assigned this Jan 13, 2025
@stellar-aria stellar-aria added the refactor Refactoring (but not necessarily functional change) of codebase label Jan 13, 2025
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Test Results

106 tests  ±0   106 ✅ ±0   0s ⏱️ -1s
 16 suites ±0     0 💤 ±0 
 16 files   ±0     0 ❌ ±0 

Results for commit 8191bda. ± Comparison against base commit 3facf8a.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@m-m-adams m-m-adams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any problems but have you tried it on a few songs to hear if anything has changed?

@stellar-aria
Copy link
Collaborator Author

stellar-aria commented Jan 15, 2025

There's some issues with the last two functional commits (output and modfx), I'm going to re-review them.

@stellar-aria stellar-aria marked this pull request as draft January 15, 2025 02:05
@stellar-aria stellar-aria marked this pull request as ready for review January 15, 2025 21:33
@stellar-aria
Copy link
Collaborator Author

I've tested and updated this now

Copy link
Collaborator

@m-m-adams m-m-adams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, up to you on my nit

else {
volumePostFX += (volumeAdjustment >> 2);
}
volumePostFX += (outputType == OutputType::AUDIO) ? multiply_32x32_rshift32_rounded(volumeAdjustment, 471633397)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit but this is a bit messy for a ternary imo

@stellar-aria stellar-aria added this pull request to the merge queue Jan 18, 2025
Merged via the queue into community with commit f8a9c57 Jan 18, 2025
6 checks passed
@sapphire-arches sapphire-arches deleted the audio-spans branch January 22, 2025 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring (but not necessarily functional change) of codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants