commit 29b703eec27b305e7b5b2343bf257643e38f6b68
author Glenn Kasten <gkasten@google.com> Mon May 12 11:06:26 2014 -0700
committer Glenn Kasten <gkasten@google.com> Thu May 22 16:19:05 2014 -0700
tree 0612b8e650dac1f4f7c2e51afa187b4601f113ca
parent 6181ffd90a436f333c43a7f812391eee2c35005a

Move validation of frameCount from set to openRecord_l

This move is needed because frameCount is validated on server side for
fast tracks (as should be done for normal tracks too).

Change-Id: I6d99e80869fd90fab373cf60ef348c01f075fbca

media/libmedia/AudioRecord.cpp

diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp
index 1c808d0..db61e85 100644
--- a/media/libmedia/AudioRecord.cpp
+++ b/media/libmedia/AudioRecord.cpp
@@ -203,23 +203,6 @@
         mFrameSize = sizeof(uint8_t);
     }
 
-    // validate framecount
-    size_t minFrameCount;
-    status_t status = AudioRecord::getMinFrameCount(&minFrameCount,
-            sampleRate, format, channelMask);
-    if (status != NO_ERROR) {
-        ALOGE("getMinFrameCount() failed for sampleRate %u, format %#x, channelMask %#x; status %d",
-                sampleRate, format, channelMask, status);
-        return status;
-    }
-    ALOGV("AudioRecord::set() minFrameCount = %d", minFrameCount);
-
-    if (frameCount == 0) {
-        frameCount = minFrameCount;
-    } else if (frameCount < minFrameCount) {
-        ALOGE("frameCount %u < minFrameCount %u", frameCount, minFrameCount);
-        return BAD_VALUE;
-    }
     // mFrameCount is initialized in openRecord_l
     mReqFrameCount = frameCount;
 
@@ -242,7 +225,7 @@
     }
 
     // create the IAudioRecord
-    status = openRecord_l(0 /*epoch*/);
+    status_t status = openRecord_l(0 /*epoch*/);
 
     if (status != NO_ERROR) {
         if (mAudioRecordThread != 0) {
@@ -464,6 +447,29 @@
     size_t frameCount = mReqFrameCount;
 
     if (!(mFlags & AUDIO_INPUT_FLAG_FAST)) {
+        // validate framecount
+        // If fast track was not requested, this preserves
+        // the old behavior of validating on client side.
+        // FIXME Eventually the validation should be done on server side
+        // regardless of whether it's a fast or normal track.  It's debatable
+        // whether to account for the input latency to provision buffers appropriately.
+        size_t minFrameCount;
+        status = AudioRecord::getMinFrameCount(&minFrameCount,
+                mSampleRate, mFormat, mChannelMask);
+        if (status != NO_ERROR) {
+            ALOGE("getMinFrameCount() failed for sampleRate %u, format %#x, channelMask %#x; "
+                    "status %d",
+                    mSampleRate, mFormat, mChannelMask, status);
+            return status;
+        }
+
+        if (frameCount == 0) {
+            frameCount = minFrameCount;
+        } else if (frameCount < minFrameCount) {
+            ALOGE("frameCount %u < minFrameCount %u", frameCount, minFrameCount);
+            return BAD_VALUE;
+        }
+
         // Make sure that application is notified with sufficient margin before overrun
         if (mNotificationFramesAct == 0 || mNotificationFramesAct > frameCount/2) {
             mNotificationFramesAct = frameCount/2;

services/audioflinger/Threads.cpp

diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index cdc9a19..b6782a9 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -5128,6 +5128,7 @@
         // to be at least 2 x the record thread frame count and cover audio hardware latency.
         // This is probably too conservative, but legacy application code may depend on it.
         // If you change this calculation, also review the start threshold which is related.
+        // FIXME It's not clear how input latency actually matters.  Perhaps this should be 0.
         uint32_t latencyMs = 50; // FIXME mInput->stream->get_latency(mInput->stream);
         size_t mNormalFrameCount = 2048; // FIXME
         uint32_t minBufCount = latencyMs / ((1000 * mNormalFrameCount) / mSampleRate);