Fix "out of buffers" crash (#3783)

Remove BufferManager implementation. Use MemoryManager allocation instead and re-use buffers where they are allocated (AudioPort.cpp & PlayHandle.cpp)
This commit is contained in:
Lukas W
2017-09-26 20:33:09 +02:00
committed by GitHub
parent dd429c5caf
commit f23cf4e0bf
9 changed files with 60 additions and 116 deletions

View File

@@ -30,9 +30,6 @@
#include "export.h"
#include "lmms_basics.h"
const int BM_INITIAL_BUFFERS = 512;
//const int BM_INCREMENT = 64;
class EXPORT BufferManager
{
public:
@@ -46,17 +43,6 @@ public:
const f_cnt_t offset = 0 );
#endif
static void release( sampleFrame * buf );
static void refresh();
// static void extend( int c );
private:
static sampleFrame ** s_available;
static AtomicInt s_availableIndex;
static sampleFrame ** s_released;
static AtomicInt s_releasedIndex;
// static QReadWriteLock s_mutex;
static int s_size;
};
#endif

View File

@@ -31,7 +31,7 @@
class MemoryHelper {
public:
static void* alignedMalloc( int );
static void* alignedMalloc( size_t );
static void alignedFree( void* );

View File

@@ -42,7 +42,7 @@ struct MemoryPool
{
void * m_pool;
char * m_free;
int m_chunks;
size_t m_chunks;
QMutex m_mutex;
MemoryPool() :
@@ -51,10 +51,10 @@ struct MemoryPool
m_chunks( 0 )
{}
MemoryPool( int chunks ) :
MemoryPool( size_t chunks ) :
m_chunks( chunks )
{
m_free = (char*) MemoryHelper::alignedMalloc( chunks );
m_free = reinterpret_cast<char*>( MemoryHelper::alignedMalloc( chunks ) );
memset( m_free, 1, chunks );
}
@@ -103,6 +103,25 @@ private:
static QMutex s_pointerMutex;
};
template<typename T>
struct MmAllocator
{
typedef T value_type;
template<class U> struct rebind { typedef MmAllocator<U> other; };
T* allocate( std::size_t n )
{
return reinterpret_cast<T*>( MemoryManager::alloc( sizeof(T) * n ) );
}
void deallocate( T* p, std::size_t )
{
MemoryManager::free( p );
}
typedef std::vector<T, MmAllocator<T> > vector;
};
#define MM_OPERATORS \
public: \
@@ -124,7 +143,7 @@ static void operator delete[] ( void * ptr ) \
}
// for use in cases where overriding new/delete isn't a possibility
#define MM_ALLOC( type, count ) (type*) MemoryManager::alloc( sizeof( type ) * count )
#define MM_ALLOC( type, count ) reinterpret_cast<type*>( MemoryManager::alloc( sizeof( type ) * count ) )
// and just for symmetry...
#define MM_FREE( ptr ) MemoryManager::free( ptr )

View File

@@ -28,6 +28,8 @@
#include <QtCore/QList>
#include <QtCore/QMutex>
#include "MemoryManager.h"
#include "ThreadableJob.h"
#include "lmms_basics.h"
@@ -142,20 +144,17 @@ public:
void releaseBuffer();
sampleFrame * buffer()
{
return m_playHandleBuffer;
}
sampleFrame * buffer();
private:
Type m_type;
f_cnt_t m_offset;
QThread* m_affinity;
QMutex m_processingLock;
sampleFrame * m_playHandleBuffer;
sampleFrame* m_playHandleBuffer;
bool m_bufferReleased;
bool m_usesBuffer;
AudioPort * m_audioPort;
} ;

View File

@@ -1,6 +1,7 @@
/*
* BufferManager.cpp - A buffer caching/memory management system
*
* Copyright (c) 2017 Lukas W <lukaswhl/at/gmail.com>
* Copyright (c) 2014 Vesa Kivimäki <contact/dot/diizy/at/nbl/dot/fi>
* Copyright (c) 2006-2014 Tobias Doerffel <tobydox/at/users.sourceforge.net>
*
@@ -25,56 +26,28 @@
#include "BufferManager.h"
#include "Engine.h"
#include "Mixer.h"
#include "MemoryManager.h"
sampleFrame ** BufferManager::s_available;
AtomicInt BufferManager::s_availableIndex = 0;
sampleFrame ** BufferManager::s_released;
AtomicInt BufferManager::s_releasedIndex = 0;
//QReadWriteLock BufferManager::s_mutex;
int BufferManager::s_size;
static fpp_t framesPerPeriod;
void BufferManager::init( fpp_t framesPerPeriod )
{
s_available = MM_ALLOC( sampleFrame*, BM_INITIAL_BUFFERS );
s_released = MM_ALLOC( sampleFrame*, BM_INITIAL_BUFFERS );
int c = framesPerPeriod * BM_INITIAL_BUFFERS;
sampleFrame * b = MM_ALLOC( sampleFrame, c );
for( int i = 0; i < BM_INITIAL_BUFFERS; ++i )
{
s_available[ i ] = b;
b += framesPerPeriod;
}
s_availableIndex = BM_INITIAL_BUFFERS - 1;
s_size = BM_INITIAL_BUFFERS;
::framesPerPeriod = framesPerPeriod;
}
sampleFrame * BufferManager::acquire()
{
if( s_availableIndex < 0 )
{
qFatal( "BufferManager: out of buffers" );
}
int i = s_availableIndex.fetchAndAddOrdered( -1 );
sampleFrame * b = s_available[ i ];
//qDebug( "acquired buffer: %p - index %d", b, i );
return b;
return MM_ALLOC( sampleFrame, ::framesPerPeriod );
}
void BufferManager::clear( sampleFrame * ab, const f_cnt_t frames,
const f_cnt_t offset )
void BufferManager::clear( sampleFrame *ab, const f_cnt_t frames, const f_cnt_t offset )
{
memset( ab + offset, 0, sizeof( *ab ) * frames );
}
#ifndef LMMS_DISABLE_SURROUND
void BufferManager::clear( surroundSampleFrame * ab, const f_cnt_t frames,
const f_cnt_t offset )
@@ -86,43 +59,6 @@ void BufferManager::clear( surroundSampleFrame * ab, const f_cnt_t frames,
void BufferManager::release( sampleFrame * buf )
{
if (buf == nullptr) return;
int i = s_releasedIndex.fetchAndAddOrdered( 1 );
s_released[ i ] = buf;
//qDebug( "released buffer: %p - index %d", buf, i );
MM_FREE( buf );
}
void BufferManager::refresh() // non-threadsafe, hence it's called periodically from mixer at a time when no other threads can interfere
{
if( s_releasedIndex == 0 ) return;
//qDebug( "refresh: %d buffers", int( s_releasedIndex ) );
int j = s_availableIndex;
for( int i = 0; i < s_releasedIndex; ++i )
{
++j;
s_available[ j ] = s_released[ i ];
}
s_availableIndex = j;
s_releasedIndex = 0;
}
/* // non-extensible for now
void BufferManager::extend( int c )
{
s_size += c;
sampleFrame ** tmp = MM_ALLOC( sampleFrame*, s_size );
MM_FREE( s_available );
s_available = tmp;
int cc = c * Engine::mixer()->framesPerPeriod();
sampleFrame * b = MM_ALLOC( sampleFrame, cc );
for( int i = 0; i < c; ++i )
{
s_available[ s_availableIndex.fetchAndAddOrdered( 1 ) + 1 ] = b;
b += Engine::mixer()->framesPerPeriod();
}
}*/

View File

@@ -30,7 +30,7 @@
* Allocate a number of bytes and return them.
* @param byteNum is the number of bytes
*/
void* MemoryHelper::alignedMalloc( int byteNum )
void* MemoryHelper::alignedMalloc( size_t byteNum )
{
char *ptr, *ptr2, *aligned_ptr;
int align_mask = ALIGN_SIZE - 1;

View File

@@ -482,9 +482,6 @@ const surroundSampleFrame * Mixer::renderNextBuffer()
Controller::triggerFrameCounter();
AutomatableModel::incrementPeriodCounter();
// refresh buffer pool
BufferManager::refresh();
s_renderingThread = false;
m_profiler.finishPeriod( processingSampleRate(), m_framesPerPeriod );

View File

@@ -24,16 +24,21 @@
#include "PlayHandle.h"
#include "BufferManager.h"
#include "Engine.h"
#include "Mixer.h"
#include <QtCore/QThread>
#include <QDebug>
#include <iterator>
PlayHandle::PlayHandle( const Type type, f_cnt_t offset ) :
m_type( type ),
m_offset( offset ),
m_affinity( QThread::currentThread() ),
m_playHandleBuffer( NULL ),
m_usesBuffer( true )
PlayHandle::PlayHandle(const Type type, f_cnt_t offset) :
m_type(type),
m_offset(offset),
m_affinity(QThread::currentThread()),
m_playHandleBuffer(BufferManager::acquire()),
m_bufferReleased(true),
m_usesBuffer(true)
{
}
@@ -48,8 +53,8 @@ void PlayHandle::doProcessing()
{
if( m_usesBuffer )
{
if( ! m_playHandleBuffer ) m_playHandleBuffer = BufferManager::acquire();
play( m_playHandleBuffer );
m_bufferReleased = false;
play( buffer() );
}
else
{
@@ -60,6 +65,10 @@ void PlayHandle::doProcessing()
void PlayHandle::releaseBuffer()
{
BufferManager::release( m_playHandleBuffer );
m_playHandleBuffer = NULL;
m_bufferReleased = true;
}
sampleFrame* PlayHandle::buffer()
{
return m_bufferReleased ? nullptr : reinterpret_cast<sampleFrame*>(m_playHandleBuffer);
};

View File

@@ -36,7 +36,7 @@ AudioPort::AudioPort( const QString & _name, bool _has_effect_chain,
FloatModel * volumeModel, FloatModel * panningModel,
BoolModel * mutedModel ) :
m_bufferUsage( false ),
m_portBuffer( NULL ),
m_portBuffer( BufferManager::acquire() ),
m_extOutputEnabled( false ),
m_nextFxChannel( 0 ),
m_name( "unnamed port" ),
@@ -57,6 +57,7 @@ AudioPort::~AudioPort()
setExtOutputEnabled( false );
Engine::mixer()->removeAudioPort( this );
delete m_effects;
BufferManager::release( m_portBuffer );
}
@@ -110,8 +111,7 @@ void AudioPort::doProcessing()
const fpp_t fpp = Engine::mixer()->framesPerPeriod();
// get a buffer for processing and clear it
m_portBuffer = BufferManager::acquire();
// clear the buffer
BufferManager::clear( m_portBuffer, fpp );
//qDebug( "Playhandles: %d", m_playHandles.size() );
@@ -225,8 +225,6 @@ void AudioPort::doProcessing()
// TODO: improve the flow here - convert to pull model
m_bufferUsage = false;
}
BufferManager::release( m_portBuffer ); // release buffer, we don't need it anymore
}