Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8f4fe50d authored by Phil Burk's avatar Phil Burk
Browse files

aaudio: refactor references to shared FIFO

Reduce passing of raw pointers.
Use shared_ptrs and unique_ptrs.
Simplify memory management.

Refactor FifoBuffer into two subclasses so that
the internal memory management is simpler.

Bug: 151650670
Test: OboeTester "TEST OUTPUT"
Test: disable EXCLUSIVE mode
Test: OboeTester "ECHO INPUT TO OUTPUT"
Test: disable INPUT EXCLUSIVE mode
Change-Id: I10981767f8719f3cc3525df211285c53219a7979
parent a524c363
Loading
Loading
Loading
Loading
+2 −2
Original line number Original line Diff line number Diff line
@@ -137,7 +137,7 @@ aaudio_result_t AudioEndpoint::configure(const EndpointDescriptor *pEndpointDesc
        return AAUDIO_ERROR_INTERNAL;
        return AAUDIO_ERROR_INTERNAL;
    }
    }


    mUpCommandQueue = std::make_unique<FifoBuffer>(
    mUpCommandQueue = std::make_unique<FifoBufferIndirect>(
            descriptor->bytesPerFrame,
            descriptor->bytesPerFrame,
            descriptor->capacityInFrames,
            descriptor->capacityInFrames,
            descriptor->readCounterAddress,
            descriptor->readCounterAddress,
@@ -166,7 +166,7 @@ aaudio_result_t AudioEndpoint::configure(const EndpointDescriptor *pEndpointDesc
                                  ? &mDataWriteCounter
                                  ? &mDataWriteCounter
                                  : descriptor->writeCounterAddress;
                                  : descriptor->writeCounterAddress;


    mDataQueue = std::make_unique<FifoBuffer>(
    mDataQueue = std::make_unique<FifoBufferIndirect>(
            descriptor->bytesPerFrame,
            descriptor->bytesPerFrame,
            descriptor->capacityInFrames,
            descriptor->capacityInFrames,
            readCounterAddress,
            readCounterAddress,
+2 −2
Original line number Original line Diff line number Diff line
@@ -93,8 +93,8 @@ public:
    void dump() const;
    void dump() const;


private:
private:
    std::unique_ptr<android::FifoBuffer> mUpCommandQueue;
    std::unique_ptr<android::FifoBufferIndirect> mUpCommandQueue;
    std::unique_ptr<android::FifoBuffer> mDataQueue;
    std::unique_ptr<android::FifoBufferIndirect> mDataQueue;
    bool                    mFreeRunning;
    bool                    mFreeRunning;
    android::fifo_counter_t mDataReadCounter; // only used if free-running
    android::fifo_counter_t mDataReadCounter; // only used if free-running
    android::fifo_counter_t mDataWriteCounter; // only used if free-running
    android::fifo_counter_t mDataWriteCounter; // only used if free-running
+17 −19
Original line number Original line Diff line number Diff line
@@ -31,40 +31,37 @@
#include "FifoBuffer.h"
#include "FifoBuffer.h"


using android::FifoBuffer;
using android::FifoBuffer;
using android::FifoBufferAllocated;
using android::FifoBufferIndirect;
using android::fifo_frames_t;
using android::fifo_frames_t;


FifoBuffer::FifoBuffer(int32_t bytesPerFrame, fifo_frames_t capacityInFrames)
FifoBuffer::FifoBuffer(int32_t bytesPerFrame)
        : mBytesPerFrame(bytesPerFrame)
        : mBytesPerFrame(bytesPerFrame) {}

FifoBufferAllocated::FifoBufferAllocated(int32_t bytesPerFrame, fifo_frames_t capacityInFrames)
        : FifoBuffer(bytesPerFrame)
{
{
    mFifo = std::make_unique<FifoController>(capacityInFrames, capacityInFrames);
    mFifo = std::make_unique<FifoController>(capacityInFrames, capacityInFrames);
    // allocate buffer
    // allocate buffer
    int32_t bytesPerBuffer = bytesPerFrame * capacityInFrames;
    int32_t bytesPerBuffer = bytesPerFrame * capacityInFrames;
    mStorage = new uint8_t[bytesPerBuffer];
    mInternalStorage = std::make_unique<uint8_t[]>(bytesPerBuffer);
    mStorageOwned = true;
    ALOGV("%s() capacityInFrames = %d, bytesPerFrame = %d",
    ALOGV("%s() capacityInFrames = %d, bytesPerFrame = %d",
          __func__, capacityInFrames, bytesPerFrame);
          __func__, capacityInFrames, bytesPerFrame);
}
}


FifoBuffer::FifoBuffer( int32_t   bytesPerFrame,
FifoBufferIndirect::FifoBufferIndirect( int32_t   bytesPerFrame,
                        fifo_frames_t   capacityInFrames,
                        fifo_frames_t   capacityInFrames,
                        fifo_counter_t *readIndexAddress,
                        fifo_counter_t *readIndexAddress,
                        fifo_counter_t *writeIndexAddress,
                        fifo_counter_t *writeIndexAddress,
                        void *  dataStorageAddress
                        void *  dataStorageAddress
                        )
                        )
        : mBytesPerFrame(bytesPerFrame)
        : FifoBuffer(bytesPerFrame)
        , mStorage(static_cast<uint8_t *>(dataStorageAddress))
        , mExternalStorage(static_cast<uint8_t *>(dataStorageAddress))
{
{
    mFifo = std::make_unique<FifoControllerIndirect>(capacityInFrames,
    mFifo = std::make_unique<FifoControllerIndirect>(capacityInFrames,
                                       capacityInFrames,
                                       capacityInFrames,
                                       readIndexAddress,
                                       readIndexAddress,
                                       writeIndexAddress);
                                       writeIndexAddress);
    mStorageOwned = false;
}

FifoBuffer::~FifoBuffer() {
    if (mStorageOwned) {
        delete[] mStorage;
    }
}
}


int32_t FifoBuffer::convertFramesToBytes(fifo_frames_t frames) {
int32_t FifoBuffer::convertFramesToBytes(fifo_frames_t frames) {
@@ -76,15 +73,16 @@ void FifoBuffer::fillWrappingBuffer(WrappingBuffer *wrappingBuffer,
                                    int32_t startIndex) {
                                    int32_t startIndex) {
    wrappingBuffer->data[1] = nullptr;
    wrappingBuffer->data[1] = nullptr;
    wrappingBuffer->numFrames[1] = 0;
    wrappingBuffer->numFrames[1] = 0;
    uint8_t *storage = getStorage();
    if (framesAvailable > 0) {
    if (framesAvailable > 0) {
        fifo_frames_t capacity = mFifo->getCapacity();
        fifo_frames_t capacity = mFifo->getCapacity();
        uint8_t *source = &mStorage[convertFramesToBytes(startIndex)];
        uint8_t *source = &storage[convertFramesToBytes(startIndex)];
        // Does the available data cross the end of the FIFO?
        // Does the available data cross the end of the FIFO?
        if ((startIndex + framesAvailable) > capacity) {
        if ((startIndex + framesAvailable) > capacity) {
            wrappingBuffer->data[0] = source;
            wrappingBuffer->data[0] = source;
            fifo_frames_t firstFrames = capacity - startIndex;
            fifo_frames_t firstFrames = capacity - startIndex;
            wrappingBuffer->numFrames[0] = firstFrames;
            wrappingBuffer->numFrames[0] = firstFrames;
            wrappingBuffer->data[1] = &mStorage[0];
            wrappingBuffer->data[1] = &storage[0];
            wrappingBuffer->numFrames[1] = framesAvailable - firstFrames;
            wrappingBuffer->numFrames[1] = framesAvailable - firstFrames;
        } else {
        } else {
            wrappingBuffer->data[0] = source;
            wrappingBuffer->data[0] = source;
@@ -191,6 +189,6 @@ fifo_frames_t FifoBuffer::getBufferCapacityInFrames() {
void FifoBuffer::eraseMemory() {
void FifoBuffer::eraseMemory() {
    int32_t numBytes = convertFramesToBytes(getBufferCapacityInFrames());
    int32_t numBytes = convertFramesToBytes(getBufferCapacityInFrames());
    if (numBytes > 0) {
    if (numBytes > 0) {
        memset(mStorage, 0, (size_t) numBytes);
        memset(getStorage(), 0, (size_t) numBytes);
    }
    }
}
}
+41 −13
Original line number Original line Diff line number Diff line
@@ -38,15 +38,9 @@ struct WrappingBuffer {


class FifoBuffer {
class FifoBuffer {
public:
public:
    FifoBuffer(int32_t bytesPerFrame, fifo_frames_t capacityInFrames);
    FifoBuffer(int32_t bytesPerFrame);


    FifoBuffer(int32_t bytesPerFrame,
    virtual ~FifoBuffer() = default;
               fifo_frames_t capacityInFrames,
               fifo_counter_t *readCounterAddress,
               fifo_counter_t *writeCounterAddress,
               void *dataStorageAddress);

    ~FifoBuffer();


    int32_t convertFramesToBytes(fifo_frames_t frames);
    int32_t convertFramesToBytes(fifo_frames_t frames);


@@ -121,19 +115,53 @@ public:
     */
     */
    void eraseMemory();
    void eraseMemory();


private:
protected:

    virtual uint8_t *getStorage() const = 0;


    void fillWrappingBuffer(WrappingBuffer *wrappingBuffer,
    void fillWrappingBuffer(WrappingBuffer *wrappingBuffer,
                            int32_t framesAvailable, int32_t startIndex);
                            int32_t framesAvailable, int32_t startIndex);


    const int32_t             mBytesPerFrame;
    const int32_t             mBytesPerFrame;
    // We do not use a std::unique_ptr for mStorage because it is often a pointer to
    // memory shared between processes and cannot be deleted trivially.
    uint8_t                  *mStorage = nullptr;
    bool                      mStorageOwned = false; // did this object allocate the storage?
    std::unique_ptr<FifoControllerBase> mFifo{};
    std::unique_ptr<FifoControllerBase> mFifo{};
};
};


// Define two subclasses to handle the two ways that storage is allocated.

// Allocate storage internally.
class FifoBufferAllocated : public FifoBuffer {
public:
    FifoBufferAllocated(int32_t bytesPerFrame, fifo_frames_t capacityInFrames);

private:

    uint8_t *getStorage() const override {
        return mInternalStorage.get();
    };

    std::unique_ptr<uint8_t[]> mInternalStorage;
};

// Allocate storage externally and pass it in.
class FifoBufferIndirect : public FifoBuffer {
public:
    // We use raw pointers because the memory may be
    // in the middle of an allocated block and cannot be deleted directly.
    FifoBufferIndirect(int32_t bytesPerFrame,
                       fifo_frames_t capacityInFrames,
                       fifo_counter_t* readCounterAddress,
                       fifo_counter_t* writeCounterAddress,
                       void* dataStorageAddress);

private:

    uint8_t *getStorage() const override {
        return mExternalStorage;
    };

    uint8_t *mExternalStorage = nullptr;
};

}  // android
}  // android


#endif //FIFO_FIFO_BUFFER_H
#endif //FIFO_FIFO_BUFFER_H
+1 −1
Original line number Original line Diff line number Diff line
@@ -27,7 +27,7 @@ namespace android {
/**
/**
 * A FifoControllerBase with counters external to the class.
 * A FifoControllerBase with counters external to the class.
 *
 *
 * The actual copunters may be stored in separate regions of shared memory
 * The actual counters may be stored in separate regions of shared memory
 * with different access rights.
 * with different access rights.
 */
 */
class FifoControllerIndirect : public FifoControllerBase {
class FifoControllerIndirect : public FifoControllerBase {
Loading