[Scummvm-git-logs] scummvm master -> af58433037e43ca06c2198e04544b16ea3a96b1a
mikrosk
noreply at scummvm.org
Mon Jul 20 11:23:24 UTC 2026
This automated email contains information about 4 new commits which have been
pushed to the 'scummvm' repo located at https://api.github.com/repos/scummvm/scummvm .
Summary:
a4a41aa8bf AUDIO: Extract writeFrame() from RateConverter's commonConvert
58fabd3053 AUDIO: Fix infinite loop in RateConverter on a partial upsample group
43c2267456 TEST: Add RateConverter regression tests
af58433037 TEST: Add another RateConverter test
Commit: a4a41aa8bf8db656fdda8fa646dac3c9a9190623
https://github.com/scummvm/scummvm/commit/a4a41aa8bf8db656fdda8fa646dac3c9a9190623
Author: yasuno45 (yasuno45 at gmail.com)
Date: 2026-07-20T21:23:18+10:00
Commit Message:
AUDIO: Extract writeFrame() from RateConverter's commonConvert
No functional change.
Changed paths:
audio/rate.cpp
diff --git a/audio/rate.cpp b/audio/rate.cpp
index eac9fb350c3..e6036a54b2d 100644
--- a/audio/rate.cpp
+++ b/audio/rate.cpp
@@ -80,6 +80,10 @@ private:
/** Current sample(s) in the input stream (left/right channel) */
int16 _inCurL, _inCurR;
+ /** Write one output frame built from a single input frame, and advance outBuffer. */
+ template<st_volume_t volL, st_volume_t volR, typename st_sample_t, MixMode mixMode>
+ FORCEINLINE void writeFrame(st_sample_t *&outBuffer, int16 inL, int16 inR, st_volume_t volL_val, st_volume_t volR_val);
+
template<st_volume_t volL, st_volume_t volR, typename st_sample_t, MixMode mixMode>
int commonConvert(AudioStream &input, st_sample_t *outBuffer, st_size_t numSamples, st_volume_t volL_val, st_volume_t volR_val, int outputSamples);
@@ -132,6 +136,49 @@ public:
bool needsDraining() const override { return _bufferSize != 0; }
};
+template<bool inStereo, bool outStereo, bool reverseStereo>
+template<st_volume_t volL, st_volume_t volR, typename st_sample_t, MixMode mixMode>
+void RateConverter_Impl<inStereo, outStereo, reverseStereo>::writeFrame(st_sample_t *&outBuffer, int16 inL, int16 inR, st_volume_t volL_val, st_volume_t volR_val) {
+ if (volL | volR) {
+ st_sample_t outL, outR;
+
+ if (volL != 0) {
+ if (volL != Audio::Mixer::kMaxMixerVolume)
+ outL = (inL * (int)volL_val) / Audio::Mixer::kMaxMixerVolume;
+ else
+ outL = inL;
+ }
+
+ if (volR != 0) {
+ if (volR != Audio::Mixer::kMaxMixerVolume)
+ outR = (inR * (int)volR_val) / Audio::Mixer::kMaxMixerVolume;
+ else
+ outR = inR;
+ }
+
+ if (outStereo) {
+ // Output left channel
+ if (volL != 0)
+ processSample<mixMode>(outBuffer[reverseStereo ], outL);
+
+ // Output right channel
+ if (volR != 0)
+ processSample<mixMode>(outBuffer[reverseStereo ^ 1], outR);
+ } else {
+ // Output mono channel
+ st_sample_t monoOut;
+ if (volL != 0 && volR != 0)
+ monoOut = (outL + outR) / 2;
+ else if (volL != 0)
+ monoOut = outL / 2;
+ else if (volR != 0)
+ monoOut = outR / 2;
+ processSample<mixMode>(outBuffer[0], monoOut);
+ }
+ }
+ outBuffer += (outStereo ? 2 : 1);
+}
+
template<bool inStereo, bool outStereo, bool reverseStereo>
template<st_volume_t volL, st_volume_t volR, typename st_sample_t, MixMode mixMode>
int RateConverter_Impl<inStereo, outStereo, reverseStereo>::commonConvert(AudioStream &input, st_sample_t *outBuffer, st_size_t numSamples, st_volume_t volL_val, st_volume_t volR_val, int outputSamples) {
@@ -148,7 +195,7 @@ int RateConverter_Impl<inStereo, outStereo, reverseStereo>::commonConvert(AudioS
return (outBuffer - outStart) / (outStereo ? 2 : 1);
}
- // Process as many samples as we can from the current buffer
+ // Process as many whole outputSample groups as we can from the current buffer
const int count = MIN(
_bufferSize / (inStereo ? 2 : 1),
(int)(outEnd - outBuffer) / (outStereo ? 2 : 1) / outputSamples);
@@ -157,67 +204,13 @@ int RateConverter_Impl<inStereo, outStereo, reverseStereo>::commonConvert(AudioS
if (volL | volR) {
// Mix the data into the output buffer
for (int i = 0; i < count; ++i) {
- int16 inL, inR;
+ // This code is eliminated if muted
+ const int16 inL = _bufferPos[0];
+ const int16 inR = inStereo ? _bufferPos[1] : _bufferPos[0];
+ _bufferPos += (inStereo ? 2 : 1);
- if (inStereo) {
- if (volL != 0)
- inL = *_bufferPos++;
- else
- _bufferPos++;
-
- if (volR != 0)
- inR = *_bufferPos++;
- else
- _bufferPos++;
- } else {
- if (volL != 0) {
- inL = *_bufferPos++;
- if (volR != 0)
- inR = inL;
- } else {
- inR = *_bufferPos++;
- }
- }
-
- st_sample_t outL, outR;
-
- if (volL != 0) {
- if (volL != Audio::Mixer::kMaxMixerVolume)
- outL = (inL * (int)volL_val) / Audio::Mixer::kMaxMixerVolume;
- else
- outL = inL;
- }
-
- if (volR != 0) {
- if (volR != Audio::Mixer::kMaxMixerVolume)
- outR = (inR * (int)volR_val) / Audio::Mixer::kMaxMixerVolume;
- else
- outR = inR;
- }
-
- // TODO: could be unrolled
- for (int j = 0; j < outputSamples; ++j) {
- if (outStereo) {
- // Output left channel
- if (volL != 0)
- processSample<mixMode>(outBuffer[reverseStereo ], outL);
-
- // Output right channel
- if (volR != 0)
- processSample<mixMode>(outBuffer[reverseStereo ^ 1], outR);
- } else {
- // Output mono channel
- st_sample_t monoOut;
- if (volL != 0 && volR != 0)
- monoOut = (outL + outR) / 2;
- else if (volL != 0)
- monoOut = outL / 2;
- else if (volR != 0)
- monoOut = outR / 2;
- processSample<mixMode>(outBuffer[0], monoOut);
- }
- outBuffer += (outStereo ? 2 : 1);
- }
+ for (int j = 0; j < outputSamples; ++j)
+ writeFrame<volL, volR, st_sample_t, mixMode>(outBuffer, inL, inR, volL_val, volR_val);
}
} else {
_bufferPos += count * (inStereo ? 2 : 1);
Commit: 58fabd305360f6a1b342d053a15bb5886c61295d
https://github.com/scummvm/scummvm/commit/58fabd305360f6a1b342d053a15bb5886c61295d
Author: yasuno45 (yasuno45 at gmail.com)
Date: 2026-07-20T21:23:18+10:00
Commit Message:
AUDIO: Fix infinite loop in RateConverter on a partial upsample group
When the output rate is an exact multiple of the input rate,
commonConvert() writes every input frame out `outputSamples` times in a
row. It only ever processes whole groups:
count = MIN(_bufferSize / inFrameSize,
outFramesLeft / outputSamples);
If fewer than `outputSamples` output frames are left, count becomes 0.
Neither _bufferSize nor outBuffer is advanced, the input buffer is not
empty so it is not refilled, and the enclosing `while (outBuffer <
outEnd)` spins forever.
Reaching this needs the requested frame count to not be a multiple of
the upsample factor, so backends handing out power-of-two buffers never
see it. The Emscripten SDL3 backend does: EmscriptenSdlMixerManager
passes SDL's `additional_amount` straight through, and with a 44100 Hz
mixer on a 48000 Hz browser device that is 1882 or 1881 frames. Playing
an 11025 Hz stream (factor 4) then hangs the browser tab on the first
mix that has a live channel.
Keep the leftover frames of a group in _pendingRepeats and finish them
at the start of the next call, so that no sample is dropped or
duplicated. While here, discard a trailing partial frame in the input
cache instead of stalling on it for the same reason.
Assisted-by: Claude Code:claude-opus-4.8
Changed paths:
audio/rate.cpp
diff --git a/audio/rate.cpp b/audio/rate.cpp
index e6036a54b2d..c4f2018360a 100644
--- a/audio/rate.cpp
+++ b/audio/rate.cpp
@@ -74,12 +74,21 @@ private:
/** Fractional position of the output stream in input stream unit */
frac_t _outPosFrac;
- /** Last sample(s) in the input stream (left/right channel) */
+ /**
+ * Last sample(s) in the input stream (left/right channel). The interpolating
+ * path interpolates from them towards _inCur*, the upsampling path copies them.
+ */
int16 _inLastL, _inLastR;
/** Current sample(s) in the input stream (left/right channel) */
int16 _inCurL, _inCurR;
+ /**
+ * How many more copies of _inLastL/_inLastR the next call has to write
+ * before reading further input (upsampling).
+ */
+ int _pendingRepeats;
+
/** Write one output frame built from a single input frame, and advance outBuffer. */
template<st_volume_t volL, st_volume_t volR, typename st_sample_t, MixMode mixMode>
FORCEINLINE void writeFrame(st_sample_t *&outBuffer, int16 inL, int16 inR, st_volume_t volL_val, st_volume_t volR_val);
@@ -127,13 +136,13 @@ public:
int convert(AudioStream &input, byte *outBuffer, uint outBytesPerSample, st_size_t numSamples, st_volume_t vol_l, st_volume_t vol_r, MixMode mixMode) override;
- void setInputRate(st_rate_t inputRate) override { _inRate = inputRate; }
- void setOutputRate(st_rate_t outputRate) override { _outRate = outputRate; }
+ void setInputRate(st_rate_t inputRate) override { _inRate = inputRate; _pendingRepeats = 0; }
+ void setOutputRate(st_rate_t outputRate) override { _outRate = outputRate; _pendingRepeats = 0; }
st_rate_t getInputRate() const override { return _inRate; }
st_rate_t getOutputRate() const override { return _outRate; }
- bool needsDraining() const override { return _bufferSize != 0; }
+ bool needsDraining() const override { return _bufferSize != 0 || _pendingRepeats != 0; }
};
template<bool inStereo, bool outStereo, bool reverseStereo>
@@ -186,19 +195,45 @@ int RateConverter_Impl<inStereo, outStereo, reverseStereo>::commonConvert(AudioS
const st_sample_t *outEnd = outBuffer + numSamples * (outStereo ? 2 : 1);
while (outBuffer < outEnd) {
- // Check if we have to refill the buffer
- if (_bufferSize == 0) {
+ // Finish a group of repeated output frames which the previous call had
+ // to cut short because it ran out of output buffer.
+ while (_pendingRepeats > 0 && outBuffer < outEnd) {
+ writeFrame<volL, volR, st_sample_t, mixMode>(outBuffer, _inLastL, _inLastR, volL_val, volR_val);
+ _pendingRepeats--;
+ }
+ if (outBuffer == outEnd)
+ break;
+
+ // Check if we have to refill the buffer. A partial frame left over in
+ // the buffer can never be used, so discard it and refill as well.
+ if (_bufferSize < (inStereo ? 2 : 1)) {
_bufferPos = _buffer;
_bufferSize = input.readBuffer(_buffer, ARRAYSIZE(_buffer));
- if (_bufferSize <= 0)
+ if (_bufferSize < (inStereo ? 2 : 1)) {
+ _bufferSize = 0;
return (outBuffer - outStart) / (outStereo ? 2 : 1);
+ }
}
// Process as many whole outputSample groups as we can from the current buffer
const int count = MIN(
_bufferSize / (inStereo ? 2 : 1),
(int)(outEnd - outBuffer) / (outStereo ? 2 : 1) / outputSamples);
+
+ if (count == 0) {
+ // Fewer than outputSamples output frames are left, so no whole
+ // group fits. Consume one input frame anyway and let the code at
+ // the top of the loop write what fits; the rest is carried over to
+ // the next call. Without this the loop could never make progress.
+ _inLastL = _bufferPos[0];
+ _inLastR = inStereo ? _bufferPos[1] : _bufferPos[0];
+ _bufferPos += (inStereo ? 2 : 1);
+ _bufferSize -= (inStereo ? 2 : 1);
+ _pendingRepeats = outputSamples;
+ continue;
+ }
+
_bufferSize -= count * (inStereo ? 2 : 1);
if (volL | volR) {
@@ -462,7 +497,8 @@ RateConverter_Impl<inStereo, outStereo, reverseStereo>::RateConverter_Impl(st_ra
_inCurL(0),
_inCurR(0),
_bufferSize(0),
- _bufferPos(nullptr) {}
+ _bufferPos(nullptr),
+ _pendingRepeats(0) {}
template<bool inStereo, bool outStereo, bool reverseStereo>
template<typename st_sample_t, MixMode mixMode>
Commit: 43c2267456e933822b14e3cb8bc612d110bfa1dd
https://github.com/scummvm/scummvm/commit/43c2267456e933822b14e3cb8bc612d110bfa1dd
Author: yasuno45 (yasuno45 at gmail.com)
Date: 2026-07-20T21:23:18+10:00
Commit Message:
TEST: Add RateConverter regression tests
Covers the partial upsample group that used to spin forever, for both a
stereo and a mono input, plus the 1:1 copy path.
Assisted-by: Claude Code:claude-opus-4.8
Changed paths:
A test/audio/rate.h
diff --git a/test/audio/rate.h b/test/audio/rate.h
new file mode 100644
index 00000000000..9c551127fe5
--- /dev/null
+++ b/test/audio/rate.h
@@ -0,0 +1,113 @@
+#include <cxxtest/TestSuite.h>
+
+#include "audio/audiostream.h"
+#include "audio/mixer.h"
+#include "audio/rate.h"
+
+/**
+ * A stream of consecutive integers, so that every sample in the output can be
+ * traced back to the position it was read from. Sample numbering starts at 1,
+ * so input frame f of a stereo stream holds the pair (2f + 1, 2f + 2).
+ */
+class CountingAudioStream : public Audio::AudioStream {
+public:
+ CountingAudioStream(int rate, bool stereo) : _rate(rate), _stereo(stereo), _pos(0) {}
+
+ int readBuffer(int16 *buffer, const int numSamples) override {
+ for (int i = 0; i < numSamples; ++i)
+ buffer[i] = (int16)(++_pos);
+ return numSamples;
+ }
+
+ bool isStereo() const override { return _stereo; }
+ int getRate() const override { return _rate; }
+ bool endOfData() const override { return false; }
+
+private:
+ int _rate;
+ bool _stereo;
+ int _pos;
+};
+
+class RateTestSuite : public CxxTest::TestSuite {
+public:
+ /**
+ * When the output rate is an exact multiple of the input rate, every input
+ * frame is written out `factor` times in a row. A request whose frame count
+ * is not a multiple of `factor` leaves such a group half written, and the
+ * converter has to finish it on the next call rather than stall.
+ */
+ void test_upsample_partial_group() {
+ // 11025 Hz -> 44100 Hz, i.e. every input frame is repeated 4 times.
+ Audio::RateConverter *converter = Audio::makeRateConverter(11025, 44100, true, true, false);
+ CountingAudioStream input(11025, true);
+
+ // 1882 = 4 * 470 + 2 and 1881 = 4 * 470 + 1, so both requests end in the
+ // middle of a group. These are the frame counts SDL3 asks for on the
+ // Emscripten backend, where the browser audio device runs at 48000 Hz.
+ const int requests[3] = { 1882, 1881, 1882 };
+ const int totalFrames = 1882 + 1881 + 1882;
+
+ int16 *out = new int16[totalFrames * 2]();
+ int16 *pos = out;
+
+ for (int i = 0; i < 3; ++i) {
+ const int written = converter->convert(input, (byte *)pos, sizeof(int16),
+ requests[i], Audio::Mixer::kMaxMixerVolume, Audio::Mixer::kMaxMixerVolume,
+ Audio::MIX_ADD);
+ TS_ASSERT_EQUALS(written, requests[i]);
+ pos += requests[i] * 2;
+ }
+
+ // Output frame k must be a copy of input frame k / 4, across call boundaries.
+ for (int k = 0; k < totalFrames; ++k) {
+ TS_ASSERT_EQUALS(out[k * 2 + 0], (int16)(2 * (k / 4) + 1));
+ TS_ASSERT_EQUALS(out[k * 2 + 1], (int16)(2 * (k / 4) + 2));
+ }
+
+ // totalFrames % 4 != 0, so a group is still open.
+ TS_ASSERT_EQUALS(converter->needsDraining(), true);
+
+ delete[] out;
+ delete converter;
+ }
+
+ /**
+ * The same, for a mono stream upsampled into a stereo buffer.
+ */
+ void test_upsample_partial_group_mono() {
+ Audio::RateConverter *converter = Audio::makeRateConverter(11025, 22050, false, true, false);
+ CountingAudioStream input(11025, false);
+
+ // 7 = 2 * 3 + 1, so the last group is cut in half.
+ int16 out[7 * 2] = {};
+ const int written = converter->convert(input, (byte *)out, sizeof(int16), 7,
+ Audio::Mixer::kMaxMixerVolume, Audio::Mixer::kMaxMixerVolume, Audio::MIX_ADD);
+
+ TS_ASSERT_EQUALS(written, 7);
+ for (int k = 0; k < 7; ++k) {
+ TS_ASSERT_EQUALS(out[k * 2 + 0], (int16)(k / 2 + 1));
+ TS_ASSERT_EQUALS(out[k * 2 + 1], (int16)(k / 2 + 1));
+ }
+
+ delete converter;
+ }
+
+ /**
+ * A 1:1 conversion never groups, and must not hold anything back.
+ */
+ void test_copy() {
+ Audio::RateConverter *converter = Audio::makeRateConverter(22050, 22050, true, true, false);
+ CountingAudioStream input(22050, true);
+
+ int16 out[5 * 2] = {};
+ const int written = converter->convert(input, (byte *)out, sizeof(int16), 5,
+ Audio::Mixer::kMaxMixerVolume, Audio::Mixer::kMaxMixerVolume, Audio::MIX_ADD);
+
+ TS_ASSERT_EQUALS(written, 5);
+ for (int i = 0; i < 10; ++i)
+ TS_ASSERT_EQUALS(out[i], (int16)(i + 1));
+
+ delete converter;
+ }
+};
Commit: af58433037e43ca06c2198e04544b16ea3a96b1a
https://github.com/scummvm/scummvm/commit/af58433037e43ca06c2198e04544b16ea3a96b1a
Author: Miro Kropacek (miro.kropacek at gmail.com)
Date: 2026-07-20T21:23:18+10:00
Commit Message:
TEST: Add another RateConverter test
As reported on OpenBSD with SegaCD's SOMI release: an 11025 Hz mono
sound effect upsampled into a 44100 Hz stereo mixer in blocks of 882
frames. 882 = 4 * 220 + 2, so every request ends in the middle of an
upsample group.
Changed paths:
test/audio/rate.h
diff --git a/test/audio/rate.h b/test/audio/rate.h
index 9c551127fe5..d045c373aae 100644
--- a/test/audio/rate.h
+++ b/test/audio/rate.h
@@ -93,6 +93,46 @@ public:
delete converter;
}
+ /**
+ * The field configuration of the SegaCD Monkey Island 1 hang on OpenBSD:
+ * an 11025 Hz mono sound effect upsampled into a 44100 Hz stereo mixer at
+ * non-maximum volume with clamped mixing, in sndio-sized blocks of 882
+ * frames. 882 = 4 * 220 + 2, so every request ends in the middle of a
+ * group.
+ */
+ void test_upsample_partial_group_sndio_blocks() {
+ Audio::RateConverter *converter = Audio::makeRateConverter(11025, 44100, false, true, false);
+ CountingAudioStream input(11025, false);
+
+ const int frames = 882;
+ const int totalFrames = 3 * frames;
+ const uint16 volume = 192;
+
+ int16 *out = new int16[totalFrames * 2]();
+ int16 *pos = out;
+
+ for (int i = 0; i < 3; ++i) {
+ const int written = converter->convert(input, (byte *)pos, sizeof(int16),
+ frames, volume, volume, Audio::MIX_CLAMPED_ADD);
+ TS_ASSERT_EQUALS(written, frames);
+ pos += frames * 2;
+ }
+
+ // Output frame k must be a volume-scaled copy of input frame k / 4,
+ // across call boundaries.
+ for (int k = 0; k < totalFrames; ++k) {
+ const int16 expected = (int16)(((k / 4 + 1) * volume) / Audio::Mixer::kMaxMixerVolume);
+ TS_ASSERT_EQUALS(out[k * 2 + 0], expected);
+ TS_ASSERT_EQUALS(out[k * 2 + 1], expected);
+ }
+
+ // totalFrames % 4 != 0, so a group is still open.
+ TS_ASSERT_EQUALS(converter->needsDraining(), true);
+
+ delete[] out;
+ delete converter;
+ }
+
/**
* A 1:1 conversion never groups, and must not hold anything back.
*/
More information about the Scummvm-git-logs
mailing list