Clean up frame size in AudioTrack and AudioFlinger

TrackBase::mFrameSize, mChannelMask, and mChannelCount are now const.
Use TrackBase::mFrameSize instead of re-calculating frame size.
AudioFlinger only sees 16-bit PCM format, conversion from 8-bit is
  now entirely on the client side.  Previously a small part of the
  responsibility was on server side also.
size_t is unsigned, so use %u in logs.
Fix theoretical bug where TrackBase constructor was over-allocating space
  for non-linear AudioTrack or 8-bit PCM AudioRecord (probably benign).

Change-Id: I7cbbba0bf4dba29ea751d8af341ab8e5cbbdc206

4 files changed, 26 insertions, 31 deletions

diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index 99d583d..6fd1b9e 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -520,7 +520,7 @@ protected:
             // address space.  AudioFlinger::TrackBase::mBuffer is for the server address space.
     void*                   mBuffers;
 
-    audio_format_t          mFormat;
+    audio_format_t          mFormat;                // as requested by client, not forced to 16-bit
     audio_stream_type_t     mStreamType;
     uint8_t                 mChannelCount;
     uint8_t                 mMuted;
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 7480807..5fb36ee 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -858,7 +858,9 @@ status_t AudioTrack::createTrack_l(
     sp<IAudioTrack> track = audioFlinger->createTrack(getpid(),
                                                       streamType,
                                                       sampleRate,
-                                                      format,
+                                                      // AudioFlinger only sees 16-bit PCM
+                                                      format == AUDIO_FORMAT_PCM_8_BIT ?
+                                                              AUDIO_FORMAT_PCM_16_BIT : format,
                                                       channelMask,
                                                       frameCount,
                                                       &trackFlags,
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 6406b6c..10f4410 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -472,6 +472,14 @@ sp<IAudioTrack> AudioFlinger::createTrack(
         goto Exit;
     }
 
+    // client is responsible for conversion of 8-bit PCM to 16-bit PCM,
+    // and we don't yet support 8.24 or 32-bit PCM
+    if (audio_is_linear_pcm(format) && format != AUDIO_FORMAT_PCM_16_BIT) {
+        ALOGE("createTrack() invalid format %d", format);
+        lStatus = BAD_VALUE;
+        goto Exit;
+    }
+
     {
         Mutex::Autolock _l(mLock);
         PlaybackThread *thread = checkPlaybackThread_l(output);
@@ -2280,7 +2288,7 @@ AudioFlinger::MixerThread::MixerThread(const sp<AudioFlinger>& audioFlinger, Aud
         // mNormalSink below
 {
     ALOGV("MixerThread() id=%d device=%#x type=%d", id, device, type);
-    ALOGV("mSampleRate=%u, mChannelMask=%#x, mChannelCount=%d, mFormat=%d, mFrameSize=%d, "
+    ALOGV("mSampleRate=%u, mChannelMask=%#x, mChannelCount=%d, mFormat=%d, mFrameSize=%u, "
             "mFrameCount=%d, mNormalFrameCount=%d",
             mSampleRate, mChannelMask, mChannelCount, mFormat, mFrameSize, mFrameCount,
             mNormalFrameCount);
@@ -4190,11 +4198,12 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
         mState(IDLE),
         mSampleRate(sampleRate),
         mFormat(format),
-        mFrameSize(0),  // will be set to correct value in constructor
+        mChannelMask(channelMask),
+        mChannelCount(popcount(channelMask)),
+        mFrameSize(audio_is_linear_pcm(format) ?
+                mChannelCount * audio_bytes_per_sample(format) : sizeof(int8_t)),
         mStepServerFailed(false),
         mSessionId(sessionId)
-        // mChannelCount
-        // mChannelMask
 {
     // client == 0 implies sharedBuffer == 0
     ALOG_ASSERT(!(client == 0 && sharedBuffer != 0));
@@ -4204,8 +4213,7 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
 
     // ALOGD("Creating track with %d buffers @ %d bytes", bufferCount, bufferSize);
     size_t size = sizeof(audio_track_cblk_t);
-    uint8_t channelCount = popcount(channelMask);
-    size_t bufferSize = frameCount*channelCount*sizeof(int16_t);
+    size_t bufferSize = frameCount * mFrameSize;
     if (sharedBuffer == 0) {
         size += bufferSize;
     }
@@ -4236,11 +4244,9 @@ AudioFlinger::ThreadBase::TrackBase::TrackBase(
 //      mCblk->server = 0xffff0000;
 //      mCblk->userBase = 0xffff0000;
 //      mCblk->serverBase = 0xffff0000;
-        mChannelCount = channelCount;
-        mChannelMask = channelMask;
         if (sharedBuffer == 0) {
             mBuffer = (char*)mCblk + sizeof(audio_track_cblk_t);
-            memset(mBuffer, 0, frameCount*channelCount*sizeof(int16_t));
+            memset(mBuffer, 0, bufferSize);
             // Force underrun condition to avoid false underrun callback until first data is
             // written to buffer (other flags are cleared)
             mCblk->flags = CBLK_UNDERRUN;
@@ -4312,17 +4318,16 @@ uint32_t AudioFlinger::ThreadBase::TrackBase::sampleRate() const {
 
 void* AudioFlinger::ThreadBase::TrackBase::getBuffer(uint32_t offset, uint32_t frames) const {
     audio_track_cblk_t* cblk = this->cblk();
-    size_t frameSize = mFrameSize;
-    int8_t *bufferStart = (int8_t *)mBuffer + (offset-cblk->serverBase)*frameSize;
-    int8_t *bufferEnd = bufferStart + frames * frameSize;
+    int8_t *bufferStart = (int8_t *)mBuffer + (offset-cblk->serverBase) * mFrameSize;
+    int8_t *bufferEnd = bufferStart + frames * mFrameSize;
 
     // Check validity of returned pointer in case the track control block would have been corrupted.
     ALOG_ASSERT(!(bufferStart < mBuffer || bufferStart > bufferEnd || bufferEnd > mBufferEnd),
             "TrackBase::getBuffer buffer out of range:\n"
                 "    start: %p, end %p , mBuffer %p mBufferEnd %p\n"
-                "    server %u, serverBase %u, user %u, userBase %u, frameSize %d",
+                "    server %u, serverBase %u, user %u, userBase %u, frameSize %u",
                 bufferStart, bufferEnd, mBuffer, mBufferEnd,
-                cblk->server, cblk->serverBase, cblk->user, cblk->userBase, frameSize);
+                cblk->server, cblk->serverBase, cblk->user, cblk->userBase, mFrameSize);
 
     return bufferStart;
 }
@@ -4364,10 +4369,6 @@ AudioFlinger::PlaybackThread::Track::Track(
     mUnderrunCount(0),
     mCachedVolume(1.0)
 {
-    // NOTE: frame size for 8 bit PCM data is based on a sample size of
-    // 16 bit because data is converted to 16 bit before being stored in buffer by AudioTrack
-    mFrameSize = audio_is_linear_pcm(format) ? mChannelCount * sizeof(int16_t) :
-            sizeof(uint8_t);
     if (mCblk != NULL) {
         // to avoid leaking a track name, do not allocate one unless there is an mCblk
         mName = thread->getTrackName_l(channelMask, sessionId);
@@ -5411,13 +5412,6 @@ AudioFlinger::RecordThread::RecordTrack::RecordTrack(
         mOverflow(false)
 {
     ALOGV("RecordTrack constructor, size %d", (int)mBufferEnd - (int)mBuffer);
-    if (format == AUDIO_FORMAT_PCM_16_BIT) {
-        mFrameSize = mChannelCount * sizeof(int16_t);
-    } else if (format == AUDIO_FORMAT_PCM_8_BIT) {
-        mFrameSize = mChannelCount * sizeof(int8_t);
-    } else {
-        mFrameSize = sizeof(int8_t);
-    }
 }
 
 AudioFlinger::RecordThread::RecordTrack::~RecordTrack()
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 8816929..9ddfe28 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -473,14 +473,13 @@ private:
             const uint32_t      mSampleRate;    // initial sample rate only; for tracks which
                                 // support dynamic rates, the current value is in control block
             const audio_format_t mFormat;
-            size_t              mFrameSize; // AudioFlinger's view of frame size in shared memory,
+            const audio_channel_mask_t mChannelMask;
+            const uint8_t       mChannelCount;
+            const size_t        mFrameSize; // AudioFlinger's view of frame size in shared memory,
                                             // where for AudioTrack (but not AudioRecord),
                                             // 8-bit PCM samples are stored as 16-bit
-                                            // FIXME should be const
             bool                mStepServerFailed;
             const int           mSessionId;
-            uint8_t             mChannelCount;
-            audio_channel_mask_t mChannelMask;
             Vector < sp<SyncEvent> >mSyncEvents;
         };