Add CopyBufferProvider class for AudioMixer

AudioMixer::ReformatBufferProvider now uses it as a base class.

Fix ReformatBufferProvider object leak.

Change-Id: If196f844eaaa124a173ffa27afe88098023c2ff9

2 files changed, 172 insertions, 114 deletions

diff --git a/services/audioflinger/AudioMixer.cpp b/services/audioflinger/AudioMixer.cpp
index e57cb8a..319d4a8 100644
--- a/services/audioflinger/AudioMixer.cpp
+++ b/services/audioflinger/AudioMixer.cpp
@@ -71,52 +71,12 @@ static const bool kUseNewMixer = false;
 // because of downmix/upmix support.
 static const bool kUseFloat = true;
 
+// Set to default copy buffer size in frames for input processing.
+static const size_t kCopyBufferFrameCount = 256;
+
 namespace android {
 
 // ----------------------------------------------------------------------------
-AudioMixer::DownmixerBufferProvider::DownmixerBufferProvider() : AudioBufferProvider(),
-        mTrackBufferProvider(NULL), mDownmixHandle(NULL)
-{
-}
-
-AudioMixer::DownmixerBufferProvider::~DownmixerBufferProvider()
-{
-    ALOGV("AudioMixer deleting DownmixerBufferProvider (%p)", this);
-    EffectRelease(mDownmixHandle);
-}
-
-status_t AudioMixer::DownmixerBufferProvider::getNextBuffer(AudioBufferProvider::Buffer *pBuffer,
-        int64_t pts) {
-    //ALOGV("DownmixerBufferProvider::getNextBuffer()");
-    if (mTrackBufferProvider != NULL) {
-        status_t res = mTrackBufferProvider->getNextBuffer(pBuffer, pts);
-        if (res == OK) {
-            mDownmixConfig.inputCfg.buffer.frameCount = pBuffer->frameCount;
-            mDownmixConfig.inputCfg.buffer.raw = pBuffer->raw;
-            mDownmixConfig.outputCfg.buffer.frameCount = pBuffer->frameCount;
-            mDownmixConfig.outputCfg.buffer.raw = mDownmixConfig.inputCfg.buffer.raw;
-            // in-place so overwrite the buffer contents, has been set in prepareTrackForDownmix()
-            //mDownmixConfig.outputCfg.accessMode = EFFECT_BUFFER_ACCESS_WRITE;
-
-            res = (*mDownmixHandle)->process(mDownmixHandle,
-                    &mDownmixConfig.inputCfg.buffer, &mDownmixConfig.outputCfg.buffer);
-            //ALOGV("getNextBuffer is downmixing");
-        }
-        return res;
-    } else {
-        ALOGE("DownmixerBufferProvider::getNextBuffer() error: NULL track buffer provider");
-        return NO_INIT;
-    }
-}
-
-void AudioMixer::DownmixerBufferProvider::releaseBuffer(AudioBufferProvider::Buffer *pBuffer) {
-    //ALOGV("DownmixerBufferProvider::releaseBuffer()");
-    if (mTrackBufferProvider != NULL) {
-        mTrackBufferProvider->releaseBuffer(pBuffer);
-    } else {
-        ALOGE("DownmixerBufferProvider::releaseBuffer() error: NULL track buffer provider");
-    }
-}
 
 template <typename T>
 T min(const T& a, const T& b)
@@ -124,98 +84,161 @@ T min(const T& a, const T& b)
     return a < b ? a : b;
 }
 
-AudioMixer::ReformatBufferProvider::ReformatBufferProvider(int32_t channels,
-        audio_format_t inputFormat, audio_format_t outputFormat) :
-        mTrackBufferProvider(NULL),
-        mChannels(channels),
-        mInputFormat(inputFormat),
-        mOutputFormat(outputFormat),
-        mInputFrameSize(channels * audio_bytes_per_sample(inputFormat)),
-        mOutputFrameSize(channels * audio_bytes_per_sample(outputFormat)),
-        mOutputData(NULL),
-        mOutputCount(0),
+AudioMixer::CopyBufferProvider::CopyBufferProvider(size_t inputFrameSize,
+        size_t outputFrameSize, size_t bufferFrameCount) :
+        mInputFrameSize(inputFrameSize),
+        mOutputFrameSize(outputFrameSize),
+        mLocalBufferFrameCount(bufferFrameCount),
+        mLocalBufferData(NULL),
         mConsumed(0)
 {
-    ALOGV("ReformatBufferProvider(%p)(%d, %#x, %#x)", this, channels, inputFormat, outputFormat);
-    if (requiresInternalBuffers()) {
-        mOutputCount = 256;
-        (void)posix_memalign(&mOutputData, 32, mOutputCount * mOutputFrameSize);
+    ALOGV("CopyBufferProvider(%p)(%zu, %zu, %zu)", this,
+            inputFrameSize, outputFrameSize, bufferFrameCount);
+    LOG_ALWAYS_FATAL_IF(inputFrameSize < outputFrameSize && bufferFrameCount == 0,
+            "Requires local buffer if inputFrameSize(%d) < outputFrameSize(%d)",
+            inputFrameSize, outputFrameSize);
+    if (mLocalBufferFrameCount) {
+        (void)posix_memalign(&mLocalBufferData, 32, mLocalBufferFrameCount * mOutputFrameSize);
     }
     mBuffer.frameCount = 0;
 }
 
-AudioMixer::ReformatBufferProvider::~ReformatBufferProvider()
+AudioMixer::CopyBufferProvider::~CopyBufferProvider()
 {
-    ALOGV("~ReformatBufferProvider(%p)", this);
+    ALOGV("~CopyBufferProvider(%p)", this);
     if (mBuffer.frameCount != 0) {
         mTrackBufferProvider->releaseBuffer(&mBuffer);
     }
-    free(mOutputData);
+    free(mLocalBufferData);
 }
 
-status_t AudioMixer::ReformatBufferProvider::getNextBuffer(AudioBufferProvider::Buffer *pBuffer,
-        int64_t pts) {
-    //ALOGV("ReformatBufferProvider(%p)::getNextBuffer(%p (%zu), %lld)",
+status_t AudioMixer::CopyBufferProvider::getNextBuffer(AudioBufferProvider::Buffer *pBuffer,
+        int64_t pts)
+{
+    //ALOGV("CopyBufferProvider(%p)::getNextBuffer(%p (%zu), %lld)",
     //        this, pBuffer, pBuffer->frameCount, pts);
-    if (!requiresInternalBuffers()) {
+    if (mLocalBufferFrameCount == 0) {
         status_t res = mTrackBufferProvider->getNextBuffer(pBuffer, pts);
         if (res == OK) {
-            memcpy_by_audio_format(pBuffer->raw, mOutputFormat, pBuffer->raw, mInputFormat,
-                    pBuffer->frameCount * mChannels);
+            copyFrames(pBuffer->raw, pBuffer->raw, pBuffer->frameCount);
         }
         return res;
     }
     if (mBuffer.frameCount == 0) {
         mBuffer.frameCount = pBuffer->frameCount;
         status_t res = mTrackBufferProvider->getNextBuffer(&mBuffer, pts);
-        // TODO: Track down a bug in the upstream provider
-        // LOG_ALWAYS_FATAL_IF(res == OK && mBuffer.frameCount == 0,
-        //        "ReformatBufferProvider::getNextBuffer():"
-        //        " Invalid zero framecount returned from getNextBuffer()");
-        if (res != OK || mBuffer.frameCount == 0) {
+        // At one time an upstream buffer provider had
+        // res == OK and mBuffer.frameCount == 0, doesn't seem to happen now 7/18/2014.
+        //
+        // By API spec, if res != OK, then mBuffer.frameCount == 0.
+        // but there may be improper implementations.
+        ALOG_ASSERT(res == OK || mBuffer.frameCount == 0);
+        if (res != OK || mBuffer.frameCount == 0) { // not needed by API spec, but to be safe.
             pBuffer->raw = NULL;
             pBuffer->frameCount = 0;
             return res;
         }
+        mConsumed = 0;
     }
     ALOG_ASSERT(mConsumed < mBuffer.frameCount);
-    size_t count = min(mOutputCount, mBuffer.frameCount - mConsumed);
+    size_t count = min(mLocalBufferFrameCount, mBuffer.frameCount - mConsumed);
     count = min(count, pBuffer->frameCount);
-    pBuffer->raw = mOutputData;
+    pBuffer->raw = mLocalBufferData;
     pBuffer->frameCount = count;
-    //ALOGV("reformatting %d frames from %#x to %#x, %d chan",
-    //        pBuffer->frameCount, mInputFormat, mOutputFormat, mChannels);
-    memcpy_by_audio_format(pBuffer->raw, mOutputFormat,
-            (uint8_t*)mBuffer.raw + mConsumed * mInputFrameSize, mInputFormat,
-            pBuffer->frameCount * mChannels);
+    copyFrames(pBuffer->raw, (uint8_t*)mBuffer.raw + mConsumed * mInputFrameSize,
+            pBuffer->frameCount);
     return OK;
 }
 
-void AudioMixer::ReformatBufferProvider::releaseBuffer(AudioBufferProvider::Buffer *pBuffer) {
-    //ALOGV("ReformatBufferProvider(%p)::releaseBuffer(%p(%zu))",
+void AudioMixer::CopyBufferProvider::releaseBuffer(AudioBufferProvider::Buffer *pBuffer)
+{
+    //ALOGV("CopyBufferProvider(%p)::releaseBuffer(%p(%zu))",
     //        this, pBuffer, pBuffer->frameCount);
-    if (!requiresInternalBuffers()) {
+    if (mLocalBufferFrameCount == 0) {
         mTrackBufferProvider->releaseBuffer(pBuffer);
         return;
     }
     // LOG_ALWAYS_FATAL_IF(pBuffer->frameCount == 0, "Invalid framecount");
     mConsumed += pBuffer->frameCount; // TODO: update for efficiency to reuse existing content
     if (mConsumed != 0 && mConsumed >= mBuffer.frameCount) {
-        mConsumed = 0;
         mTrackBufferProvider->releaseBuffer(&mBuffer);
-        // ALOG_ASSERT(mBuffer.frameCount == 0);
+        ALOG_ASSERT(mBuffer.frameCount == 0);
     }
     pBuffer->raw = NULL;
     pBuffer->frameCount = 0;
 }
 
-void AudioMixer::ReformatBufferProvider::reset() {
+void AudioMixer::CopyBufferProvider::reset()
+{
     if (mBuffer.frameCount != 0) {
         mTrackBufferProvider->releaseBuffer(&mBuffer);
     }
     mConsumed = 0;
 }
 
+AudioMixer::DownmixerBufferProvider::DownmixerBufferProvider() : AudioBufferProvider(),
+        mTrackBufferProvider(NULL), mDownmixHandle(NULL)
+{
+}
+
+AudioMixer::DownmixerBufferProvider::~DownmixerBufferProvider()
+{
+    ALOGV("AudioMixer deleting DownmixerBufferProvider (%p)", this);
+    EffectRelease(mDownmixHandle);
+}
+
+status_t AudioMixer::DownmixerBufferProvider::getNextBuffer(AudioBufferProvider::Buffer *pBuffer,
+        int64_t pts) {
+    //ALOGV("DownmixerBufferProvider::getNextBuffer()");
+    if (mTrackBufferProvider != NULL) {
+        status_t res = mTrackBufferProvider->getNextBuffer(pBuffer, pts);
+        if (res == OK) {
+            mDownmixConfig.inputCfg.buffer.frameCount = pBuffer->frameCount;
+            mDownmixConfig.inputCfg.buffer.raw = pBuffer->raw;
+            mDownmixConfig.outputCfg.buffer.frameCount = pBuffer->frameCount;
+            mDownmixConfig.outputCfg.buffer.raw = mDownmixConfig.inputCfg.buffer.raw;
+            // in-place so overwrite the buffer contents, has been set in prepareTrackForDownmix()
+            //mDownmixConfig.outputCfg.accessMode = EFFECT_BUFFER_ACCESS_WRITE;
+
+            res = (*mDownmixHandle)->process(mDownmixHandle,
+                    &mDownmixConfig.inputCfg.buffer, &mDownmixConfig.outputCfg.buffer);
+            //ALOGV("getNextBuffer is downmixing");
+        }
+        return res;
+    } else {
+        ALOGE("DownmixerBufferProvider::getNextBuffer() error: NULL track buffer provider");
+        return NO_INIT;
+    }
+}
+
+void AudioMixer::DownmixerBufferProvider::releaseBuffer(AudioBufferProvider::Buffer *pBuffer) {
+    //ALOGV("DownmixerBufferProvider::releaseBuffer()");
+    if (mTrackBufferProvider != NULL) {
+        mTrackBufferProvider->releaseBuffer(pBuffer);
+    } else {
+        ALOGE("DownmixerBufferProvider::releaseBuffer() error: NULL track buffer provider");
+    }
+}
+
+AudioMixer::ReformatBufferProvider::ReformatBufferProvider(int32_t channels,
+        audio_format_t inputFormat, audio_format_t outputFormat,
+        size_t bufferFrameCount) :
+        CopyBufferProvider(
+            channels * audio_bytes_per_sample(inputFormat),
+            channels * audio_bytes_per_sample(outputFormat),
+            bufferFrameCount),
+        mChannels(channels),
+        mInputFormat(inputFormat),
+        mOutputFormat(outputFormat)
+{
+    ALOGV("ReformatBufferProvider(%p)(%d, %#x, %#x)", this, channels, inputFormat, outputFormat);
+}
+
+void AudioMixer::ReformatBufferProvider::copyFrames(void *dst, const void *src, size_t frames)
+{
+    memcpy_by_audio_format(dst, mOutputFormat, src, mInputFormat, frames * mChannels);
+}
+
 // ----------------------------------------------------------------------------
 bool AudioMixer::sIsMultichannelCapable = false;
 
@@ -258,6 +281,7 @@ AudioMixer::AudioMixer(size_t frameCount, uint32_t sampleRate, uint32_t maxNumTr
     for (unsigned i=0 ; i < MAX_NUM_TRACKS ; i++) {
         t->resampler = NULL;
         t->downmixerBufferProvider = NULL;
+        t->mReformatBufferProvider = NULL;
         t++;
     }
 
@@ -269,6 +293,7 @@ AudioMixer::~AudioMixer()
     for (unsigned i=0 ; i < MAX_NUM_TRACKS ; i++) {
         delete t->resampler;
         delete t->downmixerBufferProvider;
+        delete t->mReformatBufferProvider;
         t++;
     }
     delete [] mState.outputTemp;
@@ -521,7 +546,8 @@ status_t AudioMixer::prepareTrackForReformat(track_t* pTrack, int trackName)
     if (pTrack->mFormat != pTrack->mMixerInFormat) {
         pTrack->mReformatBufferProvider = new ReformatBufferProvider(
                 audio_channel_count_from_out_mask(pTrack->channelMask),
-                pTrack->mFormat, pTrack->mMixerInFormat);
+                pTrack->mFormat, pTrack->mMixerInFormat,
+                kCopyBufferFrameCount);
         reconfigureBufferProviders(pTrack);
     }
     return NO_ERROR;
@@ -531,7 +557,7 @@ void AudioMixer::reconfigureBufferProviders(track_t* pTrack)
 {
     pTrack->bufferProvider = pTrack->mInputBufferProvider;
     if (pTrack->mReformatBufferProvider) {
-        pTrack->mReformatBufferProvider->mTrackBufferProvider = pTrack->bufferProvider;
+        pTrack->mReformatBufferProvider->setBufferProvider(pTrack->bufferProvider);
         pTrack->bufferProvider = pTrack->mReformatBufferProvider;
     }
     if (pTrack->downmixerBufferProvider) {
diff --git a/services/audioflinger/AudioMixer.h b/services/audioflinger/AudioMixer.h
index a9f4761..f08d9b5 100644
--- a/services/audioflinger/AudioMixer.h
+++ b/services/audioflinger/AudioMixer.h
@@ -154,7 +154,7 @@ private:
     struct state_t;
     struct track_t;
     class DownmixerBufferProvider;
-    class ReformatBufferProvider;
+    class CopyBufferProvider;
 
     typedef void (*hook_t)(track_t* t, int32_t* output, size_t numOutFrames, int32_t* temp,
                            int32_t* aux);
@@ -206,8 +206,8 @@ private:
         int32_t*           auxBuffer;
 
         // 16-byte boundary
-        AudioBufferProvider*     mInputBufferProvider;    // 4 bytes
-        ReformatBufferProvider*  mReformatBufferProvider; // 4 bytes
+        AudioBufferProvider*     mInputBufferProvider;    // externally provided buffer provider.
+        CopyBufferProvider*      mReformatBufferProvider; // provider wrapper for reformatting.
         DownmixerBufferProvider* downmixerBufferProvider; // 4 bytes
 
         int32_t     sessionId;
@@ -253,6 +253,52 @@ private:
         track_t         tracks[MAX_NUM_TRACKS] __attribute__((aligned(32)));
     };
 
+    // Base AudioBufferProvider class used for ReformatBufferProvider.
+    // It handles a private buffer for use in converting format or channel masks from the
+    // input data to a form acceptable by the mixer.
+    // TODO: Make a ResamplerBufferProvider when integers are entirely removed from the
+    // processing pipeline.
+    class CopyBufferProvider : public AudioBufferProvider {
+    public:
+        // Use a private buffer of bufferFrameCount frames (each frame is outputFrameSize bytes).
+        // If bufferFrameCount is 0, no private buffer is created and in-place modification of
+        // the upstream buffer provider's buffers is performed by copyFrames().
+        CopyBufferProvider(size_t inputFrameSize, size_t outputFrameSize,
+                size_t bufferFrameCount);
+        virtual ~CopyBufferProvider();
+
+        // Overrides AudioBufferProvider methods
+        virtual status_t getNextBuffer(Buffer* buffer, int64_t pts);
+        virtual void releaseBuffer(Buffer* buffer);
+
+        // Other public methods
+
+        // call this to release the buffer to the upstream provider.
+        // treat it as an audio discontinuity for future samples.
+        virtual void reset();
+
+        // this function should be supplied by the derived class.  It converts
+        // #frames in the *src pointer to the *dst pointer.  It is public because
+        // some providers will allow this to work on arbitrary buffers outside
+        // of the internal buffers.
+        virtual void copyFrames(void *dst, const void *src, size_t frames) = 0;
+
+        // set the upstream buffer provider. Consider calling "reset" before this function.
+        void setBufferProvider(AudioBufferProvider *p) {
+            mTrackBufferProvider = p;
+        }
+
+    protected:
+        AudioBufferProvider* mTrackBufferProvider;
+        const size_t         mInputFrameSize;
+        const size_t         mOutputFrameSize;
+    private:
+        AudioBufferProvider::Buffer mBuffer;
+        const size_t         mLocalBufferFrameCount;
+        void*                mLocalBufferData;
+        size_t               mConsumed;
+    };
+
     // AudioBufferProvider that wraps a track AudioBufferProvider by a call to a downmix effect
     class DownmixerBufferProvider : public AudioBufferProvider {
     public:
@@ -266,33 +312,19 @@ private:
         effect_config_t    mDownmixConfig;
     };
 
-    // AudioBufferProvider wrapper that reformats track to acceptable mixer input type
-    class ReformatBufferProvider : public AudioBufferProvider {
+    // ReformatBufferProvider wraps a track AudioBufferProvider to convert the input data
+    // to an acceptable mixer input format type.
+    class ReformatBufferProvider : public CopyBufferProvider {
     public:
         ReformatBufferProvider(int32_t channels,
-                audio_format_t inputFormat, audio_format_t outputFormat);
-        virtual ~ReformatBufferProvider();
-
-        // overrides AudioBufferProvider methods
-        virtual status_t getNextBuffer(Buffer* buffer, int64_t pts);
-        virtual void releaseBuffer(Buffer* buffer);
-
-        void reset();
-        inline bool requiresInternalBuffers() {
-            return true; //mInputFrameSize < mOutputFrameSize;
-        }
-
-        AudioBufferProvider* mTrackBufferProvider;
-        int32_t              mChannels;
-        audio_format_t       mInputFormat;
-        audio_format_t       mOutputFormat;
-        size_t               mInputFrameSize;
-        size_t               mOutputFrameSize;
-        // (only) required for reformatting to a larger size.
-        AudioBufferProvider::Buffer mBuffer;
-        void*                mOutputData;
-        size_t               mOutputCount;
-        size_t               mConsumed;
+                audio_format_t inputFormat, audio_format_t outputFormat,
+                size_t bufferFrameCount);
+        virtual void copyFrames(void *dst, const void *src, size_t frames);
+
+    protected:
+        const int32_t        mChannels;
+        const audio_format_t mInputFormat;
+        const audio_format_t mOutputFormat;
     };
 
     // bitmask of allocated track names, where bit 0 corresponds to TRACK0 etc.