Skip to content

Fix AudioResampler crash on empty buffer input#8364

Open
struktured wants to merge 1 commit into
LMMS:masterfrom
struktured-labs:fix-audioresampler-empty-span
Open

Fix AudioResampler crash on empty buffer input#8364
struktured wants to merge 1 commit into
LMMS:masterfrom
struktured-labs:fix-audioresampler-empty-span

Conversation

@struktured

Copy link
Copy Markdown

Summary

AudioResampler::process() aborts the application when handed an empty buffer view. libsamplerate's src_process() throws when SRC_DATA->data_in or data_out is null, and an empty std::span yields data() == nullptr.

Crash report

Reproduced during long playback with an SF2 instrument track on LMMS 1.3.0-alpha (commit ab0ed11):

terminate called after throwing an instance of 'std::runtime_error'
  what():  SRC_DATA->data_out or SRC_DATA->data_in is NULL.

The resampler's sample-rate conversion path (used when the plugin's internal rate differs from the engine's output rate) can present a zero-frame slice of Sf2Instrument::m_bufferView to process() between refill checks. libsamplerate then aborts.

Root cause

AudioResampler::process() populates SRC_DATA from InterleavedBufferView::data() without checking whether the view has any frames. An empty span has no underlying storage, so data() == nullptr. libsamplerate's documented behavior on null pointers is SRC_ERR_BAD_DATA, which process() rethrows as std::runtime_error.

Fix

Early-return {0, 0} when either buffer has zero frames. The zero/zero pair is already the documented "no progress" signal existing callers handle — e.g. Sf2Instrument::renderFrames breaks the inner loop when both inputFramesUsed and outputFramesGenerated are zero.

Test plan

  • Build clean, no new warnings introduced.
  • Long playback with an SF2 instrument at a non-default internal rate no longer aborts with SRC_DATA NULL.
  • Regression: tracks where the internal rate matches the engine rate go through the early fluid_synth_write_float path and never call AudioResampler::process() — unaffected.

libsamplerate's src_process() throws when SRC_DATA->data_in or data_out
is null. An empty std::span yields data() == nullptr, so any caller
that hands process() a zero-frame view (e.g. a fully-consumed buffer
slice in Sf2Instrument::renderFrames before the refill check fires)
crashes the app with:

    terminate called after throwing 'std::runtime_error'
      what():  SRC_DATA->data_out or SRC_DATA->data_in is NULL.

Early-return {0, 0} when either buffer has zero frames so the resampler
never touches libsamplerate with empty input. Fixes the crash without
requiring every caller to pre-check.
@rubiefawn rubiefawn added the bug label May 2, 2026
@messmerd

messmerd commented May 2, 2026

Copy link
Copy Markdown
Member

@sakertooth might be interested in this

@rubiefawn rubiefawn requested a review from sakertooth May 2, 2026 05:39
Comment thread src/core/AudioResampler.cpp
@rubiefawn rubiefawn added the needs testing This pull request needs more testing label May 12, 2026
@sakertooth

Copy link
Copy Markdown
Contributor

Assigning myself until further notice.

@sakertooth sakertooth self-assigned this Jun 10, 2026
@sakertooth

sakertooth commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

I cant push to this branch because for some reason, maintainers cant push to PRs made from organizations, even if they have permissions to edit the pull request. Because of that, I'm just going to submit my patch here (I changed more than what was originally changed in this PR. The patch also overrides any previous suggestions I made).

Instead of returning early or throwing when theres an error, I opted to print the error to std::cerr when in Debug mode. I dont think its a good idea to "swallow up" the error (it also isn't the best error handling option for real-time threads). We still have to fix the callers from passing nullptr input/output buffers though, but that can be done later.

0001-Print-errors-to-console-instead-of-immediately-throw.patch

From 37ec9700aade60f295be69603eaab53efb0481ed Mon Sep 17 00:00:00 2001
From: Sotonye Atemie <sakertooth@gmail.com>
Date: Wed, 17 Jun 2026 14:28:24 -0400
Subject: [PATCH] Print errors to console instead of immediately throwing them

---
 src/core/AudioResampler.cpp | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/core/AudioResampler.cpp b/src/core/AudioResampler.cpp
index 9d6ec5816..382978d40 100644
--- a/src/core/AudioResampler.cpp
+++ b/src/core/AudioResampler.cpp
@@ -24,6 +24,7 @@
 
 #include "AudioResampler.h"
 
+#include <iostream>
 #include <samplerate.h>
 #include <stdexcept>
 
@@ -62,14 +63,6 @@ AudioResampler::AudioResampler(Mode mode, ch_cnt_t channels)
 
 auto AudioResampler::process(InterleavedBufferView<const float> input, InterleavedBufferView<float> output) -> Result
 {
-	if (input.channels() != m_channels || output.channels() != m_channels)
-	{
-		throw std::invalid_argument{"Invalid channel count"};
-	}
-
-	// libsamplerate throws on empty buffers; bail before touching it.
-	if (input.frames() == 0 || output.frames() == 0) { return {0, 0}; }
-
 	auto data = SRC_DATA{};
 
 	data.data_in = input.data();
@@ -83,7 +76,10 @@ auto AudioResampler::process(InterleavedBufferView<const float> input, Interleav
 
 	if ((m_error = src_process(static_cast<SRC_STATE*>(m_state.get()), &data)))
 	{
-		throw std::runtime_error{src_strerror(m_error)};
+#ifdef LMMS_DEBUG
+		std::cerr << "AudioResampler::process: " << src_strerror(m_error) << '\n';
+#endif
+		return {0, 0};
 	}
 
 	return {static_cast<f_cnt_t>(data.input_frames_used), static_cast<f_cnt_t>(data.output_frames_gen)};
@@ -93,7 +89,9 @@ void AudioResampler::reset()
 {
 	if ((m_error = src_reset(static_cast<SRC_STATE*>(m_state.get()))))
 	{
-		throw std::runtime_error{src_strerror(m_error)};
+#ifdef LMMS_DEBUG
+		std::cerr << "AudioResampler::reset: " << src_strerror(m_error) << '\n';
+#endif
 	}
 }
 
-- 
2.54.0

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

Labels

bug needs clarification needs testing This pull request needs more testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants