From 66a0467fdddada4caabd0f0a999fbb367fea7bee Mon Sep 17 00:00:00 2001
From: Glenn Kasten <gkasten@google.com>
Date: Wed, 8 Jan 2014 08:53:44 -0800
Subject: [PATCH] Cleanup AudioTrack::getMinFrameCount error handling

Guarantee to return a non-zero frameCount for return status NO_ERROR;

Return the correct specific status_t if any of the AudioSystem APIs fail,
instead of the generic NO_INIT.

API change: getMinFramCount no longer defaults to zero on error, so
callers _must_ check the return status.  This change makes
getMinFrameCount more like other APIs.  All known callers were reviewed,
and they do check the return status.

Change-Id: I4a8342a75ee89a068c23c84b8380ed9d1b968507
---
 include/media/AudioTrack.h    |  2 ++
 media/libmedia/AudioTrack.cpp | 26 +++++++++++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index 9f631cf055..8afcc5d887 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -123,6 +123,8 @@ public:
      *  - NO_ERROR: successful operation
      *  - NO_INIT: audio server or audio hardware not initialized
      *  - BAD_VALUE: unsupported configuration
+     * frameCount is guaranteed to be non-zero if status is NO_ERROR,
+     * and is undefined otherwise.
      */
 
     static status_t getMinFrameCount(size_t* frameCount,
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index ddd461252c..a6ffc62e0a 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -44,9 +44,6 @@ status_t AudioTrack::getMinFrameCount(
         return BAD_VALUE;
     }
 
-    // default to 0 in case of error
-    *frameCount = 0;
-
     // FIXME merge with similar code in createTrack_l(), except we're missing
     //       some information here that is available in createTrack_l():
     //          audio_io_handle_t output
@@ -54,16 +51,20 @@ status_t AudioTrack::getMinFrameCount(
     //          audio_channel_mask_t channelMask
     //          audio_output_flags_t flags
     uint32_t afSampleRate;
-    if (AudioSystem::getOutputSamplingRate(&afSampleRate, streamType) != NO_ERROR) {
-        return NO_INIT;
+    status_t status;
+    status = AudioSystem::getOutputSamplingRate(&afSampleRate, streamType);
+    if (status != NO_ERROR) {
+        return status;
     }
     size_t afFrameCount;
-    if (AudioSystem::getOutputFrameCount(&afFrameCount, streamType) != NO_ERROR) {
-        return NO_INIT;
+    status = AudioSystem::getOutputFrameCount(&afFrameCount, streamType);
+    if (status != NO_ERROR) {
+        return status;
     }
     uint32_t afLatency;
-    if (AudioSystem::getOutputLatency(&afLatency, streamType) != NO_ERROR) {
-        return NO_INIT;
+    status = AudioSystem::getOutputLatency(&afLatency, streamType);
+    if (status != NO_ERROR) {
+        return status;
     }
 
     // Ensure that buffer depth covers at least audio hardware latency
@@ -74,6 +75,13 @@ status_t AudioTrack::getMinFrameCount(
 
     *frameCount = (sampleRate == 0) ? afFrameCount * minBufCount :
             afFrameCount * minBufCount * sampleRate / afSampleRate;
+    // The formula above should always produce a non-zero value, but return an error
+    // in the unlikely event that it does not, as that's part of the API contract.
+    if (*frameCount == 0) {
+        ALOGE("AudioTrack::getMinFrameCount failed for streamType %d, sampleRate %d",
+                streamType, sampleRate);
+        return BAD_VALUE;
+    }
     ALOGV("getMinFrameCount=%d: afFrameCount=%d, minBufCount=%d, afSampleRate=%d, afLatency=%d",
             *frameCount, afFrameCount, minBufCount, afSampleRate, afLatency);
     return NO_ERROR;
-- 
2.11.0