Using centralized isAudioPlaybackRateValid to validate parameters

Centralized validation code
bug: 20701446

Change-Id: I9d9941c7639c05b2afe069ff4f858c693c910bfe

5 files changed, 46 insertions, 22 deletions

diff --git a/include/media/AudioResamplerPublic.h b/include/media/AudioResamplerPublic.h
index 6cf2ca9..055f724 100644
--- a/include/media/AudioResamplerPublic.h
+++ b/include/media/AudioResamplerPublic.h
@@ -108,6 +108,23 @@ static inline bool isAudioPlaybackRateEqual(const AudioPlaybackRate &pr1,
            pr2.mFallbackMode == pr2.mFallbackMode;
 }
 
+static inline bool isAudioPlaybackRateValid(const AudioPlaybackRate &playbackRate) {
+    if (playbackRate.mFallbackMode == AUDIO_TIMESTRETCH_FALLBACK_FAIL &&
+            (playbackRate.mStretchMode == AUDIO_TIMESTRETCH_STRETCH_SPEECH ||
+                    playbackRate.mStretchMode == AUDIO_TIMESTRETCH_STRETCH_DEFAULT)) {
+        //test sonic specific constraints
+        return playbackRate.mSpeed >= TIMESTRETCH_SONIC_SPEED_MIN &&
+                playbackRate.mSpeed <= TIMESTRETCH_SONIC_SPEED_MAX &&
+                playbackRate.mPitch >= AUDIO_TIMESTRETCH_PITCH_MIN &&
+                playbackRate.mPitch <= AUDIO_TIMESTRETCH_PITCH_MAX;
+    } else {
+        return playbackRate.mSpeed >= AUDIO_TIMESTRETCH_SPEED_MIN &&
+                playbackRate.mSpeed <= AUDIO_TIMESTRETCH_SPEED_MAX &&
+                playbackRate.mPitch >= AUDIO_TIMESTRETCH_PITCH_MIN &&
+                playbackRate.mPitch <= AUDIO_TIMESTRETCH_PITCH_MAX;
+    }
+}
+
 // TODO: Consider putting these inlines into a class scope
 
 // Returns the source frames needed to resample to destination frames.  This is not a precise
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 81ae6d7..2649ade 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -78,7 +78,7 @@ static inline uint32_t adjustSampleRate(uint32_t sampleRate, float pitch)
 
 static inline float adjustSpeed(float speed, float pitch)
 {
-    return kFixPitch ? (speed / pitch) : speed;
+    return kFixPitch ? speed / max(pitch, AUDIO_TIMESTRETCH_PITCH_MIN_DELTA) : speed;
 }
 
 static inline float adjustPitch(float pitch)
@@ -809,25 +809,33 @@ status_t AudioTrack::setPlaybackRate(const AudioPlaybackRate &playbackRate)
     const uint32_t effectiveRate = adjustSampleRate(mSampleRate, playbackRate.mPitch);
     const float effectiveSpeed = adjustSpeed(playbackRate.mSpeed, playbackRate.mPitch);
     const float effectivePitch = adjustPitch(playbackRate.mPitch);
-    if (effectiveSpeed < AUDIO_TIMESTRETCH_SPEED_MIN
-            || effectiveSpeed > AUDIO_TIMESTRETCH_SPEED_MAX
-            || effectivePitch < AUDIO_TIMESTRETCH_PITCH_MIN
-            || effectivePitch > AUDIO_TIMESTRETCH_PITCH_MAX) {
+    AudioPlaybackRate playbackRateTemp = playbackRate;
+    playbackRateTemp.mSpeed = effectiveSpeed;
+    playbackRateTemp.mPitch = effectivePitch;
+
+    if (!isAudioPlaybackRateValid(playbackRateTemp)) {
         return BAD_VALUE;
-        //TODO: add function in AudioResamplerPublic.h to check for validity.
     }
     // Check if the buffer size is compatible.
     if (!isSampleRateSpeedAllowed_l(effectiveRate, effectiveSpeed)) {
         ALOGV("setPlaybackRate(%f, %f) failed", playbackRate.mSpeed, playbackRate.mPitch);
         return BAD_VALUE;
     }
-    mPlaybackRate = playbackRate;
-    mProxy->setPlaybackRate(playbackRate);
 
-    //modify this
-    AudioPlaybackRate playbackRateTemp = playbackRate;
-    playbackRateTemp.mSpeed = effectiveSpeed;
-    playbackRateTemp.mPitch = effectivePitch;
+    // Check resampler ratios are within bounds
+    if (effectiveRate > mSampleRate * AUDIO_RESAMPLER_DOWN_RATIO_MAX) {
+        ALOGV("setPlaybackRate(%f, %f) failed. Resample rate exceeds max accepted value",
+                playbackRate.mSpeed, playbackRate.mPitch);
+        return BAD_VALUE;
+    }
+
+    if (effectiveRate * AUDIO_RESAMPLER_UP_RATIO_MAX < mSampleRate) {
+        ALOGV("setPlaybackRate(%f, %f) failed. Resample rate below min accepted value",
+                        playbackRate.mSpeed, playbackRate.mPitch);
+        return BAD_VALUE;
+    }
+    mPlaybackRate = playbackRate;
+    //set effective rates
     mProxy->setPlaybackRate(playbackRateTemp);
     mProxy->setSampleRate(effectiveRate); // FIXME: not quite "atomic" with setPlaybackRate
     return NO_ERROR;
diff --git a/services/audioflinger/AudioMixer.cpp b/services/audioflinger/AudioMixer.cpp
index 01efc53..8a9a837 100644
--- a/services/audioflinger/AudioMixer.cpp
+++ b/services/audioflinger/AudioMixer.cpp
@@ -733,13 +733,9 @@ void AudioMixer::setParameter(int name, int target, int param, void *value)
             case PLAYBACK_RATE: {
                 const AudioPlaybackRate *playbackRate =
                         reinterpret_cast<AudioPlaybackRate*>(value);
-                ALOG_ASSERT(AUDIO_TIMESTRETCH_SPEED_MIN <= playbackRate->mSpeed
-                        && playbackRate->mSpeed <= AUDIO_TIMESTRETCH_SPEED_MAX,
-                        "bad speed %f", playbackRate->mSpeed);
-                ALOG_ASSERT(AUDIO_TIMESTRETCH_PITCH_MIN <= playbackRate->mPitch
-                        && playbackRate->mPitch <= AUDIO_TIMESTRETCH_PITCH_MAX,
-                        "bad pitch %f", playbackRate->mPitch);
-                //TODO: use function from AudioResamplerPublic.h to test validity.
+                ALOGW_IF(!isAudioPlaybackRateValid(*playbackRate),
+                        "bad parameters speed %f, pitch %f",playbackRate->mSpeed,
+                        playbackRate->mPitch);
                 if (track.setPlaybackRate(*playbackRate)) {
                     ALOGV("setParameter(TIMESTRETCH, PLAYBACK_RATE, STRETCH_MODE, FALLBACK_MODE "
                             "%f %f %d %d",
diff --git a/services/audioflinger/BufferProviders.cpp b/services/audioflinger/BufferProviders.cpp
index 8a580e8..3566ee2 100644
--- a/services/audioflinger/BufferProviders.cpp
+++ b/services/audioflinger/BufferProviders.cpp
@@ -332,7 +332,8 @@ TimestretchBufferProvider::TimestretchBufferProvider(int32_t channelCount,
         mLocalBufferData(NULL),
         mRemaining(0),
         mSonicStream(sonicCreateStream(sampleRate, mChannelCount)),
-        mFallbackFailErrorShown(false)
+        mFallbackFailErrorShown(false),
+        mAudioPlaybackRateValid(false)
 {
     LOG_ALWAYS_FATAL_IF(mSonicStream == NULL,
             "TimestretchBufferProvider can't allocate Sonic stream");
@@ -460,6 +461,8 @@ status_t TimestretchBufferProvider::setPlaybackRate(const AudioPlaybackRate &pla
     sonicSetSpeed(mSonicStream, mPlaybackRate.mSpeed);
     //TODO: pitch is ignored for now
     //TODO: optimize: if parameters are the same, don't do any extra computation.
+
+    mAudioPlaybackRateValid = isAudioPlaybackRateValid(mPlaybackRate);
     return OK;
 }
 
@@ -479,8 +482,7 @@ void TimestretchBufferProvider::processFrames(void *dstBuffer, size_t *dstFrames
         *srcFrames = targetSrc + 1;
     }
 
-    if (mPlaybackRate.mSpeed< TIMESTRETCH_SONIC_SPEED_MIN  ||
-            mPlaybackRate.mSpeed >  TIMESTRETCH_SONIC_SPEED_MAX ) {
+    if (!mAudioPlaybackRateValid) {
         //fallback mode
         if (*dstFrames > 0) {
             switch(mPlaybackRate.mFallbackMode) {
diff --git a/services/audioflinger/BufferProviders.h b/services/audioflinger/BufferProviders.h
index 4970b6c..4bc895c 100644
--- a/services/audioflinger/BufferProviders.h
+++ b/services/audioflinger/BufferProviders.h
@@ -188,6 +188,7 @@ private:
     sonicStream          mSonicStream;            // handle to sonic timestretch object
     //FIXME: this dependency should be abstracted out
     bool                 mFallbackFailErrorShown; // log fallback error only once
+    bool                 mAudioPlaybackRateValid; // flag for current parameters validity
 };
 
 // ----------------------------------------------------------------------------