From 29b703eec27b305e7b5b2343bf257643e38f6b68 Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Mon, 12 May 2014 11:06:26 -0700 Subject: [PATCH] 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 | 42 ++++++++++++++++++++++----------------- services/audioflinger/Threads.cpp | 1 + 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp index 1c808d0ae5..db61e85de5 100644 --- a/media/libmedia/AudioRecord.cpp +++ b/media/libmedia/AudioRecord.cpp @@ -203,23 +203,6 @@ status_t AudioRecord::set( 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 @@ status_t AudioRecord::set( } // 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 @@ status_t AudioRecord::openRecord_l(size_t epoch) 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; diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp index cdc9a19c22..b6782a930d 100644 --- a/services/audioflinger/Threads.cpp +++ b/services/audioflinger/Threads.cpp @@ -5128,6 +5128,7 @@ sp AudioFlinger::RecordThread::createRe // 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); -- 2.11.0