DO NOT MERGE: camera: Fix setParameters for Preview FPS single/range values

As a workaround, duplicate CameraParameters into CameraParameters2 to
prevent ABI break for some camera HALs that directly link into
CameraParameters.

CameraParameters2 implements the real fixes needed in the framework,
while CameraParameters is left in to satisfy older camera HALs.

Bug: 12609188
Change-Id: I82ea6f5de2183dd046d4bf5683600c97f37ab4da
diff --git a/camera/Android.mk b/camera/Android.mk
index e633450..5cedab0 100644
--- a/camera/Android.mk
+++ b/camera/Android.mk
@@ -8,6 +8,7 @@
 	Camera.cpp \
 	CameraMetadata.cpp \
 	CameraParameters.cpp \
+	CameraParameters2.cpp \
 	ICamera.cpp \
 	ICameraClient.cpp \
 	ICameraService.cpp \
diff --git a/camera/CameraParameters2.cpp b/camera/CameraParameters2.cpp
new file mode 100644
index 0000000..eac79e1
--- /dev/null
+++ b/camera/CameraParameters2.cpp
@@ -0,0 +1,381 @@
+/*
+**
+** Copyright 2008, The Android Open Source Project
+**
+** Licensed under the Apache License, Version 2.0 (the "License");
+** you may not use this file except in compliance with the License.
+** You may obtain a copy of the License at
+**
+**     http://www.apache.org/licenses/LICENSE-2.0
+**
+** Unless required by applicable law or agreed to in writing, software
+** distributed under the License is distributed on an "AS IS" BASIS,
+** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+** See the License for the specific language governing permissions and
+** limitations under the License.
+*/
+
+#define LOG_TAG "CameraParams2"
+// #define LOG_NDEBUG 0
+#include <utils/Log.h>
+
+#include <string.h>
+#include <stdlib.h>
+#include <camera/CameraParameters2.h>
+
+namespace android {
+
+CameraParameters2::CameraParameters2()
+                : mMap()
+{
+}
+
+CameraParameters2::~CameraParameters2()
+{
+}
+
+String8 CameraParameters2::flatten() const
+{
+    String8 flattened("");
+    size_t size = mMap.size();
+
+    for (size_t i = 0; i < size; i++) {
+        String8 k, v;
+        k = mMap.keyAt(i);
+        v = mMap.valueAt(i);
+
+        flattened += k;
+        flattened += "=";
+        flattened += v;
+        if (i != size-1)
+            flattened += ";";
+    }
+
+    ALOGV("%s: Flattened params = %s", __FUNCTION__, flattened.string());
+
+    return flattened;
+}
+
+void CameraParameters2::unflatten(const String8 &params)
+{
+    const char *a = params.string();
+    const char *b;
+
+    mMap.clear();
+
+    for (;;) {
+        // Find the bounds of the key name.
+        b = strchr(a, '=');
+        if (b == 0)
+            break;
+
+        // Create the key string.
+        String8 k(a, (size_t)(b-a));
+
+        // Find the value.
+        a = b+1;
+        b = strchr(a, ';');
+        if (b == 0) {
+            // If there's no semicolon, this is the last item.
+            String8 v(a);
+            mMap.add(k, v);
+            break;
+        }
+
+        String8 v(a, (size_t)(b-a));
+        mMap.add(k, v);
+        a = b+1;
+    }
+}
+
+
+void CameraParameters2::set(const char *key, const char *value)
+{
+    // XXX i think i can do this with strspn()
+    if (strchr(key, '=') || strchr(key, ';')) {
+        //XXX ALOGE("Key \"%s\"contains invalid character (= or ;)", key);
+        return;
+    }
+
+    if (strchr(value, '=') || strchr(value, ';')) {
+        //XXX ALOGE("Value \"%s\"contains invalid character (= or ;)", value);
+        return;
+    }
+
+    // Replacing a value updates the key's order to be the new largest order
+    ssize_t res = mMap.replaceValueFor(String8(key), String8(value));
+    LOG_ALWAYS_FATAL_IF(res < 0, "replaceValueFor(%s,%s) failed", key, value);
+}
+
+void CameraParameters2::set(const char *key, int value)
+{
+    char str[16];
+    sprintf(str, "%d", value);
+    set(key, str);
+}
+
+void CameraParameters2::setFloat(const char *key, float value)
+{
+    char str[16];  // 14 should be enough. We overestimate to be safe.
+    snprintf(str, sizeof(str), "%g", value);
+    set(key, str);
+}
+
+const char *CameraParameters2::get(const char *key) const
+{
+    ssize_t idx = mMap.indexOfKey(String8(key));
+    if (idx < 0) {
+        return NULL;
+    } else {
+        return mMap.valueAt(idx).string();
+    }
+}
+
+int CameraParameters2::getInt(const char *key) const
+{
+    const char *v = get(key);
+    if (v == 0)
+        return -1;
+    return strtol(v, 0, 0);
+}
+
+float CameraParameters2::getFloat(const char *key) const
+{
+    const char *v = get(key);
+    if (v == 0) return -1;
+    return strtof(v, 0);
+}
+
+status_t CameraParameters2::compareSetOrder(const char *key1, const char *key2,
+        int *order) const {
+    if (key1 == NULL) {
+        ALOGE("%s: key1 must not be NULL", __FUNCTION__);
+        return BAD_VALUE;
+    } else if (key2 == NULL) {
+        ALOGE("%s: key2 must not be NULL", __FUNCTION__);
+        return BAD_VALUE;
+    } else if (order == NULL) {
+        ALOGE("%s: order must not be NULL", __FUNCTION__);
+        return BAD_VALUE;
+    }
+
+    ssize_t index1 = mMap.indexOfKey(String8(key1));
+    ssize_t index2 = mMap.indexOfKey(String8(key2));
+    if (index1 < 0) {
+        ALOGW("%s: Key1 (%s) was not set", __FUNCTION__, key1);
+        return NAME_NOT_FOUND;
+    } else if (index2 < 0) {
+        ALOGW("%s: Key2 (%s) was not set", __FUNCTION__, key2);
+        return NAME_NOT_FOUND;
+    }
+
+    *order = (index1 == index2) ? 0  :
+             (index1 < index2)  ? -1 :
+             1;
+
+    return OK;
+}
+
+void CameraParameters2::remove(const char *key)
+{
+    mMap.removeItem(String8(key));
+}
+
+// Parse string like "640x480" or "10000,20000"
+static int parse_pair(const char *str, int *first, int *second, char delim,
+                      char **endptr = NULL)
+{
+    // Find the first integer.
+    char *end;
+    int w = (int)strtol(str, &end, 10);
+    // If a delimeter does not immediately follow, give up.
+    if (*end != delim) {
+        ALOGE("Cannot find delimeter (%c) in str=%s", delim, str);
+        return -1;
+    }
+
+    // Find the second integer, immediately after the delimeter.
+    int h = (int)strtol(end+1, &end, 10);
+
+    *first = w;
+    *second = h;
+
+    if (endptr) {
+        *endptr = end;
+    }
+
+    return 0;
+}
+
+static void parseSizesList(const char *sizesStr, Vector<Size> &sizes)
+{
+    if (sizesStr == 0) {
+        return;
+    }
+
+    char *sizeStartPtr = (char *)sizesStr;
+
+    while (true) {
+        int width, height;
+        int success = parse_pair(sizeStartPtr, &width, &height, 'x',
+                                 &sizeStartPtr);
+        if (success == -1 || (*sizeStartPtr != ',' && *sizeStartPtr != '\0')) {
+            ALOGE("Picture sizes string \"%s\" contains invalid character.", sizesStr);
+            return;
+        }
+        sizes.push(Size(width, height));
+
+        if (*sizeStartPtr == '\0') {
+            return;
+        }
+        sizeStartPtr++;
+    }
+}
+
+void CameraParameters2::setPreviewSize(int width, int height)
+{
+    char str[32];
+    sprintf(str, "%dx%d", width, height);
+    set(CameraParameters::KEY_PREVIEW_SIZE, str);
+}
+
+void CameraParameters2::getPreviewSize(int *width, int *height) const
+{
+    *width = *height = -1;
+    // Get the current string, if it doesn't exist, leave the -1x-1
+    const char *p = get(CameraParameters::KEY_PREVIEW_SIZE);
+    if (p == 0)  return;
+    parse_pair(p, width, height, 'x');
+}
+
+void CameraParameters2::getPreferredPreviewSizeForVideo(int *width, int *height) const
+{
+    *width = *height = -1;
+    const char *p = get(CameraParameters::KEY_PREFERRED_PREVIEW_SIZE_FOR_VIDEO);
+    if (p == 0)  return;
+    parse_pair(p, width, height, 'x');
+}
+
+void CameraParameters2::getSupportedPreviewSizes(Vector<Size> &sizes) const
+{
+    const char *previewSizesStr = get(CameraParameters::KEY_SUPPORTED_PREVIEW_SIZES);
+    parseSizesList(previewSizesStr, sizes);
+}
+
+void CameraParameters2::setVideoSize(int width, int height)
+{
+    char str[32];
+    sprintf(str, "%dx%d", width, height);
+    set(CameraParameters::KEY_VIDEO_SIZE, str);
+}
+
+void CameraParameters2::getVideoSize(int *width, int *height) const
+{
+    *width = *height = -1;
+    const char *p = get(CameraParameters::KEY_VIDEO_SIZE);
+    if (p == 0) return;
+    parse_pair(p, width, height, 'x');
+}
+
+void CameraParameters2::getSupportedVideoSizes(Vector<Size> &sizes) const
+{
+    const char *videoSizesStr = get(CameraParameters::KEY_SUPPORTED_VIDEO_SIZES);
+    parseSizesList(videoSizesStr, sizes);
+}
+
+void CameraParameters2::setPreviewFrameRate(int fps)
+{
+    set(CameraParameters::KEY_PREVIEW_FRAME_RATE, fps);
+}
+
+int CameraParameters2::getPreviewFrameRate() const
+{
+    return getInt(CameraParameters::KEY_PREVIEW_FRAME_RATE);
+}
+
+void CameraParameters2::getPreviewFpsRange(int *min_fps, int *max_fps) const
+{
+    *min_fps = *max_fps = -1;
+    const char *p = get(CameraParameters::KEY_PREVIEW_FPS_RANGE);
+    if (p == 0) return;
+    parse_pair(p, min_fps, max_fps, ',');
+}
+
+void CameraParameters2::setPreviewFpsRange(int min_fps, int max_fps)
+{
+    String8 str = String8::format("%d,%d", min_fps, max_fps);
+    set(CameraParameters::KEY_PREVIEW_FPS_RANGE, str.string());
+}
+
+void CameraParameters2::setPreviewFormat(const char *format)
+{
+    set(CameraParameters::KEY_PREVIEW_FORMAT, format);
+}
+
+const char *CameraParameters2::getPreviewFormat() const
+{
+    return get(CameraParameters::KEY_PREVIEW_FORMAT);
+}
+
+void CameraParameters2::setPictureSize(int width, int height)
+{
+    char str[32];
+    sprintf(str, "%dx%d", width, height);
+    set(CameraParameters::KEY_PICTURE_SIZE, str);
+}
+
+void CameraParameters2::getPictureSize(int *width, int *height) const
+{
+    *width = *height = -1;
+    // Get the current string, if it doesn't exist, leave the -1x-1
+    const char *p = get(CameraParameters::KEY_PICTURE_SIZE);
+    if (p == 0) return;
+    parse_pair(p, width, height, 'x');
+}
+
+void CameraParameters2::getSupportedPictureSizes(Vector<Size> &sizes) const
+{
+    const char *pictureSizesStr = get(CameraParameters::KEY_SUPPORTED_PICTURE_SIZES);
+    parseSizesList(pictureSizesStr, sizes);
+}
+
+void CameraParameters2::setPictureFormat(const char *format)
+{
+    set(CameraParameters::KEY_PICTURE_FORMAT, format);
+}
+
+const char *CameraParameters2::getPictureFormat() const
+{
+    return get(CameraParameters::KEY_PICTURE_FORMAT);
+}
+
+void CameraParameters2::dump() const
+{
+    ALOGD("dump: mMap.size = %d", mMap.size());
+    for (size_t i = 0; i < mMap.size(); i++) {
+        String8 k, v;
+        k = mMap.keyAt(i);
+        v = mMap.valueAt(i);
+        ALOGD("%s: %s\n", k.string(), v.string());
+    }
+}
+
+status_t CameraParameters2::dump(int fd, const Vector<String16>& args) const
+{
+    const size_t SIZE = 256;
+    char buffer[SIZE];
+    String8 result;
+    snprintf(buffer, 255, "CameraParameters2::dump: mMap.size = %zu\n", mMap.size());
+    result.append(buffer);
+    for (size_t i = 0; i < mMap.size(); i++) {
+        String8 k, v;
+        k = mMap.keyAt(i);
+        v = mMap.valueAt(i);
+        snprintf(buffer, 255, "\t%s: %s\n", k.string(), v.string());
+        result.append(buffer);
+    }
+    write(fd, result.string(), result.size());
+    return NO_ERROR;
+}
+
+}; // namespace android
diff --git a/include/camera/CameraParameters2.h b/include/camera/CameraParameters2.h
new file mode 100644
index 0000000..88ad812
--- /dev/null
+++ b/include/camera/CameraParameters2.h
@@ -0,0 +1,203 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_HARDWARE_CAMERA_PARAMETERS2_H
+#define ANDROID_HARDWARE_CAMERA_PARAMETERS2_H
+
+#include <utils/Vector.h>
+#include <utils/String8.h>
+#include "CameraParameters.h"
+
+namespace android {
+
+/**
+ * A copy of CameraParameters plus ABI-breaking changes. Needed
+ * because some camera HALs directly link to CameraParameters and cannot
+ * tolerate an ABI change.
+ */
+class CameraParameters2
+{
+public:
+    CameraParameters2();
+    CameraParameters2(const String8 &params) { unflatten(params); }
+    ~CameraParameters2();
+
+    String8 flatten() const;
+    void unflatten(const String8 &params);
+
+    void set(const char *key, const char *value);
+    void set(const char *key, int value);
+    void setFloat(const char *key, float value);
+    // Look up string value by key.
+    // -- The string remains valid until the next set/remove of the same key,
+    //    or until the map gets cleared.
+    const char *get(const char *key) const;
+    int getInt(const char *key) const;
+    float getFloat(const char *key) const;
+
+    // Compare the order that key1 was set vs the order that key2 was set.
+    //
+    // Sets the order parameter to an integer less than, equal to, or greater
+    // than zero if key1's set order was respectively, to be less than, to
+    // match, or to be greater than key2's set order.
+    //
+    // Error codes:
+    //  * NAME_NOT_FOUND - if either key has not been set previously
+    //  * BAD_VALUE - if any of the parameters are NULL
+    status_t compareSetOrder(const char *key1, const char *key2,
+            /*out*/
+            int *order) const;
+
+    void remove(const char *key);
+
+    void setPreviewSize(int width, int height);
+    void getPreviewSize(int *width, int *height) const;
+    void getSupportedPreviewSizes(Vector<Size> &sizes) const;
+
+    // Set the dimensions in pixels to the given width and height
+    // for video frames. The given width and height must be one
+    // of the supported dimensions returned from
+    // getSupportedVideoSizes(). Must not be called if
+    // getSupportedVideoSizes() returns an empty Vector of Size.
+    void setVideoSize(int width, int height);
+    // Retrieve the current dimensions (width and height)
+    // in pixels for video frames, which must be one of the
+    // supported dimensions returned from getSupportedVideoSizes().
+    // Must not be called if getSupportedVideoSizes() returns an
+    // empty Vector of Size.
+    void getVideoSize(int *width, int *height) const;
+    // Retrieve a Vector of supported dimensions (width and height)
+    // in pixels for video frames. If sizes returned from the method
+    // is empty, the camera does not support calls to setVideoSize()
+    // or getVideoSize(). In adddition, it also indicates that
+    // the camera only has a single output, and does not have
+    // separate output for video frames and preview frame.
+    void getSupportedVideoSizes(Vector<Size> &sizes) const;
+    // Retrieve the preferred preview size (width and height) in pixels
+    // for video recording. The given width and height must be one of
+    // supported preview sizes returned from getSupportedPreviewSizes().
+    // Must not be called if getSupportedVideoSizes() returns an empty
+    // Vector of Size. If getSupportedVideoSizes() returns an empty
+    // Vector of Size, the width and height returned from this method
+    // is invalid, and is "-1x-1".
+    void getPreferredPreviewSizeForVideo(int *width, int *height) const;
+
+    void setPreviewFrameRate(int fps);
+    int getPreviewFrameRate() const;
+    void getPreviewFpsRange(int *min_fps, int *max_fps) const;
+    void setPreviewFpsRange(int min_fps, int max_fps);
+    void setPreviewFormat(const char *format);
+    const char *getPreviewFormat() const;
+    void setPictureSize(int width, int height);
+    void getPictureSize(int *width, int *height) const;
+    void getSupportedPictureSizes(Vector<Size> &sizes) const;
+    void setPictureFormat(const char *format);
+    const char *getPictureFormat() const;
+
+    void dump() const;
+    status_t dump(int fd, const Vector<String16>& args) const;
+
+private:
+
+    // Quick and dirty map that maintains insertion order
+    template <typename KeyT, typename ValueT>
+    struct OrderedKeyedVector {
+
+        ssize_t add(const KeyT& key, const ValueT& value) {
+            return mList.add(Pair(key, value));
+        }
+
+        size_t size() const {
+            return mList.size();
+        }
+
+        const KeyT& keyAt(size_t idx) const {
+            return mList[idx].mKey;
+        }
+
+        const ValueT& valueAt(size_t idx) const {
+            return mList[idx].mValue;
+        }
+
+        const ValueT& valueFor(const KeyT& key) const {
+            ssize_t i = indexOfKey(key);
+            LOG_ALWAYS_FATAL_IF(i<0, "%s: key not found", __PRETTY_FUNCTION__);
+
+            return valueAt(i);
+        }
+
+        ssize_t indexOfKey(const KeyT& key) const {
+                size_t vectorIdx = 0;
+                for (; vectorIdx < mList.size(); ++vectorIdx) {
+                    if (mList[vectorIdx].mKey == key) {
+                        return (ssize_t) vectorIdx;
+                    }
+                }
+
+                return NAME_NOT_FOUND;
+        }
+
+        ssize_t removeItem(const KeyT& key) {
+            size_t vectorIdx = (size_t) indexOfKey(key);
+
+            if (vectorIdx < 0) {
+                return vectorIdx;
+            }
+
+            return mList.removeAt(vectorIdx);
+        }
+
+        void clear() {
+            mList.clear();
+        }
+
+        // Same as removing and re-adding. The key's index changes to max.
+        ssize_t replaceValueFor(const KeyT& key, const ValueT& value) {
+            removeItem(key);
+            return add(key, value);
+        }
+
+    private:
+
+        struct Pair {
+            Pair() : mKey(), mValue() {}
+            Pair(const KeyT& key, const ValueT& value) :
+                    mKey(key),
+                    mValue(value) {}
+            KeyT   mKey;
+            ValueT mValue;
+        };
+
+        Vector<Pair> mList;
+    };
+
+    /**
+     * Order matters: Keys that are set() later are stored later in the map.
+     *
+     * If two keys have meaning that conflict, then the later-set key
+     * wins.
+     *
+     * For example, preview FPS and preview FPS range conflict since only
+     * we only want to use the FPS range if that's the last thing that was set.
+     * So in that case, only use preview FPS range if it was set later than
+     * the preview FPS.
+     */
+    OrderedKeyedVector<String8,String8>    mMap;
+};
+
+}; // namespace android
+
+#endif
diff --git a/services/camera/libcameraservice/api1/client2/Parameters.cpp b/services/camera/libcameraservice/api1/client2/Parameters.cpp
index 5196e09..61e3588 100644
--- a/services/camera/libcameraservice/api1/client2/Parameters.cpp
+++ b/services/camera/libcameraservice/api1/client2/Parameters.cpp
@@ -16,7 +16,7 @@
 
 #define LOG_TAG "Camera2-Parameters"
 #define ATRACE_TAG ATRACE_TAG_CAMERA
-//#define LOG_NDEBUG 0
+// #define LOG_NDEBUG 0
 
 #include <utils/Log.h>
 #include <utils/Trace.h>
@@ -92,26 +92,6 @@
         staticInfo(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES, 2);
     if (!availableFpsRanges.count) return NO_INIT;
 
-    previewFpsRange[0] = availableFpsRanges.data.i32[0];
-    previewFpsRange[1] = availableFpsRanges.data.i32[1];
-
-    params.set(CameraParameters::KEY_PREVIEW_FPS_RANGE,
-            String8::format("%d,%d",
-                    previewFpsRange[0] * kFpsToApiScale,
-                    previewFpsRange[1] * kFpsToApiScale));
-
-    {
-        String8 supportedPreviewFpsRange;
-        for (size_t i=0; i < availableFpsRanges.count; i += 2) {
-            if (i != 0) supportedPreviewFpsRange += ",";
-            supportedPreviewFpsRange += String8::format("(%d,%d)",
-                    availableFpsRanges.data.i32[i] * kFpsToApiScale,
-                    availableFpsRanges.data.i32[i+1] * kFpsToApiScale);
-        }
-        params.set(CameraParameters::KEY_SUPPORTED_PREVIEW_FPS_RANGE,
-                supportedPreviewFpsRange);
-    }
-
     previewFormat = HAL_PIXEL_FORMAT_YCrCb_420_SP;
     params.set(CameraParameters::KEY_PREVIEW_FORMAT,
             formatEnumToString(previewFormat)); // NV21
@@ -179,6 +159,9 @@
                 supportedPreviewFormats);
     }
 
+    previewFpsRange[0] = availableFpsRanges.data.i32[0];
+    previewFpsRange[1] = availableFpsRanges.data.i32[1];
+
     // PREVIEW_FRAME_RATE / SUPPORTED_PREVIEW_FRAME_RATES are deprecated, but
     // still have to do something sane for them
 
@@ -187,6 +170,27 @@
     params.set(CameraParameters::KEY_PREVIEW_FRAME_RATE,
             previewFps);
 
+    // PREVIEW_FPS_RANGE
+    // -- Order matters. Set range after single value to so that a roundtrip
+    //    of setParameters(getParameters()) would keep the FPS range in higher
+    //    order.
+    params.set(CameraParameters::KEY_PREVIEW_FPS_RANGE,
+            String8::format("%d,%d",
+                    previewFpsRange[0] * kFpsToApiScale,
+                    previewFpsRange[1] * kFpsToApiScale));
+
+    {
+        String8 supportedPreviewFpsRange;
+        for (size_t i=0; i < availableFpsRanges.count; i += 2) {
+            if (i != 0) supportedPreviewFpsRange += ",";
+            supportedPreviewFpsRange += String8::format("(%d,%d)",
+                    availableFpsRanges.data.i32[i] * kFpsToApiScale,
+                    availableFpsRanges.data.i32[i+1] * kFpsToApiScale);
+        }
+        params.set(CameraParameters::KEY_SUPPORTED_PREVIEW_FPS_RANGE,
+                supportedPreviewFpsRange);
+    }
+
     {
         SortedVector<int32_t> sortedPreviewFrameRates;
 
@@ -1084,7 +1088,7 @@
 status_t Parameters::set(const String8& paramString) {
     status_t res;
 
-    CameraParameters newParams(paramString);
+    CameraParameters2 newParams(paramString);
 
     // TODO: Currently ignoring any changes to supposedly read-only parameters
     // such as supported preview sizes, etc. Should probably produce an error if
@@ -1127,29 +1131,73 @@
     // RECORDING_HINT (always supported)
     validatedParams.recordingHint = boolFromString(
         newParams.get(CameraParameters::KEY_RECORDING_HINT) );
-    bool recordingHintChanged = validatedParams.recordingHint != recordingHint;
-    ALOGV_IF(recordingHintChanged, "%s: Recording hint changed to %d",
-            __FUNCTION__, recordingHintChanged);
+    IF_ALOGV() { // Avoid unused variable warning
+        bool recordingHintChanged =
+                validatedParams.recordingHint != recordingHint;
+        if (recordingHintChanged) {
+            ALOGV("%s: Recording hint changed to %d",
+                  __FUNCTION__, validatedParams.recordingHint);
+        }
+    }
 
     // PREVIEW_FPS_RANGE
-    bool fpsRangeChanged = false;
-    int32_t lastSetFpsRange[2];
 
-    params.getPreviewFpsRange(&lastSetFpsRange[0], &lastSetFpsRange[1]);
-    lastSetFpsRange[0] /= kFpsToApiScale;
-    lastSetFpsRange[1] /= kFpsToApiScale;
+    /**
+     * Use the single FPS value if it was set later than the range.
+     * Otherwise, use the range value.
+     */
+    bool fpsUseSingleValue;
+    {
+        const char *fpsRange, *fpsSingle;
 
+        fpsRange = newParams.get(CameraParameters::KEY_PREVIEW_FRAME_RATE);
+        fpsSingle = newParams.get(CameraParameters::KEY_PREVIEW_FPS_RANGE);
+
+        /**
+         * Pick either the range or the single key if only one was set.
+         *
+         * If both are set, pick the one that has greater set order.
+         */
+        if (fpsRange == NULL && fpsSingle == NULL) {
+            ALOGE("%s: FPS was not set. One of %s or %s must be set.",
+                  __FUNCTION__, CameraParameters::KEY_PREVIEW_FRAME_RATE,
+                  CameraParameters::KEY_PREVIEW_FPS_RANGE);
+            return BAD_VALUE;
+        } else if (fpsRange == NULL) {
+            fpsUseSingleValue = true;
+            ALOGV("%s: FPS range not set, using FPS single value",
+                  __FUNCTION__);
+        } else if (fpsSingle == NULL) {
+            fpsUseSingleValue = false;
+            ALOGV("%s: FPS single not set, using FPS range value",
+                  __FUNCTION__);
+        } else {
+            int fpsKeyOrder;
+            res = newParams.compareSetOrder(
+                    CameraParameters::KEY_PREVIEW_FRAME_RATE,
+                    CameraParameters::KEY_PREVIEW_FPS_RANGE,
+                    &fpsKeyOrder);
+            LOG_ALWAYS_FATAL_IF(res != OK, "Impossibly bad FPS keys");
+
+            fpsUseSingleValue = (fpsKeyOrder > 0);
+
+        }
+
+        ALOGV("%s: Preview FPS value is used from '%s'",
+              __FUNCTION__, fpsUseSingleValue ? "single" : "range");
+    }
     newParams.getPreviewFpsRange(&validatedParams.previewFpsRange[0],
             &validatedParams.previewFpsRange[1]);
+
     validatedParams.previewFpsRange[0] /= kFpsToApiScale;
     validatedParams.previewFpsRange[1] /= kFpsToApiScale;
 
-    // Compare the FPS range value from the last set() to the current set()
-    // to determine if the client has changed it
-    if (validatedParams.previewFpsRange[0] != lastSetFpsRange[0] ||
-            validatedParams.previewFpsRange[1] != lastSetFpsRange[1]) {
+    // Ignore the FPS range if the FPS single has higher precedence
+    if (!fpsUseSingleValue) {
+        ALOGV("%s: Preview FPS range (%d, %d)", __FUNCTION__,
+                validatedParams.previewFpsRange[0],
+                validatedParams.previewFpsRange[1]);
 
-        fpsRangeChanged = true;
         camera_metadata_ro_entry_t availablePreviewFpsRanges =
             staticInfo(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES, 2);
         for (i = 0; i < availablePreviewFpsRanges.count; i += 2) {
@@ -1200,14 +1248,13 @@
         }
     }
 
-    // PREVIEW_FRAME_RATE Deprecated, only use if the preview fps range is
-    // unchanged this time.  The single-value FPS is the same as the minimum of
-    // the range.  To detect whether the application has changed the value of
-    // previewFps, compare against their last-set preview FPS.
-    if (!fpsRangeChanged) {
+    // PREVIEW_FRAME_RATE Deprecated
+    // - Use only if the single FPS value was set later than the FPS range
+    if (fpsUseSingleValue) {
         int previewFps = newParams.getPreviewFrameRate();
-        int lastSetPreviewFps = params.getPreviewFrameRate();
-        if (previewFps != lastSetPreviewFps || recordingHintChanged) {
+        ALOGV("%s: Preview FPS single value requested: %d",
+              __FUNCTION__, previewFps);
+        {
             camera_metadata_ro_entry_t availableFrameRates =
                 staticInfo(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES);
             /**
@@ -1276,6 +1323,35 @@
         }
     }
 
+    /**
+     * Update Preview FPS and Preview FPS ranges based on
+     * what we actually set.
+     *
+     * This updates the API-visible (Camera.Parameters#getParameters) values of
+     * the FPS fields, not only the internal versions.
+     *
+     * Order matters: The value that was set last takes precedence.
+     * - If the client does a setParameters(getParameters()) we retain
+     *   the same order for preview FPS.
+     */
+    if (!fpsUseSingleValue) {
+        // Set fps single, then fps range (range wins)
+        newParams.setPreviewFrameRate(
+                fpsFromRange(/*min*/validatedParams.previewFpsRange[0],
+                             /*max*/validatedParams.previewFpsRange[1]));
+        newParams.setPreviewFpsRange(
+                validatedParams.previewFpsRange[0] * kFpsToApiScale,
+                validatedParams.previewFpsRange[1] * kFpsToApiScale);
+    } else {
+        // Set fps range, then fps single (single wins)
+        newParams.setPreviewFpsRange(
+                validatedParams.previewFpsRange[0] * kFpsToApiScale,
+                validatedParams.previewFpsRange[1] * kFpsToApiScale);
+        // Set this to the same value, but with higher priority
+        newParams.setPreviewFrameRate(
+                newParams.getPreviewFrameRate());
+    }
+
     // PICTURE_SIZE
     newParams.getPictureSize(&validatedParams.pictureWidth,
             &validatedParams.pictureHeight);
diff --git a/services/camera/libcameraservice/api1/client2/Parameters.h b/services/camera/libcameraservice/api1/client2/Parameters.h
index 32dbd42..da07ccf 100644
--- a/services/camera/libcameraservice/api1/client2/Parameters.h
+++ b/services/camera/libcameraservice/api1/client2/Parameters.h
@@ -25,6 +25,7 @@
 #include <utils/Vector.h>
 #include <utils/KeyedVector.h>
 #include <camera/CameraParameters.h>
+#include <camera/CameraParameters2.h>
 #include <camera/CameraMetadata.h>
 
 namespace android {
@@ -32,7 +33,7 @@
 
 /**
  * Current camera state; this is the full state of the Camera under the old
- * camera API (contents of the CameraParameters object in a more-efficient
+ * camera API (contents of the CameraParameters2 object in a more-efficient
  * format, plus other state). The enum values are mostly based off the
  * corresponding camera2 enums, not the camera1 strings. A few are defined here
  * if they don't cleanly map to camera2 values.
@@ -128,7 +129,7 @@
         LIGHTFX_HDR
     } lightFx;
 
-    CameraParameters params;
+    CameraParameters2 params;
     String8 paramsFlattened;
 
     // These parameters are also part of the camera API-visible state, but not