clean up AudioRingBuffer

libraries/audio/src/AudioRingBuffer.cpp

@@ -24,32 +24,34 @@
 #include "AudioRingBuffer.h"
 
 static const QString RING_BUFFER_OVERFLOW_DEBUG { "AudioRingBuffer::writeData has overflown the buffer. Overwriting old data." };
+static const QString DROPPED_SILENT_DEBUG { "AudioRingBuffer::addSilentSamples dropping silent samples to prevent overflow." };
 
 AudioRingBuffer::AudioRingBuffer(int numFrameSamples, int numFramesCapacity) :
+    _numFrameSamples(numFrameSamples),
     _frameCapacity(numFramesCapacity),
     _sampleCapacity(numFrameSamples * numFramesCapacity),
-    _bufferLength(numFrameSamples * (numFramesCapacity + 1)),
+    _bufferLength(numFrameSamples * (numFramesCapacity + 1))
-    _numFrameSamples(numFrameSamples),
-    _overflowCount(0)
 {
     if (numFrameSamples) {
         _buffer = new int16_t[_bufferLength];
         memset(_buffer, 0, _bufferLength * sizeof(int16_t));
         _nextOutput = _buffer;
         _endOfLastWrite = _buffer;
-    } else {
-        _buffer = NULL;
-        _nextOutput = NULL;
-        _endOfLastWrite = NULL;
     }
 
-    static QString repeatedMessage = LogHandler::getInstance().addRepeatedMessageRegex(RING_BUFFER_OVERFLOW_DEBUG);
+    static QString repeatedOverflowMessage = LogHandler::getInstance().addRepeatedMessageRegex(RING_BUFFER_OVERFLOW_DEBUG);
+    static QString repeatedDroppedMessage = LogHandler::getInstance().addRepeatedMessageRegex(DROPPED_SILENT_DEBUG);
 };
 
 AudioRingBuffer::~AudioRingBuffer() {
     delete[] _buffer;
 }
 
+void AudioRingBuffer::clear() {
+    _endOfLastWrite = _buffer;
+    _nextOutput = _buffer;
+}
+
 void AudioRingBuffer::reset() {
     clear();
     _overflowCount = 0;
@@ -57,93 +59,93 @@ void AudioRingBuffer::reset() {
 
 void AudioRingBuffer::resizeForFrameSize(int numFrameSamples) {
     delete[] _buffer;
+    _numFrameSamples = numFrameSamples;
     _sampleCapacity = numFrameSamples * _frameCapacity;
     _bufferLength = numFrameSamples * (_frameCapacity + 1);
-    _numFrameSamples = numFrameSamples;
+
+    if (numFrameSamples) {
         _buffer = new int16_t[_bufferLength];
         memset(_buffer, 0, _bufferLength * sizeof(int16_t));
-    reset();
+    } else {
+        _buffer = nullptr;
     }
 
-void AudioRingBuffer::clear() {
+    reset();
-    _endOfLastWrite = _buffer;
-    _nextOutput = _buffer;
 }
 
 int AudioRingBuffer::readSamples(int16_t* destination, int maxSamples) {
     return readData((char*)destination, maxSamples * sizeof(int16_t)) / sizeof(int16_t);
 }
 
+int AudioRingBuffer::writeSamples(const int16_t* source, int maxSamples) {
+    return writeData((char*)source, maxSamples * sizeof(int16_t)) / sizeof(int16_t);
+}
+
 int AudioRingBuffer::readData(char *data, int maxSize) {
-
     // only copy up to the number of samples we have available
-    int numReadSamples = std::min((int)(maxSize / sizeof(int16_t)), samplesAvailable());
+    int maxSamples = maxSize / sizeof(int16_t);
-
+    int numReadSamples = std::min(maxSamples, samplesAvailable());
-    // If we're in random access mode, then we consider our number of available read samples slightly
-    // differently. Namely, if anything has been written, we say we have as many samples as they ask for
-    // otherwise we say we have nothing available
 
     if (_nextOutput + numReadSamples > _buffer + _bufferLength) {
         // we're going to need to do two reads to get this data, it wraps around the edge
+        int numSamplesToEnd = (_buffer + _bufferLength) - _nextOutput;
 
         // read to the end of the buffer
-        int numSamplesToEnd = (_buffer + _bufferLength) - _nextOutput;
         memcpy(data, _nextOutput, numSamplesToEnd * sizeof(int16_t));
 
         // read the rest from the beginning of the buffer
         memcpy(data + (numSamplesToEnd * sizeof(int16_t)), _buffer, (numReadSamples - numSamplesToEnd) * sizeof(int16_t));
     } else {
-        // read the data
         memcpy(data, _nextOutput, numReadSamples * sizeof(int16_t));
     }
 
-    // push the position of _nextOutput by the number of samples read
+    shiftReadPosition(numReadSamples);
-    _nextOutput = shiftedPositionAccomodatingWrap(_nextOutput, numReadSamples);
 
     return numReadSamples * sizeof(int16_t);
 }
 
-int AudioRingBuffer::writeSamples(const int16_t* source, int maxSamples) {
-    return writeData((const char*)source, maxSamples * sizeof(int16_t)) / sizeof(int16_t);
-}
-
 int AudioRingBuffer::writeData(const char* data, int maxSize) {
-    // make sure we have enough bytes left for this to be the right amount of audio
+    // only copy up to the number of samples we have capacity for
-    // otherwise we should not copy that data, and leave the buffer pointers where they are
+    int maxSamples = maxSize / sizeof(int16_t);
-    int samplesToCopy = std::min((int)(maxSize / sizeof(int16_t)), _sampleCapacity);
+    int numWriteSamples = std::min(maxSamples, _sampleCapacity);
-
     int samplesRoomFor = _sampleCapacity - samplesAvailable();
-    if (samplesToCopy > samplesRoomFor) {
+
+    if (numWriteSamples > samplesRoomFor) {
         // there's not enough room for this write. erase old data to make room for this new data
-        int samplesToDelete = samplesToCopy - samplesRoomFor;
+        int samplesToDelete = numWriteSamples - samplesRoomFor;
         _nextOutput = shiftedPositionAccomodatingWrap(_nextOutput, samplesToDelete);
         _overflowCount++;
 
         qCDebug(audio) << qPrintable(RING_BUFFER_OVERFLOW_DEBUG);
     }
 
-    if (_endOfLastWrite + samplesToCopy <= _buffer + _bufferLength) {
+    if (_endOfLastWrite + numWriteSamples > _buffer + _bufferLength) {
-        memcpy(_endOfLastWrite, data, samplesToCopy * sizeof(int16_t));
+        // we're going to need to do two writes to set this data, it wraps around the edge
-    } else {
         int numSamplesToEnd = (_buffer + _bufferLength) - _endOfLastWrite;
+
+        // write to the end of the buffer
         memcpy(_endOfLastWrite, data, numSamplesToEnd * sizeof(int16_t));
-        memcpy(_buffer, data + (numSamplesToEnd * sizeof(int16_t)), (samplesToCopy - numSamplesToEnd) * sizeof(int16_t));
+
+        // write the rest to the beginning of the buffer
+        memcpy(_buffer, data + (numSamplesToEnd * sizeof(int16_t)), (numWriteSamples - numSamplesToEnd) * sizeof(int16_t));
+    } else {
+        memcpy(_endOfLastWrite, data, numWriteSamples * sizeof(int16_t));
     }
 
-    _endOfLastWrite = shiftedPositionAccomodatingWrap(_endOfLastWrite, samplesToCopy);
+    _endOfLastWrite = shiftedPositionAccomodatingWrap(_endOfLastWrite, numWriteSamples);
 
-    return samplesToCopy * sizeof(int16_t);
+    return numWriteSamples * sizeof(int16_t);
 }
 
-int16_t& AudioRingBuffer::operator[](const int index) {
+inline int16_t& AudioRingBuffer::operator[](const int index) {
     return *shiftedPositionAccomodatingWrap(_nextOutput, index);
 }
 
-const int16_t& AudioRingBuffer::operator[] (const int index) const {
+inline const int16_t& AudioRingBuffer::operator[] (const int index) const {
     return *shiftedPositionAccomodatingWrap(_nextOutput, index);
 }
 
-void AudioRingBuffer::shiftReadPosition(unsigned int numSamples) {
+inline void AudioRingBuffer::shiftReadPosition(unsigned int numSamples) {
     _nextOutput = shiftedPositionAccomodatingWrap(_nextOutput, numSamples);
 }
 
@@ -160,35 +162,31 @@ int AudioRingBuffer::samplesAvailable() const {
 }
 
 int AudioRingBuffer::addSilentSamples(int silentSamples) {
-
+    // NOTE: This implementation is nearly identical to writeData save for s/memcpy/memset, refer to comments there
+    int numWriteSamples = std::min(silentSamples, _sampleCapacity);
     int samplesRoomFor = _sampleCapacity - samplesAvailable();
-    if (silentSamples > samplesRoomFor) {
-        // there's not enough room for this write. write as many silent samples as we have room for
-        silentSamples = samplesRoomFor;
 
-        static const QString DROPPED_SILENT_DEBUG {
+    if (numWriteSamples > samplesRoomFor) {
-            "AudioRingBuffer::addSilentSamples dropping silent samples to prevent overflow."
+        numWriteSamples = samplesRoomFor;
-        };
+
-        static QString repeatedMessage = LogHandler::getInstance().addRepeatedMessageRegex(DROPPED_SILENT_DEBUG);
         qCDebug(audio) << qPrintable(DROPPED_SILENT_DEBUG);
     }
 
-    // memset zeroes into the buffer, accomodate a wrap around the end
+    if (_endOfLastWrite + numWriteSamples > _buffer + _bufferLength) {
-    // push the _endOfLastWrite to the correct spot
-    if (_endOfLastWrite + silentSamples <= _buffer + _bufferLength) {
-        memset(_endOfLastWrite, 0, silentSamples * sizeof(int16_t));
-    } else {
         int numSamplesToEnd = (_buffer + _bufferLength) - _endOfLastWrite;
         memset(_endOfLastWrite, 0, numSamplesToEnd * sizeof(int16_t));
-        memset(_buffer, 0, (silentSamples - numSamplesToEnd) * sizeof(int16_t));
+        memset(_buffer, 0, (numWriteSamples - numSamplesToEnd) * sizeof(int16_t));
+    } else {
+        memset(_endOfLastWrite, 0, numWriteSamples * sizeof(int16_t));
     }
-    _endOfLastWrite = shiftedPositionAccomodatingWrap(_endOfLastWrite, silentSamples);
 
-    return silentSamples;
+    _endOfLastWrite = shiftedPositionAccomodatingWrap(_endOfLastWrite, numWriteSamples);
+
+    return numWriteSamples;
 }
 
 int16_t* AudioRingBuffer::shiftedPositionAccomodatingWrap(int16_t* position, int numSamplesShift) const {
-
+    // NOTE: It is possible to shift out-of-bounds if (|numSamplesShift| > 2 * _bufferLength), but this should not occur
     if (numSamplesShift > 0 && position + numSamplesShift >= _buffer + _bufferLength) {
         // this shift will wrap the position around to the beginning of the ring
         return position + numSamplesShift - _bufferLength;
@@ -203,11 +201,12 @@ int16_t* AudioRingBuffer::shiftedPositionAccomodatingWrap(int16_t* position, int
 float AudioRingBuffer::getFrameLoudness(const int16_t* frameStart) const {
     float loudness = 0.0f;
     const int16_t* sampleAt = frameStart;
-    const int16_t* _bufferLastAt = _buffer + _bufferLength - 1;
+    const int16_t* bufferLastAt = _buffer + _bufferLength - 1;
 
     for (int i = 0; i < _numFrameSamples; ++i) {
         loudness += (float) std::abs(*sampleAt);
-        sampleAt = sampleAt == _bufferLastAt ? _buffer : sampleAt + 1;
+        // wrap if necessary
+        sampleAt = sampleAt == bufferLastAt ? _buffer : sampleAt + 1;
     }
     loudness /= _numFrameSamples;
     loudness /= AudioConstants::MAX_SAMPLE_VALUE;
@@ -222,10 +221,6 @@ float AudioRingBuffer::getFrameLoudness(ConstIterator frameStart) const {
     return getFrameLoudness(&(*frameStart));
 }
 
-float AudioRingBuffer::getNextOutputFrameLoudness() const {
-    return getFrameLoudness(_nextOutput);
-}
-
 int AudioRingBuffer::writeSamples(ConstIterator source, int maxSamples) {
     int samplesToCopy = std::min(maxSamples, _sampleCapacity);
     int samplesRoomFor = _sampleCapacity - samplesAvailable();

libraries/audio/src/AudioRingBuffer.h

@@ -26,69 +26,66 @@ public:
     AudioRingBuffer(int numFrameSamples, int numFramesCapacity = DEFAULT_RING_BUFFER_FRAME_CAPACITY);
     ~AudioRingBuffer();
 
-    void reset();
+    // disallow copying
-    void resizeForFrameSize(int numFrameSamples);
+    AudioRingBuffer(const AudioRingBuffer&) = delete;
+    AudioRingBuffer(AudioRingBuffer&&) = delete;
+    AudioRingBuffer& operator=(const AudioRingBuffer&) = delete;
 
+    /// Invalidate any data in the buffer
     void clear();
 
-    int getSampleCapacity() const { return _sampleCapacity; }
+    /// Clear and reset the overflow count
-    int getFrameCapacity() const { return _frameCapacity; }
+    void reset();
 
+    /// Resize frame size (causes a reset())
+    // FIXME: discards any data in the buffer
+    void resizeForFrameSize(int numFrameSamples);
+
+    /// Read up to maxSamples into destination (will only read up to samplesAvailable())
+    /// Returns number of read samples
     int readSamples(int16_t* destination, int maxSamples);
+
+    /// Write up to maxSamples from source (will only write up to sample capacity)
+    /// Returns number of written samples
     int writeSamples(const int16_t* source, int maxSamples);
 
-    int readData(char* data, int maxSize);
+    /// Write up to maxSamples silent samples (will only write until other data exists in the buffer)
-    int writeData(const char* data, int maxSize);
+    /// This method will not overwrite existing data in the buffer, instead dropping silent samples that would overflow
+    /// Returns number of written silent samples
+    int addSilentSamples(int maxSamples);
 
+    /// Read up to maxSize into destination
+    /// Returns number of read bytes
+    int readData(char* destination, int maxSize);
+
+    /// Write up to maxSize from source
+    /// Returns number of written bytes
+    int writeData(const char* source, int maxSize);
+
+    /// Returns a reference to the index-th sample offset from the current read sample
     int16_t& operator[](const int index);
     const int16_t& operator[] (const int index) const;
 
+    /// Essentially discards the next numSamples from the ring buffer
+    /// NOTE: This is not checked - it is possible to shift past written data
+    ///       Use samplesAvailable() to see the distance a valid shift can go
     void shiftReadPosition(unsigned int numSamples);
 
-    float getNextOutputFrameLoudness() const;
-
     int samplesAvailable() const;
     int framesAvailable() const { return (_numFrameSamples == 0) ? 0 : samplesAvailable() / _numFrameSamples; }
+    float getNextOutputFrameLoudness() const { return getFrameLoudness(_nextOutput); }
+
 
     int getNumFrameSamples() const { return _numFrameSamples; }
+    int getFrameCapacity() const { return _frameCapacity; }
+    int getSampleCapacity() const { return _sampleCapacity; }
+    /// Return times the ring buffer has overwritten old data
+    int getOverflowCount() const { return _overflowCount; }
 
-    int getOverflowCount() const { return _overflowCount; } /// how many times has the ring buffer has overwritten old data
+    class ConstIterator {
-
-    int addSilentSamples(int samples);
-
-private:
-    float getFrameLoudness(const int16_t* frameStart) const;
-
-protected:
-    // disallow copying of AudioRingBuffer objects
-    AudioRingBuffer(const AudioRingBuffer&);
-    AudioRingBuffer& operator= (const AudioRingBuffer&);
-
-    int16_t* shiftedPositionAccomodatingWrap(int16_t* position, int numSamplesShift) const;
-
-    int _frameCapacity;
-    int _sampleCapacity;
-    int _bufferLength;      // actual length of _buffer: will be one frame larger than _sampleCapacity
-    int _numFrameSamples;
-    int16_t* _nextOutput;
-    int16_t* _endOfLastWrite;
-    int16_t* _buffer;
-
-    int _overflowCount; /// how many times has the ring buffer has overwritten old data
-
     public:
-    class ConstIterator { //public std::iterator < std::forward_iterator_tag, int16_t > {
+        ConstIterator();
-    public:
+        ConstIterator(int16_t* bufferFirst, int capacity, int16_t* at);
-        ConstIterator()
-            : _bufferLength(0),
-            _bufferFirst(NULL),
-            _bufferLast(NULL),
-            _at(NULL) {}
-        ConstIterator(int16_t* bufferFirst, int capacity, int16_t* at)
-            : _bufferLength(capacity),
-            _bufferFirst(bufferFirst),
-            _bufferLast(bufferFirst + capacity - 1),
-            _at(at) {}
         ConstIterator(const ConstIterator& rhs) = default;
 
         bool isNull() const { return _at == NULL; }
@@ -97,7 +94,64 @@ public:
         bool operator!=(const ConstIterator& rhs) { return _at != rhs._at; }
         const int16_t& operator*() { return *_at; }
 
-        ConstIterator& operator=(const ConstIterator& rhs) {
+        ConstIterator& operator=(const ConstIterator& rhs);
+        ConstIterator& operator++();
+        ConstIterator operator++(int);
+        ConstIterator& operator--();
+        ConstIterator operator--(int);
+        const int16_t& operator[] (int i);
+        ConstIterator operator+(int i);
+        ConstIterator operator-(int i);
+
+        void readSamples(int16_t* dest, int numSamples);
+        void readSamplesWithFade(int16_t* dest, int numSamples, float fade);
+
+    private:
+        int16_t* atShiftedBy(int i);
+
+        int _bufferLength;
+        int16_t* _bufferFirst;
+        int16_t* _bufferLast;
+        int16_t* _at;
+    };
+
+    ConstIterator nextOutput() const;
+    ConstIterator lastFrameWritten() const;
+
+    int writeSamples(ConstIterator source, int maxSamples);
+    int writeSamplesWithFade(ConstIterator source, int maxSamples, float fade);
+
+    float getFrameLoudness(ConstIterator frameStart) const;
+
+protected:
+    int16_t* shiftedPositionAccomodatingWrap(int16_t* position, int numSamplesShift) const;
+    float getFrameLoudness(const int16_t* frameStart) const;
+
+    int _numFrameSamples;
+    int _frameCapacity;
+    int _sampleCapacity;
+    int _bufferLength; // actual _buffer length (_sampleCapacity + 1)
+    int _overflowCount{ 0 }; // times the ring buffer has overwritten data
+
+    int16_t* _nextOutput{ nullptr };
+    int16_t* _endOfLastWrite{ nullptr };
+    int16_t* _buffer{ nullptr };
+};
+
+// inline the iterator:
+inline AudioRingBuffer::ConstIterator::ConstIterator() :
+    _bufferLength(0),
+    _bufferFirst(NULL),
+    _bufferLast(NULL),
+    _at(NULL) {}
+
+inline AudioRingBuffer::ConstIterator::ConstIterator(int16_t* bufferFirst, int capacity, int16_t* at) :
+    _bufferLength(capacity),
+    _bufferFirst(bufferFirst),
+    _bufferLast(bufferFirst + capacity - 1),
+    _at(at) {}
+
+inline AudioRingBuffer::ConstIterator& AudioRingBuffer::ConstIterator::operator=(const ConstIterator& rhs) {
     _bufferLength = rhs._bufferLength;
     _bufferFirst = rhs._bufferFirst;
     _bufferLast = rhs._bufferLast;
@@ -105,41 +159,49 @@ public:
     return *this;
 }
 
-        ConstIterator& operator++() {
+inline AudioRingBuffer::ConstIterator& AudioRingBuffer::ConstIterator::operator++() {
     _at = (_at == _bufferLast) ? _bufferFirst : _at + 1;
     return *this;
 }
 
-        ConstIterator operator++(int) {
+inline AudioRingBuffer::ConstIterator AudioRingBuffer::ConstIterator::operator++(int) {
     ConstIterator tmp(*this);
     ++(*this);
     return tmp;
 }
 
-        ConstIterator& operator--() {
+inline AudioRingBuffer::ConstIterator& AudioRingBuffer::ConstIterator::operator--() {
     _at = (_at == _bufferFirst) ? _bufferLast : _at - 1;
     return *this;
 }
 
-        ConstIterator operator--(int) {
+inline AudioRingBuffer::ConstIterator AudioRingBuffer::ConstIterator::operator--(int) {
     ConstIterator tmp(*this);
     --(*this);
     return tmp;
 }
 
-        const int16_t& operator[] (int i) {
+inline const int16_t& AudioRingBuffer::ConstIterator::operator[] (int i) {
     return *atShiftedBy(i);
 }
 
-        ConstIterator operator+(int i) {
+inline AudioRingBuffer::ConstIterator AudioRingBuffer::ConstIterator::operator+(int i) {
     return ConstIterator(_bufferFirst, _bufferLength, atShiftedBy(i));
 }
 
-        ConstIterator operator-(int i) {
+inline AudioRingBuffer::ConstIterator AudioRingBuffer::ConstIterator::operator-(int i) {
     return ConstIterator(_bufferFirst, _bufferLength, atShiftedBy(-i));
 }
 
-        void readSamples(int16_t* dest, int numSamples) {
+inline int16_t* AudioRingBuffer::ConstIterator::atShiftedBy(int i) {
+    i = (_at - _bufferFirst + i) % _bufferLength;
+    if (i < 0) {
+        i += _bufferLength;
+    }
+    return _bufferFirst + i;
+}
+
+inline void AudioRingBuffer::ConstIterator::readSamples(int16_t* dest, int numSamples) {
     auto samplesToEnd = _bufferLast - _at + 1;
 
     if (samplesToEnd >= numSamples) {
@@ -154,7 +216,7 @@ public:
     }
 }
 
-        void readSamplesWithFade(int16_t* dest, int numSamples, float fade) {
+inline void AudioRingBuffer::ConstIterator::readSamplesWithFade(int16_t* dest, int numSamples, float fade) {
     int16_t* at = _at;
     for (int i = 0; i < numSamples; i++) {
         *dest = (float)*at * fade;
@@ -163,29 +225,12 @@ public:
     }
 }
 
-    private:
+inline AudioRingBuffer::ConstIterator AudioRingBuffer::nextOutput() const {
-        int16_t* atShiftedBy(int i) {
+    return ConstIterator(_buffer, _bufferLength, _nextOutput);
-            i = (_at - _bufferFirst + i) % _bufferLength;
-            if (i < 0) {
-                i += _bufferLength;
-            }
-            return _bufferFirst + i;
 }
 
-    private:
+inline AudioRingBuffer::ConstIterator AudioRingBuffer::lastFrameWritten() const {
-        int _bufferLength;
+    return ConstIterator(_buffer, _bufferLength, _endOfLastWrite) - _numFrameSamples;
-        int16_t* _bufferFirst;
+}
-        int16_t* _bufferLast;
-        int16_t* _at;
-    };
-
-    ConstIterator nextOutput() const { return ConstIterator(_buffer, _bufferLength, _nextOutput); }
-    ConstIterator lastFrameWritten() const { return ConstIterator(_buffer, _bufferLength, _endOfLastWrite) - _numFrameSamples; }
-
-    float getFrameLoudness(ConstIterator frameStart) const;
-
-    int writeSamples(ConstIterator source, int maxSamples);
-    int writeSamplesWithFade(ConstIterator source, int maxSamples, float fade);
-};
 
 #endif // hifi_AudioRingBuffer_h