[cinder-audio] write position consistency in BufferRecorderNode::process()

Developing Cinder

MrBl00m

[cinder-audio] write position consistency in BufferRecorderNode::process()

in Developing Cinder • 4 years ago

Looking at the code of BufferRecorderNode::process() - reported below - and being mWritePos a std::atomic

can I be sure the value of mWritePos at the beginning of the function be consistent with the value at the end ?

Cause it looks like there could be inconsistencies otherwise.

For example let's assume the following:

1. numWriteFrames (lenght of buffer) is 1024 samples long

2. mWritePos is equal to 2000 at the beginning of the function.

4. somewhere before line 16 is executed, bufferRecorder::start() is called from the GUI thread to start recording from the beginning. This sets mWritePos to 0

5 line 16 is executed and now mWritePos is equal to 0+1024

So the new recording starts at 1024 and the first 1024 samples of mBuffer are skipped and left with the values of the previous recording !

Can this scenario possibly happen ?

thanks

void BufferRecorderNode::process( Buffer *buffer )
{
const size_t writePos = mWritePos;
size_t numWriteFrames = buffer->getNumFrames();
if( writePos + numWriteFrames > mRecorderBuffer.getNumFrames() )
numWriteFrames = mRecorderBuffer.getNumFrames() - writePos;
mRecorderBuffer.copyOffset( *buffer, numWriteFrames, writePos, 0 );
if( numWriteFrames < buffer->getNumFrames() )
mLastOverrun = getContext()->getNumProcessedFrames();
mWritePos += numWriteFrames;
}

Replies(4)

rich.e

Re: [cinder-audio] write position consistency in BufferRecorderNode::process()

4 years ago

I think you're correct in its current form, that BufferRecorderNode doesn't have the ability to start or stop recording at a specific sample / time on the audio thread, since you fire it from the main thread and it begins the next time that the audio thread notices that recording should start / stop. However I don't think it would be difficult to add, take a look at how I did something similar for BufferPlayerNode::start( double when ) (the ability to schedule things with sample accuracy came after BufferRecorderNode was originally written).

The catch of course is that you must schedule for the recording to start in the future, so you should do the same for whatever it is that you want to record.

cheers,

Rich

MrBl00m

Re: [cinder-audio] write position consistency in BufferRecorderNode::process()

4 years ago

Hi Rich,

thank you for your reply.

If I understood you correctly, let me point out that my concern is not about sample accurate start / stop but rather whether mWritePos is modified in a way that possibly makes it jump the first samples of the new recording.

to recap what I said, imagine these instructions to happen in sequence

-> audio thread reads mWritePos as 2000 // fine

-> audio thread writes samples from 2000 to 3024 // fine, just finish the job in this block of samples

-> GUI thread sets mWritePos to 0 // ok, that's what GUI's supposed to do

-> audio thread sets mWritePos to 1024 ( mWritePos += 1024) // wrong!

-> process() is executed again, audio thread reads mWritePos as 1024, but it should be reading 0

Probably this will hardly happen because process() will be completely executed till the end as it's got higher priority.

However, for the sake of correctness, how about this fix ? Not 100% sure it's correct though. But it looks so to me...

size_t writePos = mWritePos;
size_t numWriteFrames = buffer->getNumFrames();
if ( writePos + numWriteFrames > mRecorderBuffer.getNumFrames() )
numWriteFrames = mRecorderBuffer.getNumFrames() - writePos;
mRecorderBuffer.copyOffset(*buffer, numWriteFrames, writePos, 0);
if ( numWriteFrames < buffer->getNumFrames() )
mLastOverrun = getContext()->getNumProcessedFrames();
const size_t writePosNew = writePos + numWriteFrames;
// now use compare_exchange to check if write position has been reset by the GUI thread
// if not mWritePos = writePosNew
// otherwise it means mWritePos is zero (from ::start()) so next time ::process() is executed,
// mWritePos will be 0 and theerefore the recording will start from 0
mWritePos.compare_exchange_strong( writePos, writePosNew );

Hope this makes sense...

thanks!

rich.e

Re: [cinder-audio] write position consistency in BufferRecorderNode::process()

4 years ago

Hey, sorry for the delay on this, I'm just getting caught up on outstanding issues..

Thanks for the clarification, I understand the problem that you're bringing to light and think you've found an appropriate fix. Would you like to send a pull request with the proposed changes? Only thing I'd ask is that if you do, try to keep the comment brief (hopefully to one line), as I tend to think in audio processing implementation code it is better to see the math going on rather than lengthy explanations.

Thank you, for helping to improve this code!

Rich

MrBl00m