Skip to content

Skip sample rate conversion in Sample if the resampling ratio is 1#8262

Open
sakertooth wants to merge 1 commit intoLMMS:masterfrom
sakertooth:fix-sample-audio-quality
Open

Skip sample rate conversion in Sample if the resampling ratio is 1#8262
sakertooth wants to merge 1 commit intoLMMS:masterfrom
sakertooth:fix-sample-audio-quality

Conversation

@sakertooth
Copy link
Contributor

@sakertooth sakertooth commented Feb 17, 2026

This PR changes Sample to skip resampling entirely if the sample rates match.

@sakertooth sakertooth force-pushed the fix-sample-audio-quality branch from bab7fe6 to 318ca7f Compare February 17, 2026 00:45
@sakertooth sakertooth changed the title Convert sample rates in SamplePlayHandle and skip conversion when possible Skip sample rate conversion in Sample if the resampling ratio is 1x Feb 17, 2026
@sakertooth sakertooth changed the title Skip sample rate conversion in Sample if the resampling ratio is 1x Skip sample rate conversion in Sample if the resampling ratio is 1 Feb 17, 2026
if (finalRatio == 1.)
{
render(dst, numFrames, state, loop);
return true;
Copy link
Member

Choose a reason for hiding this comment

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

Should this return numFrames < Engine::audioEngine()->framesPerPeriod()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be checking the return value from render actually. Missed that.

@sakertooth
Copy link
Contributor Author

sakertooth commented Feb 17, 2026

This PR was originally because I thought there was a serious bug, but I realized its mostly fine. It feels wrong to just add a fast path in Sample and that's it.. The PR doesn't do much. It wouldn't hurt to add the fast path though.

@sakertooth
Copy link
Contributor Author

I was originally intending for this to fix a sound quality issue reported in the Discord but it didn't so I might investigate a bit more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants