Real time safe recording with ring buffer stage one #7903

steven-jaro · 2025-05-23T20:45:10Z

This is the first stage of the implementation of the ring buffer for real time safety when recording as this is needed to solve issue number 4 in #7786. Issue 4 says that recording is not safe with the current method. Here are the changes made to the code in this PR:

AudioEngine.h:

Included "#include "LocklessRingBuffer.h"
Added needed ringbuffer variables.
Added new function "processBufferedInputFrames".

AudioEngine.cpp:

Included:
#include
#include
#include
#include
#include "LocklessRingBuffer.h"
Created static constant for the buffer size:
static const f_cnt_t FIXED_INPUT_BUFFER_CAPACITY = DEFAULT_BUFFER_SIZE * 100;
In the constructor, we make use of the constant "DEFAULT_BUFFER_SIZE". Changed the name of "wt" to "workerThread". Added the initialization in the constructor for the 3 ringbuffer needed variables.
Added code to the destructor to safely delete the worker threads.
Changed "pushInputFrames" function to write in the ringbuffer.
Created new function "void AudioEngine::processBufferedInputFrames()". This function runs continuously. First checks if there is data in the ringbuffer, if not, it returns. Then reads the data read in the ringbuffer into two halfs so that data can be written continously. Then it writes the data of the two halfs from the ringbuffer to m_inputBufferFrames.

AudioDevie.h:

Added function "isProcessing()" to check the value of m_inProcess. This is so that the destructor can check if "m_inProcess" is true so that it closes the processing correctly.

How to test this PR?"
This PR just makes that recording is memmory safe, so one should look for memmory leaks, segfaults or possible reallocatio in the code.

firiox · 2025-05-23T21:30:57Z

No downloable .exe

regulus79 · 2025-05-23T21:32:40Z

Oh yeah that's because the workflows weren't running. They're going now, so an exe should be available in the next 15-20 minutes.

firiox · 2025-05-23T22:35:51Z

Ok how I test It dude? . .

tresf · 2025-05-24T05:00:17Z

Ok how I test It dude? . .

https://lmms.io/download/pull-request/7903

AW1534 · 2025-05-24T11:46:27Z

Works well but i get these occasional crackles and the audio peaks.

edit: Audacity doesn't have this issue

steven-jaro · 2025-05-24T13:36:57Z

Works well but i get these occasional crackles and the audio peaks.

edit: Audacity doesn't have this issue

Hey, thanks for the feedback.

steven-jaro · 2025-05-25T14:59:07Z

I changed the approach as it was pretty unstable. Please feel free to test the code and review it. My new approach also is supposed to fix the random segfaults and clipping audio. I also tried to fix all the naming issues with the variables for the camelCase, if some are still remaining, I will fix it in the next days.

JohannesLorenz · 2025-05-25T16:16:41Z

src/core/AudioEngine.cpp

-		delete [] buf;
+        f_cnt_t newSize = std::max(currentWriteBufSize * 2, requiredSize + DEFAULT_BUFFER_SIZE);
+
+        auto newBuffer = new SampleFrame[newSize];


If this function is run in realtime context, then this line would cause an allocation, which is not realtime-safe.

JohannesLorenz · 2025-05-25T16:17:47Z

src/core/AudioEngine.cpp

+        m_inputBufferFrames[i] = 0;
+        m_inputBufferSize[i] = initialBufferSize;
+
+        delete[] m_inputBuffer[i];


delete[], new[] and reserve are - sadly - not realtime-safe either.

(In initialization code it is fine though)

Thanks for the feedback.

I will solve any problem.

AW1534 · 2025-05-25T17:41:12Z

You fixed it, great job! Still seems to run smoothly too, haven't had any crashes or bugs

steven-jaro · 2025-05-25T17:55:25Z

You fixed it, great job! Still seems run smoothly too, haven't had any crashes or bugs

Thank you for the feedback and testing.

steven-jaro · 2025-05-25T22:07:16Z

My most recent push has the intent to remove all the allocation I could think of to get more real time safety. I think there is still some points with non real time safety, but at least I would like to get feedback as I consider this a progress and not the final deal.

regulus79 · 2025-05-26T00:28:47Z

Help me understand this code...

Before this PR, were we using a single buffer m_inputBuffer[m_inputBufferWrite] for recording audio which would just be scaled up 2x every time it got filled up?

And this PR replaces that with a ring buffer.... which is slowly filled up and copied over to the other buffer ?

steven-jaro · 2025-05-26T17:02:14Z

Help me understand this code...

Before this PR, were we using a single buffer m_inputBuffer[m_inputBufferWrite] for recording audio which would just be scaled up 2x every time it got filled up?

And this PR replaces that with a ring buffer.... which is slowly filled up and copied over to the other buffer ?

Hello, this is the explaination of the code for this PR.
First I will answer your questions:

Before this PR, were we using a single buffer "m_inputBuffer[m_inputBufferWrite]" for recording audio which would just be scaled up 2x every time it got filled up?
R/ Basically yes. Here is how old recording used to work:
a. The function "AudioEngine::pushInputFrames()" was called directly by the os audio driver's callback.
b. It would directly write incoming audio frames to "m_inputBuffer[m_inputBufferWrite]".
c. If the space "m_inputBuffer[m_inputBufferWrite]" wasn't enough anymore, "pushInputFrames" would: Lock a mutex using "requestChangeInModel()", ALLOCATE a new larger buffer, copy data from the old buffer to the new one and delete the old buffer and the lock the mutex again.

All of that is not real time safe as we were allocating with "new" and deallocating with "delete[]".

"And this PR replaces that with a ring buffer.... which is slowly filled up and copied over to the other buffer ?"
R/ Yes. My most recent approach does this:
a. "AudioEngine::pushInputFrames()" is called by the audio driver.

But now it would only write incoming audio to the lockless ring buffer "m_inputAudioRingBuffer".
The functions lightweight as it doesn't perform a mutex locking or allocation. If the ring buffer is full, it drops the frames which is better than allocating.

b. I added the function "AudioEngine::processBufferedInputFrames()".

It is called from "AudioEngine::renderStageNoteSetup()" which is called continously.
It first checks if "m_inputAudioRingBuffer" has new data.
Then it copies the ring buffer into "m_inputBuffer[m_inputBufferWrite]".
It is safe as it doesn't run in a less critical context than the OS calling it directly. So now it can use "requestChangeInModel()" safely.
"m_inputBuffer[0]" and "m_inputBuffer[1]" are now allocanted one time by the constructor to a fixed capacity. So now "processBufferedInputFrames" doesn't do reallocation.
"std::vector m_tempInputProcessingBuffer" is used to consolidate the data from the ring buffer. The vector is reserver in the constructor to a capacity that is based on the ring buffer size. In "processBufferedInputFrames" I use "resize()" but to the actual number of frames that are being processed. And this is withing the reserved capacity so we are not doing heal reallocations.

steven-jaro · 2025-05-30T00:35:59Z

I solved the problems with the resize and reserve. Now it is supposed to be completely real time safe.

JohannesLorenz · 2025-05-30T08:46:55Z

I solved the problems with the resize and reserve. Now it is supposed to be completely real time safe.

OK, I will look later at it. But please, can you fix the indentation issues first? Thanks!

src/core/AudioEngine.cpp

JohannesLorenz

Partial code review finished, will need another one after the rework.

What I checked:

Functionality - partially
Style - partially

src/core/AudioEngine.cpp

steven-jaro · 2025-05-30T12:42:24Z

All changes requested will be applied. But I just have that doubt with the and statement.

steven-jaro · 2025-05-30T18:45:04Z

All the resquests were applied.

szeli1

I did not test this PR. I believe I found a size_t casting bug which might cause a crash. It is similar to old AudioFileProcessor code, where size_t values were subtracted from carelessly resulting in out of index reads / writes. I may be incorrect about my finding tho.

src/core/AudioEngine.cpp

include/AudioEngine.h

szeli1 · 2025-05-30T19:35:27Z

src/core/AudioEngine.cpp

-		m_inputBuffer[i] = new SampleFrame[ DEFAULT_BUFFER_SIZE * 100 ];
-		zeroSampleFrames(m_inputBuffer[i], m_inputBufferSize[i]);
+		m_inputBufferSize[i] = FIXED_INPUT_BUFFER_CAPACITY;
+		m_inputBuffer[i] = new SampleFrame[ FIXED_INPUT_BUFFER_CAPACITY ];


Suggested change

m_inputBuffer[i] = new SampleFrame[ FIXED_INPUT_BUFFER_CAPACITY ];

m_inputBuffer[i] = new SampleFrame[FIXED_INPUT_BUFFER_CAPACITY];

Maybe replace this with an std::vector?
Allocation, size is handled by default.

Or 2 std vectors

You closed this without expressing your opinion about using std::vector

What is the benefit of using an std::vector when we just need a fixed size simple array? Is the benefit on the exception safety?

The benefit is that std::vector does everything we already do, but it can destruct itself. (And it incorporates the size data, meaning we don't have to store it)

Using std::array would be the replacement, not std::vector.

Using std::array would be the replacement, not std::vector.

This is correct, I thought m_inputBuffer was still increased in void AudioEngine::pushInputFrames( SampleFrame* _ab, const f_cnt_t _frames )

Fixed in the recent refactor.

I've replaced the C-style arrays with std::array<std::vector<SampleFrame>, 2>.

The vectors are pre-allocated with .reserve(FIXED_INPUT_BUFFER_CAPACITY) and grow
dynamically with .resize() as frames are added (up to the reserved capacity), so:

No manual memory management (new/delete)

Size is tracked automatically (.size() instead of m_inputBufferFrames)

Automatic cleanup (RAII)

Still real-time safe (no allocations during recording since we stay within reserved capacity)

The reason I used std::vector instead of std::array is because m_inputBuffer needs
to grow dynamically as frames are added in processBufferedInputFrames(), then gets
cleared in swapBuffers(). A fixed-size std::array wouldn't work for this use case.

@Veratil - Good point about std::array being typical for fixed-size, but in this case
the variable size behavior is needed (though bounded by the reserved capacity).

src/core/AudioEngine.cpp

szeli1

I did not test this PR. What steps to follow to test this PR?

src/core/AudioEngine.cpp

szeli1 · 2025-11-23T12:09:21Z

src/core/AudioEngine.cpp

+
+	if (framesInSequence == 0) return;
+
+	requestChangeInModel();


Is it really real time safe when it uses mutexes?

Yes, it is real-time safe. The mutex in requestChangeInModel() is NOT in the real-time
audio callback path. Here's why:

Normal playback mode:

Uses fifoWriter (needsFifo = true by default in startProcessing())

Audio callback only calls nextBuffer() → m_fifo->read() (lockless, no mutex)

The fifoWriter thread (separate, non-RT thread) calls renderNextBuffer() which includes
the mutex, but this is outside the RT path

Export/rendering mode:

Doesn't use fifoWriter (needsFifo = false in ProjectRenderer.cpp:171)

But there's no real-time audio callback in this mode, just writing to a file

No RT constraints, so mutex is perfectly fine

The architecture ensures that pushInputFrames() (called from audio device callback)
only writes to the lockless ring buffer, while processBufferedInputFrames() (which has
the mutex) runs either in the fifoWriter thread or in rendering mode where RT-safety
isn't required.

szeli1 · 2025-11-23T12:11:24Z

src/core/AudioEngine.cpp

+	delete m_midiClient; m_midiClient = nullptr;
+	delete m_audioDev; m_audioDev = nullptr;


Maybe do = nullptr on new lines?

You are right, it will be fixed.

szeli1 · 2025-11-23T12:12:47Z

include/AudioEngine.h

+
+	std::unique_ptr<LocklessRingBuffer<SampleFrame>> m_inputAudioRingBuffer;
+	std::unique_ptr<LocklessRingBufferReader<SampleFrame>> m_inputAudioRingBufferReader;
+	std::vector<SampleFrame> m_tempInputProcessingBuffer;


Please could you leave a comment why a temporary vector is needed?

Good catch. After reviewing the code, I realized this temporary vector is not actually
being used. The current implementation copies directly from the ring buffer sequence
(handling the two potential halves with first_half_ptr() and second_half_ptr()) to the
destination buffer without needing an intermediate vector.

I've removed it to keep the code clean.

szeli1 · 2025-11-23T12:33:19Z

src/core/AudioEngine.cpp

+	SampleFrame* currentWriteBufPtr = m_inputBuffer[m_inputBufferWrite];
+	f_cnt_t currentWriteBufCapacity = m_inputBufferSize[m_inputBufferWrite];
+	f_cnt_t currentFramesInWriteBuf = m_inputBufferFrames[m_inputBufferWrite];


It would be better to use vectors instead of C arrays. That way the size is also known. I reviewed the code and to me, it seems like m_inputBuffer is used in about 3 functions, it shouldn't be hard to replace it with std::vector. (and also it would get rid of m_inputBufferFrames)

I think it would be a reasonable request to ask you to refactor m_inputBuffer. Please would you mind doing that?

Done. I've refactored m_inputBuffer to use std::vector as you suggested.

Changes made:

Header (AudioEngine.h):

Replaced SampleFrame* m_inputBuffer[2], f_cnt_t m_inputBufferFrames[2], and
f_cnt_t m_inputBufferSize[2] with std::array<std::vector<SampleFrame>, 2> m_inputBuffer

Updated inputBuffer() to return m_inputBuffer[m_inputBufferRead].data()

Updated inputBufferFrames() to return m_inputBuffer[m_inputBufferRead].size()

Implementation (AudioEngine.cpp):

Constructor: Changed from new SampleFrame[FIXED_INPUT_BUFFER_CAPACITY] to
m_inputBuffer[i].reserve(FIXED_INPUT_BUFFER_CAPACITY)

Destructor: Removed manual delete[] - vectors clean up automatically

processBufferedInputFrames():

Use vector references instead of raw pointers

Call .resize() before memcpy() to accommodate new frames

Removed manual size tracking (now uses .size(), .capacity())

swapBuffers(): Changed from m_inputBufferFrames[m_inputBufferWrite] = 0 to
m_inputBuffer[m_inputBufferWrite].clear()

Real-time safety maintained:

Vectors are pre-allocated with .reserve(FIXED_INPUT_BUFFER_CAPACITY) in constructor

.resize() only increases size up to the reserved capacity, so no heap allocations
occur during recording (capacity never exceeded)

All operations remain real-time safe

Testing:

Compiled successfully and tested recording for several minutes without issues.

Thanks for the suggestion. The code is cleaner and more maintainable now.

src/core/AudioEngine.cpp

Co-authored-by: szeli1 <[email protected]>

messmerd · 2025-12-07T04:50:17Z

@steven-jaro Please refrain from rebasing feature branches after you open a PR and people have started reviewing it. It rewrites the git history and makes it harder for reviewers to see what recent changes you made. If you need to fix merge conflicts, you can merge master (or in this case feature/recording-stage-one) into your feature branch instead.

steven-jaro · 2025-12-08T18:53:02Z

@steven-jaro Please refrain from rebasing feature branches after you open a PR and people have started reviewing it. It rewrites the git history and makes it harder for reviewers to see what recent changes you made. If you need to fix merge conflicts, you can merge master (or in this case feature/recording-stage-one) into your feature branch instead.

I am really sorry, I wasn't aware of this. Won't happen again.

Veratil · 2025-12-08T20:49:19Z

You can rebase at the end before merging though.

Co-authored-by: szeli1 <[email protected]>

steven-jaro mentioned this pull request May 23, 2025

Sample Track Recording Stage One (by Reflexe, LMMS/lmms continuation) #7786

Draft

steven-jaro changed the title ~~Reltime safe recording with ring buffer stange one~~ Real time safe recording with ring buffer stange one May 23, 2025

steven-jaro changed the title ~~Real time safe recording with ring buffer stange one~~ Real time safe recording with ring buffer stane one May 23, 2025

steven-jaro changed the title ~~Real time safe recording with ring buffer stane one~~ Real time safe recording with ring buffer stage one May 23, 2025

JohannesLorenz reviewed May 25, 2025

View reviewed changes

steven-jaro requested a review from JohannesLorenz May 26, 2025 17:17

szeli1 self-requested a review May 29, 2025 21:23

JohannesLorenz reviewed May 30, 2025

View reviewed changes

src/core/AudioEngine.cpp Outdated Show resolved Hide resolved

JohannesLorenz requested changes May 30, 2025

View reviewed changes

steven-jaro requested a review from JohannesLorenz May 30, 2025 16:47

szeli1 suggested changes May 30, 2025

View reviewed changes

steven-jaro requested a review from szeli1 November 23, 2025 00:33

szeli1 reviewed Nov 23, 2025

View reviewed changes

steven-jaro and others added 20 commits November 23, 2025 09:46

Fixed spaces instead of tabs size 4 and Johannes requests

aa33e45

Changed DEFAULT_BUFFER_SIZE to 100

8541272

Update src/core/AudioEngine.cpp

dad148e

Co-authored-by: szeli1 <[email protected]>

Update include/AudioEngine.h

e6c1097

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

e25a58e

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

bb00189

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

264d15b

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

3d02d41

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

6b4d07d

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

69734f9

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

1dae270

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

d9a22b2

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

cae9d61

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

00ab49d

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

d6cbe34

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

ed4ecb8

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

2047da5

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

11432b1

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

5820a43

Co-authored-by: szeli1 <[email protected]>

Fixed unsafe substraction in processBufferedInputFrames

52868fd

steven-jaro force-pushed the recording-with-ringbuffer branch from e492d48 to 52868fd Compare November 23, 2025 15:48

Fixed szeli1 Requested changes includir refator

89f6521

steven-jaro requested review from Veratil and szeli1 December 4, 2025 14:22

steven-jaro and others added 2 commits December 9, 2025 16:23

Update src/core/AudioEngine.cpp

45cdb83

Co-authored-by: szeli1 <[email protected]>

Update src/core/AudioEngine.cpp

95bbfd6

Co-authored-by: szeli1 <[email protected]>

steven-jaro marked this pull request as ready for review December 9, 2025 23:03

	m_inputBuffer[i] = new SampleFrame[ FIXED_INPUT_BUFFER_CAPACITY ];
	m_inputBuffer[i] = new SampleFrame[FIXED_INPUT_BUFFER_CAPACITY];

		delete m_midiClient; m_midiClient = nullptr;
		delete m_audioDev; m_audioDev = nullptr;

Uh oh!

Real time safe recording with ring buffer stage one #7903

Are you sure you want to change the base?

Real time safe recording with ring buffer stage one #7903

Conversation

steven-jaro commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

firiox commented May 23, 2025

Uh oh!

regulus79 commented May 23, 2025

Uh oh!

firiox commented May 23, 2025

Uh oh!

tresf commented May 24, 2025

Uh oh!

AW1534 commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steven-jaro commented May 24, 2025

Uh oh!

steven-jaro commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohannesLorenz May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AW1534 commented May 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steven-jaro commented May 25, 2025

Uh oh!

steven-jaro commented May 25, 2025

Uh oh!

regulus79 commented May 26, 2025

Uh oh!

steven-jaro commented May 26, 2025

Uh oh!

steven-jaro commented May 30, 2025

Uh oh!

JohannesLorenz commented May 30, 2025

Uh oh!

Uh oh!

JohannesLorenz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steven-jaro commented May 30, 2025

Uh oh!

steven-jaro commented May 30, 2025

Uh oh!

szeli1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szeli1 May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

szeli1 Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

steven-jaro commented May 23, 2025 •

edited

Loading

AW1534 commented May 24, 2025 •

edited

Loading

steven-jaro commented May 25, 2025 •

edited

Loading

JohannesLorenz May 25, 2025 •

edited

Loading

AW1534 commented May 25, 2025 •

edited

Loading

szeli1 May 31, 2025 •

edited

Loading

szeli1 Nov 23, 2025 •

edited

Loading