Merge "Refactored Mtp driver interface into multiple classes." am: 16b37bb469
am: 465e8aadf2

Change-Id: I0af37eaa414272fdda0eb8a43c49c812e6742524
diff --git a/media/libaudioclient/IEffect.cpp b/media/libaudioclient/IEffect.cpp
index 115ca75..ce72dae 100644
--- a/media/libaudioclient/IEffect.cpp
+++ b/media/libaudioclient/IEffect.cpp
@@ -25,6 +25,9 @@
 
 namespace android {
 
+// Maximum command/reply size expected
+#define EFFECT_PARAM_SIZE_MAX       65536
+
 enum {
     ENABLE = IBinder::FIRST_CALL_TRANSACTION,
     DISABLE,
@@ -156,6 +159,10 @@
             uint32_t cmdSize = data.readInt32();
             char *cmd = NULL;
             if (cmdSize) {
+                if (cmdSize > EFFECT_PARAM_SIZE_MAX) {
+                    reply->writeInt32(NO_MEMORY);
+                    return NO_ERROR;
+                }
                 cmd = (char *)calloc(cmdSize, 1);
                 if (cmd == NULL) {
                     reply->writeInt32(NO_MEMORY);
@@ -167,6 +174,11 @@
             uint32_t replySz = replySize;
             char *resp = NULL;
             if (replySize) {
+                if (replySize > EFFECT_PARAM_SIZE_MAX) {
+                    free(cmd);
+                    reply->writeInt32(NO_MEMORY);
+                    return NO_ERROR;
+                }
                 resp = (char *)calloc(replySize, 1);
                 if (resp == NULL) {
                     free(cmd);
diff --git a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
index d7b5f65..101dc6b 100644
--- a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
+++ b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
@@ -2357,8 +2357,12 @@
 
     case EQ_PARAM_BAND_LEVEL:
         param2 = *pParamTemp;
-        if (param2 >= FIVEBAND_NUMBANDS) {
+        if (param2 < 0 || param2 >= FIVEBAND_NUMBANDS) {
             status = -EINVAL;
+            if (param2 < 0) {
+                android_errorWriteLog(0x534e4554, "32438598");
+                ALOGW("\tERROR Equalizer_getParameter() EQ_PARAM_BAND_LEVEL band %d", param2);
+            }
             break;
         }
         *(int16_t *)pValue = (int16_t)EqualizerGetBandLevel(pContext, param2);
@@ -2368,8 +2372,12 @@
 
     case EQ_PARAM_CENTER_FREQ:
         param2 = *pParamTemp;
-        if (param2 >= FIVEBAND_NUMBANDS) {
+        if (param2 < 0 || param2 >= FIVEBAND_NUMBANDS) {
             status = -EINVAL;
+            if (param2 < 0) {
+                android_errorWriteLog(0x534e4554, "32436341");
+                ALOGW("\tERROR Equalizer_getParameter() EQ_PARAM_CENTER_FREQ band %d", param2);
+            }
             break;
         }
         *(int32_t *)pValue = EqualizerGetCentreFrequency(pContext, param2);
@@ -2379,8 +2387,12 @@
 
     case EQ_PARAM_BAND_FREQ_RANGE:
         param2 = *pParamTemp;
-        if (param2 >= FIVEBAND_NUMBANDS) {
+        if (param2 < 0 || param2 >= FIVEBAND_NUMBANDS) {
             status = -EINVAL;
+            if (param2 < 0) {
+                android_errorWriteLog(0x534e4554, "32247948");
+                ALOGW("\tERROR Equalizer_getParameter() EQ_PARAM_BAND_FREQ_RANGE band %d", param2);
+            }
             break;
         }
         EqualizerGetBandFreqRange(pContext, param2, (uint32_t *)pValue, ((uint32_t *)pValue + 1));
@@ -2407,9 +2419,13 @@
 
     case EQ_PARAM_GET_PRESET_NAME:
         param2 = *pParamTemp;
-        if (param2 >= EqualizerGetNumPresets()) {
-        //if (param2 >= 20) {     // AGO FIX
+        if ((param2 < 0 && param2 != PRESET_CUSTOM) ||  param2 >= EqualizerGetNumPresets()) {
             status = -EINVAL;
+            if (param2 < 0) {
+                android_errorWriteLog(0x534e4554, "32448258");
+                ALOGE("\tERROR Equalizer_getParameter() EQ_PARAM_GET_PRESET_NAME preset %d",
+                        param2);
+            }
             break;
         }
         name = (char *)pValue;
@@ -2479,8 +2495,12 @@
         band =  *pParamTemp;
         level = (int32_t)(*(int16_t *)pValue);
         //ALOGV("\tEqualizer_setParameter() EQ_PARAM_BAND_LEVEL band %d, level %d", band, level);
-        if (band >= FIVEBAND_NUMBANDS) {
+        if (band < 0 || band >= FIVEBAND_NUMBANDS) {
             status = -EINVAL;
+            if (band < 0) {
+                android_errorWriteLog(0x534e4554, "32095626");
+                ALOGE("\tERROR Equalizer_setParameter() EQ_PARAM_BAND_LEVEL band %d", band);
+            }
             break;
         }
         EqualizerSetBandLevel(pContext, band, level);
@@ -3097,10 +3117,6 @@
             //ALOGV("\tEffect_command cmdCode Case: EFFECT_CMD_GET_PARAM start");
 
             effect_param_t *p = (effect_param_t *)pCmdData;
-            if (SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) {
-                android_errorWriteLog(0x534e4554, "26347509");
-                return -EINVAL;
-            }
             if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) ||
                     cmdSize < (sizeof(effect_param_t) + p->psize) ||
                     pReplyData == NULL || replySize == NULL ||
@@ -3108,13 +3124,32 @@
                 ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: ERROR");
                 return -EINVAL;
             }
+            if (EFFECT_PARAM_SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) {
+                android_errorWriteLog(0x534e4554, "26347509");
+                ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: psize too big");
+                return -EINVAL;
+            }
+            uint32_t paddedParamSize = ((p->psize + sizeof(int32_t) - 1) / sizeof(int32_t)) *
+                    sizeof(int32_t);
+            if ((EFFECT_PARAM_SIZE_MAX - sizeof(effect_param_t) < paddedParamSize) ||
+                (EFFECT_PARAM_SIZE_MAX - sizeof(effect_param_t) - paddedParamSize <
+                    p->vsize)) {
+                ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: padded_psize or vsize too big");
+                return -EINVAL;
+            }
+            uint32_t expectedReplySize = sizeof(effect_param_t) + paddedParamSize + p->vsize;
+            if (*replySize < expectedReplySize) {
+                ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: min. replySize %u, got %u bytes",
+                        expectedReplySize, *replySize);
+                android_errorWriteLog(0x534e4554, "32705438");
+                return -EINVAL;
+            }
 
             memcpy(pReplyData, pCmdData, sizeof(effect_param_t) + p->psize);
 
             p = (effect_param_t *)pReplyData;
 
-            int voffset = ((p->psize - 1) / sizeof(int32_t) + 1) * sizeof(int32_t);
-
+            uint32_t voffset = paddedParamSize;
             if(pContext->EffectType == LVM_BASS_BOOST){
                 p->status = android::BassBoost_getParameter(pContext,
                                                             p->data,
diff --git a/media/libeffects/visualizer/EffectVisualizer.cpp b/media/libeffects/visualizer/EffectVisualizer.cpp
index 21fddb1..b7d27d6 100644
--- a/media/libeffects/visualizer/EffectVisualizer.cpp
+++ b/media/libeffects/visualizer/EffectVisualizer.cpp
@@ -59,6 +59,8 @@
 
 #define DISCARD_MEASUREMENTS_TIME_MS 2000 // discard measurements older than this number of ms
 
+#define MAX_LATENCY_MS 3000 // 3 seconds of latency for audio pipeline
+
 // maximum number of buffers for which we keep track of the measurements
 #define MEASUREMENT_WINDOW_MAX_SIZE_IN_BUFFERS 25 // note: buffer index is stored in uint8_t
 
@@ -521,18 +523,29 @@
             break;
         }
         switch (*(uint32_t *)p->data) {
-        case VISUALIZER_PARAM_CAPTURE_SIZE:
-            pContext->mCaptureSize = *((uint32_t *)p->data + 1);
-            ALOGV("set mCaptureSize = %" PRIu32, pContext->mCaptureSize);
-            break;
+        case VISUALIZER_PARAM_CAPTURE_SIZE: {
+            const uint32_t captureSize = *((uint32_t *)p->data + 1);
+            if (captureSize > VISUALIZER_CAPTURE_SIZE_MAX) {
+                android_errorWriteLog(0x534e4554, "31781965");
+                *(int32_t *)pReplyData = -EINVAL;
+                ALOGW("set mCaptureSize = %u > %u", captureSize, VISUALIZER_CAPTURE_SIZE_MAX);
+            } else {
+                pContext->mCaptureSize = captureSize;
+                ALOGV("set mCaptureSize = %u", captureSize);
+            }
+            } break;
         case VISUALIZER_PARAM_SCALING_MODE:
             pContext->mScalingMode = *((uint32_t *)p->data + 1);
             ALOGV("set mScalingMode = %" PRIu32, pContext->mScalingMode);
             break;
-        case VISUALIZER_PARAM_LATENCY:
-            pContext->mLatency = *((uint32_t *)p->data + 1);
-            ALOGV("set mLatency = %" PRIu32, pContext->mLatency);
-            break;
+        case VISUALIZER_PARAM_LATENCY: {
+            uint32_t latency = *((uint32_t *)p->data + 1);
+            if (latency > MAX_LATENCY_MS) {
+                latency = MAX_LATENCY_MS; // clamp latency b/31781965
+            }
+            pContext->mLatency = latency;
+            ALOGV("set mLatency = %u", latency);
+            } break;
         case VISUALIZER_PARAM_MEASUREMENT_MODE:
             pContext->mMeasurementMode = *((uint32_t *)p->data + 1);
             ALOGV("set mMeasurementMode = %" PRIu32, pContext->mMeasurementMode);
@@ -571,10 +584,18 @@
                 if (latencyMs < 0) {
                     latencyMs = 0;
                 }
-                const uint32_t deltaSmpl =
-                    pContext->mConfig.inputCfg.samplingRate * latencyMs / 1000;
-                int32_t capturePoint = pContext->mCaptureIdx - captureSize - deltaSmpl;
+                uint32_t deltaSmpl = captureSize
+                        + pContext->mConfig.inputCfg.samplingRate * latencyMs / 1000;
 
+                // large sample rate, latency, or capture size, could cause overflow.
+                // do not offset more than the size of buffer.
+                if (deltaSmpl > CAPTURE_BUF_SIZE) {
+                    android_errorWriteLog(0x534e4554, "31781965");
+                    deltaSmpl = CAPTURE_BUF_SIZE;
+                }
+
+                int32_t capturePoint = pContext->mCaptureIdx - deltaSmpl;
+                // a negative capturePoint means we wrap the buffer.
                 if (capturePoint < 0) {
                     uint32_t size = -capturePoint;
                     if (size > captureSize) {
diff --git a/media/libmediaplayerservice/MediaRecorderClient.cpp b/media/libmediaplayerservice/MediaRecorderClient.cpp
index d011d70..609b00d 100644
--- a/media/libmediaplayerservice/MediaRecorderClient.cpp
+++ b/media/libmediaplayerservice/MediaRecorderClient.cpp
@@ -368,10 +368,21 @@
     mRecorder->setListener(listener);
 
     sp<IServiceManager> sm = defaultServiceManager();
-    sp<IBinder> binder = sm->getService(String16("media.camera"));
-    mCameraDeathListener = new ServiceDeathNotifier(binder, listener,
-            MediaPlayerService::CAMERA_PROCESS_DEATH);
-    binder->linkToDeath(mCameraDeathListener);
+
+    // WORKAROUND: We don't know if camera exists here and getService might block for 5 seconds.
+    // Use checkService for camera if we don't know it exists.
+    static std::atomic<bool> sCameraChecked(false);  // once true never becomes false.
+    static std::atomic<bool> sCameraVerified(false); // once true never becomes false.
+    sp<IBinder> binder = (sCameraVerified || !sCameraChecked)
+        ? sm->getService(String16("media.camera")) : sm->checkService(String16("media.camera"));
+    // If the device does not have a camera, do not create a death listener for it.
+    if (binder != NULL) {
+        sCameraVerified = true;
+        mCameraDeathListener = new ServiceDeathNotifier(binder, listener,
+                MediaPlayerService::CAMERA_PROCESS_DEATH);
+        binder->linkToDeath(mCameraDeathListener);
+    }
+    sCameraChecked = true;
 
     binder = sm->getService(String16("media.codec"));
     mCodecDeathListener = new ServiceDeathNotifier(binder, listener,
diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp
index cf38efc..594128c 100644
--- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp
+++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp
@@ -754,7 +754,7 @@
 
 status_t NuPlayer::Decoder::fetchInputData(sp<AMessage> &reply) {
     sp<ABuffer> accessUnit;
-    bool dropAccessUnit;
+    bool dropAccessUnit = true;
     do {
         status_t err = mSource->dequeueAccessUnit(mIsAudio, &accessUnit);
 
diff --git a/media/libstagefright/VBRISeeker.cpp b/media/libstagefright/VBRISeeker.cpp
index 58f2c60..5b8f23a 100644
--- a/media/libstagefright/VBRISeeker.cpp
+++ b/media/libstagefright/VBRISeeker.cpp
@@ -83,8 +83,23 @@
          scale,
          entrySize);
 
+    if (entrySize > 4) {
+        ALOGE("invalid VBRI entry size: %zu", entrySize);
+        return NULL;
+    }
+
+    sp<VBRISeeker> seeker = new (std::nothrow) VBRISeeker;
+    if (seeker == NULL) {
+        ALOGW("Couldn't allocate VBRISeeker");
+        return NULL;
+    }
+
     size_t totalEntrySize = numEntries * entrySize;
-    uint8_t *buffer = new uint8_t[totalEntrySize];
+    uint8_t *buffer = new (std::nothrow) uint8_t[totalEntrySize];
+    if (!buffer) {
+        ALOGW("Couldn't allocate %zu bytes", totalEntrySize);
+        return NULL;
+    }
 
     n = source->readAt(pos + sizeof(vbriHeader), buffer, totalEntrySize);
     if (n < (ssize_t)totalEntrySize) {
@@ -94,7 +109,6 @@
         return NULL;
     }
 
-    sp<VBRISeeker> seeker = new VBRISeeker;
     seeker->mBasePos = post_id3_pos + frameSize;
     // only update mDurationUs if the calculated duration is valid (non zero)
     // otherwise, leave duration at -1 so that getDuration() and getOffsetForTime()
diff --git a/media/libstagefright/id3/ID3.cpp b/media/libstagefright/id3/ID3.cpp
index aaf6b3d..a0eb630 100644
--- a/media/libstagefright/id3/ID3.cpp
+++ b/media/libstagefright/id3/ID3.cpp
@@ -839,20 +839,21 @@
     }
 }
 
-static size_t StringSize(const uint8_t *start, uint8_t encoding) {
+// return includes terminator;  if unterminated, returns > limit
+static size_t StringSize(const uint8_t *start, size_t limit, uint8_t encoding) {
+
     if (encoding == 0x00 || encoding == 0x03) {
         // ISO 8859-1 or UTF-8
-        return strlen((const char *)start) + 1;
+        return strnlen((const char *)start, limit) + 1;
     }
 
     // UCS-2
     size_t n = 0;
-    while (start[n] != '\0' || start[n + 1] != '\0') {
+    while ((n+1 < limit) && (start[n] != '\0' || start[n + 1] != '\0')) {
         n += 2;
     }
-
-    // Add size of null termination.
-    return n + 2;
+    n += 2;
+    return n;
 }
 
 const void *
@@ -873,11 +874,19 @@
 
         if (mVersion == ID3_V2_3 || mVersion == ID3_V2_4) {
             uint8_t encoding = data[0];
-            mime->setTo((const char *)&data[1]);
-            size_t mimeLen = strlen((const char *)&data[1]) + 1;
+            size_t consumed = 1;
+
+            // *always* in an 8-bit encoding
+            size_t mimeLen = StringSize(&data[consumed], size - consumed, 0x00);
+            if (mimeLen > size - consumed) {
+                ALOGW("bogus album art size: mime");
+                return NULL;
+            }
+            mime->setTo((const char *)&data[consumed]);
+            consumed += mimeLen;
 
 #if 0
-            uint8_t picType = data[1 + mimeLen];
+            uint8_t picType = data[consumed];
             if (picType != 0x03) {
                 // Front Cover Art
                 it.next();
@@ -885,20 +894,30 @@
             }
 #endif
 
-            size_t descLen = StringSize(&data[2 + mimeLen], encoding);
-
-            if (size < 2 ||
-                    size - 2 < mimeLen ||
-                    size - 2 - mimeLen < descLen) {
-                ALOGW("bogus album art sizes");
+            consumed++;
+            if (consumed >= size) {
+                ALOGW("bogus album art size: pic type");
                 return NULL;
             }
-            *length = size - 2 - mimeLen - descLen;
 
-            return &data[2 + mimeLen + descLen];
+            size_t descLen = StringSize(&data[consumed], size - consumed, encoding);
+            consumed += descLen;
+
+            if (consumed >= size) {
+                ALOGW("bogus album art size: description");
+                return NULL;
+            }
+
+            *length = size - consumed;
+
+            return &data[consumed];
         } else {
             uint8_t encoding = data[0];
 
+            if (size <= 5) {
+                return NULL;
+            }
+
             if (!memcmp(&data[1], "PNG", 3)) {
                 mime->setTo("image/png");
             } else if (!memcmp(&data[1], "JPG", 3)) {
@@ -918,7 +937,10 @@
             }
 #endif
 
-            size_t descLen = StringSize(&data[5], encoding);
+            size_t descLen = StringSize(&data[5], size - 5, encoding);
+            if (descLen > size - 5) {
+                return NULL;
+            }
 
             *length = size - 5 - descLen;
 
diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp
index 8c10310..355a2dd 100644
--- a/media/libstagefright/omx/OMXNodeInstance.cpp
+++ b/media/libstagefright/omx/OMXNodeInstance.cpp
@@ -805,13 +805,6 @@
         }
         memset(data, 0, allottedSize);
 
-        // if we are not connecting the buffers, the sizes must match
-        if (allottedSize != params->size()) {
-            CLOG_ERROR(useBuffer, BAD_VALUE, SIMPLE_BUFFER(portIndex, (size_t)allottedSize, data));
-            delete[] data;
-            return BAD_VALUE;
-        }
-
         buffer_meta = new BufferMeta(
                 params, portIndex, false /* copyToOmx */, false /* copyFromOmx */, data);
     } else {
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 20d2896..8507bee 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -602,6 +602,29 @@
         android_errorWriteLog(0x534e4554, "29251553");
         return -EINVAL;
     }
+    if (cmdCode == EFFECT_CMD_GET_PARAM &&
+            (sizeof(effect_param_t) > cmdSize ||
+                    ((effect_param_t *)pCmdData)->psize > cmdSize
+                                                          - sizeof(effect_param_t))) {
+        android_errorWriteLog(0x534e4554, "32438594");
+        return -EINVAL;
+    }
+    if (cmdCode == EFFECT_CMD_GET_PARAM &&
+        (sizeof(effect_param_t) > *replySize
+          || ((effect_param_t *)pCmdData)->psize > *replySize
+                                                   - sizeof(effect_param_t)
+          || ((effect_param_t *)pCmdData)->vsize > *replySize
+                                                   - sizeof(effect_param_t)
+                                                   - ((effect_param_t *)pCmdData)->psize
+          || roundUpDelta(((effect_param_t *)pCmdData)->psize, (uint32_t)sizeof(int)) >
+                                                   *replySize
+                                                   - sizeof(effect_param_t)
+                                                   - ((effect_param_t *)pCmdData)->psize
+                                                   - ((effect_param_t *)pCmdData)->vsize)) {
+        ALOGV("\tLVM_ERROR : EFFECT_CMD_GET_PARAM: reply size inconsistent");
+                     android_errorWriteLog(0x534e4554, "32705438");
+        return -EINVAL;
+    }
     if ((cmdCode == EFFECT_CMD_SET_PARAM
             || cmdCode == EFFECT_CMD_SET_PARAM_DEFERRED) &&  // DEFERRED not generally used
         (sizeof(effect_param_t) > cmdSize
@@ -1284,36 +1307,54 @@
         // particular client process:  no risk to block the whole media server process or mixer
         // threads if we are stuck here
         Mutex::Autolock _l(mCblk->lock);
-        if (mCblk->clientIndex > EFFECT_PARAM_BUFFER_SIZE ||
-            mCblk->serverIndex > EFFECT_PARAM_BUFFER_SIZE) {
+
+        // keep local copy of index in case of client corruption b/32220769
+        const uint32_t clientIndex = mCblk->clientIndex;
+        const uint32_t serverIndex = mCblk->serverIndex;
+        if (clientIndex > EFFECT_PARAM_BUFFER_SIZE ||
+            serverIndex > EFFECT_PARAM_BUFFER_SIZE) {
             mCblk->serverIndex = 0;
             mCblk->clientIndex = 0;
             return BAD_VALUE;
         }
         status_t status = NO_ERROR;
-        while (mCblk->serverIndex < mCblk->clientIndex) {
-            int reply;
-            uint32_t rsize = sizeof(int);
-            int *p = (int *)(mBuffer + mCblk->serverIndex);
-            int size = *p++;
-            if (((uint8_t *)p + size) > mBuffer + mCblk->clientIndex) {
+        effect_param_t *param = NULL;
+        for (uint32_t index = serverIndex; index < clientIndex;) {
+            int *p = (int *)(mBuffer + index);
+            const int size = *p++;
+            if (size < 0
+                    || size > EFFECT_PARAM_BUFFER_SIZE
+                    || ((uint8_t *)p + size) > mBuffer + clientIndex) {
                 ALOGW("command(): invalid parameter block size");
+                status = BAD_VALUE;
                 break;
             }
-            effect_param_t *param = (effect_param_t *)p;
-            if (param->psize == 0 || param->vsize == 0) {
-                ALOGW("command(): null parameter or value size");
-                mCblk->serverIndex += size;
-                continue;
+
+            // copy to local memory in case of client corruption b/32220769
+            param = (effect_param_t *)realloc(param, size);
+            if (param == NULL) {
+                ALOGW("command(): out of memory");
+                status = NO_MEMORY;
+                break;
             }
-            uint32_t psize = sizeof(effect_param_t) +
-                             ((param->psize - 1) / sizeof(int) + 1) * sizeof(int) +
-                             param->vsize;
+            memcpy(param, p, size);
+
+            int reply = 0;
+            uint32_t rsize = sizeof(reply);
             status_t ret = mEffect->command(EFFECT_CMD_SET_PARAM,
-                                            psize,
-                                            p,
+                                            size,
+                                            param,
                                             &rsize,
                                             &reply);
+
+            // verify shared memory: server index shouldn't change; client index can't go back.
+            if (serverIndex != mCblk->serverIndex
+                    || clientIndex > mCblk->clientIndex) {
+                android_errorWriteLog(0x534e4554, "32220769");
+                status = BAD_VALUE;
+                break;
+            }
+
             // stop at first error encountered
             if (ret != NO_ERROR) {
                 status = ret;
@@ -1323,8 +1364,9 @@
                 *(int *)pReplyData = reply;
                 break;
             }
-            mCblk->serverIndex += size;
+            index += size;
         }
+        free(param);
         mCblk->serverIndex = 0;
         mCblk->clientIndex = 0;
         return status;
diff --git a/services/camera/libcameraservice/api1/client2/Parameters.cpp b/services/camera/libcameraservice/api1/client2/Parameters.cpp
index 9d5f33c..9a7839b 100644
--- a/services/camera/libcameraservice/api1/client2/Parameters.cpp
+++ b/services/camera/libcameraservice/api1/client2/Parameters.cpp
@@ -238,6 +238,10 @@
     {
         String8 supportedPreviewFpsRange;
         for (size_t i=0; i < availableFpsRanges.count; i += 2) {
+            if (!isFpsSupported(availablePreviewSizes,
+                HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED, availableFpsRanges.data.i32[i+1])) {
+                continue;
+            }
             if (i != 0) supportedPreviewFpsRange += ",";
             supportedPreviewFpsRange += String8::format("(%d,%d)",
                     availableFpsRanges.data.i32[i] * kFpsToApiScale,
@@ -255,7 +259,10 @@
             // from the [min, max] fps range use the max value
             int fps = fpsFromRange(availableFpsRanges.data.i32[i],
                                    availableFpsRanges.data.i32[i+1]);
-
+            if (!isFpsSupported(availablePreviewSizes,
+                    HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED, fps)) {
+                continue;
+            }
             // de-dupe frame rates
             if (sortedPreviewFrameRates.indexOf(fps) == NAME_NOT_FOUND) {
                 sortedPreviewFrameRates.add(fps);
@@ -951,21 +958,40 @@
         return NO_INIT;
     }
 
+    // Get supported preview fps ranges.
+    Vector<Size> supportedPreviewSizes;
+    Vector<FpsRange> supportedPreviewFpsRanges;
+    const Size PREVIEW_SIZE_BOUND = { MAX_PREVIEW_WIDTH, MAX_PREVIEW_HEIGHT };
+    status_t res = getFilteredSizes(PREVIEW_SIZE_BOUND, &supportedPreviewSizes);
+    if (res != OK) return res;
+    for (size_t i=0; i < availableFpsRanges.count; i += 2) {
+        if (!isFpsSupported(supportedPreviewSizes,
+                HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED, availableFpsRanges.data.i32[i+1])) {
+            continue;
+        }
+        FpsRange fpsRange = {availableFpsRanges.data.i32[i], availableFpsRanges.data.i32[i+1]};
+        supportedPreviewFpsRanges.add(fpsRange);
+    }
+    if (supportedPreviewFpsRanges.size() == 0) {
+        ALOGE("Supported preview fps range is empty");
+        return NO_INIT;
+    }
+
     int32_t bestStillCaptureFpsRange[2] = {
-        availableFpsRanges.data.i32[0], availableFpsRanges.data.i32[1]
+        supportedPreviewFpsRanges[0].low, supportedPreviewFpsRanges[0].high
     };
     int32_t curRange =
             bestStillCaptureFpsRange[1] - bestStillCaptureFpsRange[0];
-    for (size_t i = 2; i < availableFpsRanges.count; i += 2) {
+    for (size_t i = 1; i < supportedPreviewFpsRanges.size(); i ++) {
         int32_t nextRange =
-                availableFpsRanges.data.i32[i + 1] -
-                availableFpsRanges.data.i32[i];
+                supportedPreviewFpsRanges[i].high -
+                supportedPreviewFpsRanges[i].low;
         if ( (nextRange > curRange) ||       // Maximize size of FPS range first
                 (nextRange == curRange &&    // Then minimize low-end FPS
-                 bestStillCaptureFpsRange[0] > availableFpsRanges.data.i32[i])) {
+                 bestStillCaptureFpsRange[0] > supportedPreviewFpsRanges[i].low)) {
 
-            bestStillCaptureFpsRange[0] = availableFpsRanges.data.i32[i];
-            bestStillCaptureFpsRange[1] = availableFpsRanges.data.i32[i + 1];
+            bestStillCaptureFpsRange[0] = supportedPreviewFpsRanges[i].low;
+            bestStillCaptureFpsRange[1] = supportedPreviewFpsRanges[i].high;
             curRange = nextRange;
         }
     }
@@ -2836,22 +2862,7 @@
 
 int64_t Parameters::getJpegStreamMinFrameDurationNs(Parameters::Size size) {
     if (mDeviceVersion >= CAMERA_DEVICE_API_VERSION_3_2) {
-        const int STREAM_DURATION_SIZE = 4;
-        const int STREAM_FORMAT_OFFSET = 0;
-        const int STREAM_WIDTH_OFFSET = 1;
-        const int STREAM_HEIGHT_OFFSET = 2;
-        const int STREAM_DURATION_OFFSET = 3;
-        camera_metadata_ro_entry_t availableStreamMinDurations =
-                    staticInfo(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS);
-        for (size_t i = 0; i < availableStreamMinDurations.count; i+= STREAM_DURATION_SIZE) {
-            int64_t format = availableStreamMinDurations.data.i64[i + STREAM_FORMAT_OFFSET];
-            int64_t width = availableStreamMinDurations.data.i64[i + STREAM_WIDTH_OFFSET];
-            int64_t height = availableStreamMinDurations.data.i64[i + STREAM_HEIGHT_OFFSET];
-            int64_t duration = availableStreamMinDurations.data.i64[i + STREAM_DURATION_OFFSET];
-            if (format == HAL_PIXEL_FORMAT_BLOB && width == size.width && height == size.height) {
-                return duration;
-            }
-        }
+        return getMinFrameDurationNs(size, HAL_PIXEL_FORMAT_BLOB);
     } else {
         Vector<Size> availableJpegSizes = getAvailableJpegSizes();
         size_t streamIdx = availableJpegSizes.size();
@@ -2875,6 +2886,57 @@
     return -1;
 }
 
+int64_t Parameters::getMinFrameDurationNs(Parameters::Size size, int fmt) {
+    if (mDeviceVersion < CAMERA_DEVICE_API_VERSION_3_2) {
+        ALOGE("Min frame duration for HAL 3.1 or lower is not supported");
+        return -1;
+    }
+
+    const int STREAM_DURATION_SIZE = 4;
+    const int STREAM_FORMAT_OFFSET = 0;
+    const int STREAM_WIDTH_OFFSET = 1;
+    const int STREAM_HEIGHT_OFFSET = 2;
+    const int STREAM_DURATION_OFFSET = 3;
+    camera_metadata_ro_entry_t availableStreamMinDurations =
+                staticInfo(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS);
+    for (size_t i = 0; i < availableStreamMinDurations.count; i+= STREAM_DURATION_SIZE) {
+        int64_t format = availableStreamMinDurations.data.i64[i + STREAM_FORMAT_OFFSET];
+        int64_t width = availableStreamMinDurations.data.i64[i + STREAM_WIDTH_OFFSET];
+        int64_t height = availableStreamMinDurations.data.i64[i + STREAM_HEIGHT_OFFSET];
+        int64_t duration = availableStreamMinDurations.data.i64[i + STREAM_DURATION_OFFSET];
+        if (format == fmt && width == size.width && height == size.height) {
+            return duration;
+        }
+    }
+
+    return -1;
+}
+
+bool Parameters::isFpsSupported(const Vector<Size> &sizes, int format, int32_t fps) {
+    // Skip the check for older HAL version, as the min duration is not supported.
+    if (mDeviceVersion < CAMERA_DEVICE_API_VERSION_3_2) {
+        return true;
+    }
+
+    // Get min frame duration for each size and check if the given fps range can be supported.
+    const int32_t FPS_MARGIN = 1;
+    for (size_t i = 0 ; i < sizes.size(); i++) {
+        int64_t minFrameDuration = getMinFrameDurationNs(sizes[i], format);
+        if (minFrameDuration <= 0) {
+            ALOGE("Min frame duration (%" PRId64") for size (%dx%d) and format 0x%x is wrong!",
+                minFrameDuration, sizes[i].width, sizes[i].height, format);
+            return false;
+        }
+        int32_t maxSupportedFps = 1e9 / minFrameDuration;
+        // Add some margin here for the case where the hal supports 29.xxxfps.
+        maxSupportedFps += FPS_MARGIN;
+        if (fps > maxSupportedFps) {
+            return false;
+        }
+    }
+    return true;
+}
+
 SortedVector<int32_t> Parameters::getAvailableOutputFormats() {
     SortedVector<int32_t> outputFormats; // Non-duplicated output formats
     if (mDeviceVersion >= CAMERA_DEVICE_API_VERSION_3_2) {
diff --git a/services/camera/libcameraservice/api1/client2/Parameters.h b/services/camera/libcameraservice/api1/client2/Parameters.h
index 798aab5..c8ecbba 100644
--- a/services/camera/libcameraservice/api1/client2/Parameters.h
+++ b/services/camera/libcameraservice/api1/client2/Parameters.h
@@ -115,6 +115,11 @@
         int32_t height;
     };
 
+    struct FpsRange {
+        int32_t low;
+        int32_t high;
+    };
+
     int32_t exposureCompensation;
     bool autoExposureLock;
     bool autoWhiteBalanceLock;
@@ -390,6 +395,15 @@
     // return -1 if input jpeg size cannot be found in supported size list
     int64_t getJpegStreamMinFrameDurationNs(Parameters::Size size);
 
+    // Helper function to get minimum frame duration for a size/format combination
+    // return -1 if input size/format combination cannot be found.
+    int64_t getMinFrameDurationNs(Parameters::Size size, int format);
+
+    // Helper function to check if a given fps is supported by all the sizes with
+    // the same format.
+    // return true if the device doesn't support min frame duration metadata tag.
+    bool isFpsSupported(const Vector<Size> &size, int format, int32_t fps);
+
     // Helper function to get non-duplicated available output formats
     SortedVector<int32_t> getAvailableOutputFormats();
     // Helper function to get available output jpeg sizes