Merge "Visualizer: validate capture size" into main am: 27666e3dd8

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/2982839

Change-Id: I1a65dfdd4fec3bc29fbf218b624d2949ed59eaa7
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 36 insertions, 19 deletions

diff --git a/media/java/android/media/audiofx/Visualizer.java b/media/java/android/media/audiofx/Visualizer.java
index 2795cfe4ba61..f05ea9c2684f 100644
--- a/media/java/android/media/audiofx/Visualizer.java
+++ b/media/java/android/media/audiofx/Visualizer.java
@@ -336,8 +336,9 @@ public class Visualizer {
      * This method must not be called when the Visualizer is enabled.
      * @param size requested capture size
      * @return {@link #SUCCESS} in case of success,
-     * {@link #ERROR_BAD_VALUE} in case of failure.
-     * @throws IllegalStateException
+     * {@link #ERROR_INVALID_OPERATION} if Visualizer effect enginer not enabled.
+     * @throws IllegalStateException if the effect is not in proper state.
+     * @throws IllegalArgumentException if the size parameter is invalid (out of supported range).
      */
     public int setCaptureSize(int size)
     throws IllegalStateException {
@@ -345,7 +346,13 @@ public class Visualizer {
             if (mState != STATE_INITIALIZED) {
                 throw(new IllegalStateException("setCaptureSize() called in wrong state: "+mState));
             }
-            return native_setCaptureSize(size);
+
+            int ret = native_setCaptureSize(size);
+            if (ret == ERROR_BAD_VALUE) {
+                throw(new IllegalArgumentException("setCaptureSize to " + size + " failed"));
+            }
+
+            return ret;
         }
     }
 
diff --git a/media/jni/audioeffect/Visualizer.cpp b/media/jni/audioeffect/Visualizer.cpp
index 09c45ea97e9d..9ae5c991514a 100644
--- a/media/jni/audioeffect/Visualizer.cpp
+++ b/media/jni/audioeffect/Visualizer.cpp
@@ -25,7 +25,6 @@
 #include <limits.h>
 
 #include <audio_utils/fixedfft.h>
-#include <cutils/bitops.h>
 #include <utils/Thread.h>
 
 #include <android/content/AttributionSourceState.h>
@@ -59,8 +58,8 @@ status_t Visualizer::set(int32_t priority,
     status_t status = AudioEffect::set(
             SL_IID_VISUALIZATION, nullptr, priority, cbf, user, sessionId, io, device, probe);
     if (status == NO_ERROR || status == ALREADY_EXISTS) {
-        initCaptureSize();
-        initSampleRate();
+        status = initCaptureSize();
+        if (status == NO_ERROR) initSampleRate();
     }
     return status;
 }
@@ -152,9 +151,8 @@ status_t Visualizer::setCaptureCallBack(capture_cbk_t cbk, void* user, uint32_t
 
 status_t Visualizer::setCaptureSize(uint32_t size)
 {
-    if (size > VISUALIZER_CAPTURE_SIZE_MAX ||
-        size < VISUALIZER_CAPTURE_SIZE_MIN ||
-        popcount(size) != 1) {
+    if (!isCaptureSizeValid(size)) {
+        ALOGE("%s with invalid capture size %u from HAL", __func__, size);
         return BAD_VALUE;
     }
 
@@ -172,7 +170,7 @@ status_t Visualizer::setCaptureSize(uint32_t size)
     *((int32_t *)p->data + 1)= size;
     status_t status = setParameter(p);
 
-    ALOGV("setCaptureSize size %d  status %d p->status %d", size, status, p->status);
+    ALOGV("setCaptureSize size %u status %d p->status %d", size, status, p->status);
 
     if (status == NO_ERROR) {
         status = p->status;
@@ -257,8 +255,8 @@ status_t Visualizer::getIntMeasurements(uint32_t type, uint32_t number, int32_t
     if ((type != MEASUREMENT_MODE_PEAK_RMS)
             // for peak+RMS measurement, the results are 2 int32_t values
             || (number != 2)) {
-        ALOGE("Cannot retrieve int measurements, MEASUREMENT_MODE_PEAK_RMS returns 2 ints, not %d",
-                        number);
+        ALOGE("Cannot retrieve int measurements, MEASUREMENT_MODE_PEAK_RMS returns 2 ints, not %u",
+              number);
         return BAD_VALUE;
     }
 
@@ -390,7 +388,7 @@ void Visualizer::periodicCapture()
     }
 }
 
-uint32_t Visualizer::initCaptureSize()
+status_t Visualizer::initCaptureSize()
 {
     uint32_t buf32[sizeof(effect_param_t) / sizeof(uint32_t) + 2];
     effect_param_t *p = (effect_param_t *)buf32;
@@ -405,14 +403,20 @@ uint32_t Visualizer::initCaptureSize()
     }
 
     uint32_t size = 0;
-    if (status == NO_ERROR) {
-        size = *((int32_t *)p->data + 1);
+    if (status != NO_ERROR) {
+        ALOGE("%s getParameter failed status %d", __func__, status);
+        return status;
     }
-    mCaptureSize = size;
 
-    ALOGV("initCaptureSize size %d status %d", mCaptureSize, status);
+    size = *((int32_t *)p->data + 1);
+    if (!isCaptureSizeValid(size)) {
+        ALOGE("%s with invalid capture size %u from HAL", __func__, size);
+        return BAD_VALUE;
+    }
 
-    return size;
+    mCaptureSize = size;
+    ALOGV("%s size %u status %d", __func__, mCaptureSize, status);
+    return NO_ERROR;
 }
 
 void Visualizer::initSampleRate()
diff --git a/media/jni/audioeffect/Visualizer.h b/media/jni/audioeffect/Visualizer.h
index b38c01f62cf1..26d58d0f56e9 100644
--- a/media/jni/audioeffect/Visualizer.h
+++ b/media/jni/audioeffect/Visualizer.h
@@ -20,6 +20,8 @@
 #include <media/AudioEffect.h>
 #include <system/audio_effects/effect_visualizer.h>
 #include <utils/Thread.h>
+#include <cstdint>
+#include <cutils/bitops.h>
 #include "android/content/AttributionSourceState.h"
 
 /**
@@ -170,8 +172,12 @@ private:
 
     status_t doFft(uint8_t *fft, uint8_t *waveform);
     void periodicCapture();
-    uint32_t initCaptureSize();
+    status_t initCaptureSize();
     void initSampleRate();
+    static constexpr bool isCaptureSizeValid(uint32_t size) {
+        return size <= VISUALIZER_CAPTURE_SIZE_MAX && size >= VISUALIZER_CAPTURE_SIZE_MIN &&
+                popcount(size) == 1;
+    }
 
     Mutex mCaptureLock;
     uint32_t mCaptureRate = CAPTURE_RATE_DEF;