AUDIO: Optimize case for !inStereo && outStereo

[mikrosk]

This is almost invisible on high-end machines but does it job nicely on lower-end ones (<100 MHz). Instead of calling clampedAdd with the same values just copy the value over.

As many (older) games use mono samples nearly always, it can be widely used, especially on platform which do not offer 16-bit mono output (yes, they exist ;)).

[coderabbitai]

Walkthrough

Walkthrough

The changes involve updates to the audio rate conversion logic, ensuring proper handling of stereo and mono channels during the conversion process. The RateConverter_Impl class methods now account for both input and output stereo status. Additionally, the clampedAdd function in the header file has been modified to return an int16 value instead of being a void function, which changes its signature and the way its result is used.

Changes

File Path	Change Summary
audio/rate.cpp	Updated RateConverter_Impl methods (copyConvert, simpleConvert, interpolateConvert) to handle stereo and mono channels correctly.
audio/rate.h	Changed clampedAdd function to return an int16 value, altering its signature and return type.

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[sev-]

I don't understand this change. You still call clampedAdd repeatedly.

Maybe you wanted to write something instead of:

		} else {
			// Output mono channel
			st_sample_t val = clampedAdd(outBuffer[0], (outL + outR) / 2);

			outBuffer += 1;

			if (outStereo)
				*outBuffer++ = val;
		}

something like

		} else {
			// Output mono channel
			if (outStereo) {
			      st_sample_t val = clampedAdd(outBuffer[0], (outL + outR) / 2);
                              outBuffer++;
                              *outBuffer++ = val;
                        } else {
			     outBuffer++;
                       }
		}

[mikrosk]

@sev- no, the change is correct:

• if inStereo && outStereo: call clampedAdd two times, for each channel
• if inStereo && !outStereo: call clampedAdd with the value of (outL+outR)/2 to the mono dst channel
• if !inStereo && !outStereo: call clampedAdd with the value of (outL+outR)/2 (where outL == outR) to the mono dst channel
• if !inStereo && outStereo: call clampedAdd with the value of (outL+outR)/2 (where outL == outR) to the left dst channel and additionally copy the value over to the right destination channel

The intended optimisation is the last case, currently it ends up in the first case, calling clampedAdd two times, even though outL and outR are the same value. Plus I have verified its intended functionality with mono samples and stereo playback.

Your proposed change wouldn't write anything in case of !outStereo unless I've overlooked something?

[sev-]

Okay, thank you for your explanation. Merging.

[fracturehill]

• if !inStereo && outStereo: call clampedAdd with the value of (outL+outR)/2 (where outL == outR) to the left dst channel and additionally copy the value over to the right destination channel

Unfortunately, I've had to revert this PR due to the logic quoted above. When mixing a mono sound into a stereo output, there's no reason to assume the stereo output's left and right channel are the same. If you blindly copy the value of the left channel over to the right, you're erasing any stereo information previously present in the sound you're mixing into, while also introducing subtle artifacts. Case in point: a user on the bugtracker caught this exact issue in Gobliiins 5.

[mikrosk]

@fracturehill oh dear, you are completely correct. It never occured to me that one can mix stereo and mono samples together! Apologies for the noise (pun intended ;)) and kudos to the reporter.