Revert "camera: Fix setParameters for Preview FPS single/range values"

Causes a regression on some devices, so reverting until we're in a position to fix those devices.

This reverts commit 9078a1b3b9f9c0c48046ade0e8e18b0d79a659db.

Bug: 13563098
Change-Id: I7aedd01fde8b8fdee77e972ec395f0ecadbf8ccb

3 files changed, 47 insertions, 267 deletions

diff --git a/camera/CameraParameters.cpp b/camera/CameraParameters.cpp
index 99e5df4..af091f4 100644
--- a/camera/CameraParameters.cpp
+++ b/camera/CameraParameters.cpp
@@ -16,7 +16,6 @@
 */
 
 #define LOG_TAG "CameraParams"
-// #define LOG_NDEBUG 0
 #include <utils/Log.h>
 
 #include <string.h>
@@ -199,8 +198,6 @@ String8 CameraParameters::flatten() const
             flattened += ";";
     }
 
-    ALOGV("%s: Flattened params = %s", __FUNCTION__, flattened.string());
-
     return flattened;
 }
 
@@ -250,9 +247,7 @@ void CameraParameters::set(const char *key, const char *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);
+    mMap.replaceValueFor(String8(key), String8(value));
 }
 
 void CameraParameters::set(const char *key, int value)
@@ -271,12 +266,10 @@ void CameraParameters::setFloat(const char *key, float value)
 
 const char *CameraParameters::get(const char *key) const
 {
-    ssize_t idx = mMap.indexOfKey(String8(key));
-    if (idx < 0) {
-        return NULL;
-    } else {
-        return mMap.valueAt(idx).string();
-    }
+    String8 v = mMap.valueFor(String8(key));
+    if (v.length() == 0)
+        return 0;
+    return v.string();
 }
 
 int CameraParameters::getInt(const char *key) const
@@ -294,36 +287,6 @@ float CameraParameters::getFloat(const char *key) const
     return strtof(v, 0);
 }
 
-status_t CameraParameters::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 CameraParameters::remove(const char *key)
 {
     mMap.removeItem(String8(key));
@@ -449,12 +412,6 @@ void CameraParameters::getPreviewFpsRange(int *min_fps, int *max_fps) const
     parse_pair(p, min_fps, max_fps, ',');
 }
 
-void CameraParameters::setPreviewFpsRange(int min_fps, int max_fps)
-{
-    String8 str = String8::format("%d,%d", min_fps, max_fps);
-    set(KEY_PREVIEW_FPS_RANGE, str.string());
-}
-
 void CameraParameters::setPreviewFormat(const char *format)
 {
     set(KEY_PREVIEW_FORMAT, format);
diff --git a/include/camera/CameraParameters.h b/include/camera/CameraParameters.h
index 833ba76..d521543 100644
--- a/include/camera/CameraParameters.h
+++ b/include/camera/CameraParameters.h
@@ -18,7 +18,6 @@
 #define ANDROID_HARDWARE_CAMERA_PARAMETERS_H
 
 #include <utils/KeyedVector.h>
-#include <utils/Vector.h>
 #include <utils/String8.h>
 
 namespace android {
@@ -51,26 +50,10 @@ public:
     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);
@@ -108,7 +91,6 @@ public:
     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);
@@ -693,91 +675,7 @@ public:
     static const char LIGHTFX_HDR[];
 
 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;
+    DefaultKeyedVector<String8,String8>    mMap;
 };
 
 }; // namespace android
diff --git a/services/camera/libcameraservice/api1/client2/Parameters.cpp b/services/camera/libcameraservice/api1/client2/Parameters.cpp
index d5be58c..5bfb969 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,6 +92,26 @@ status_t Parameters::initialize(const CameraMetadata *info) {
         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
@@ -159,9 +179,6 @@ status_t Parameters::initialize(const CameraMetadata *info) {
                 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
 
@@ -170,27 +187,6 @@ status_t Parameters::initialize(const CameraMetadata *info) {
     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;
 
@@ -1131,72 +1127,29 @@ status_t Parameters::set(const String8& paramString) {
     // RECORDING_HINT (always supported)
     validatedParams.recordingHint = boolFromString(
         newParams.get(CameraParameters::KEY_RECORDING_HINT) );
-    IF_ALOGV() { // Avoid unused variable warning
-        bool recordingHintChanged =
-                validatedParams.recordingHint != recordingHint;
-        if (recordingHintChanged) {
-            ALOGV("%s: Recording hint changed to %d",
-                  __FUNCTION__, validatedParams.recordingHint);
-        }
-    }
+    bool recordingHintChanged = validatedParams.recordingHint != recordingHint;
+    ALOGV_IF(recordingHintChanged, "%s: Recording hint changed to %d",
+            __FUNCTION__, recordingHintChanged);
 
     // PREVIEW_FPS_RANGE
+    bool fpsRangeChanged = false;
+    int32_t lastSetFpsRange[2];
 
-    /**
-     * 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 = params.get(CameraParameters::KEY_PREVIEW_FRAME_RATE);
-        fpsSingle = params.get(CameraParameters::KEY_PREVIEW_FPS_RANGE);
+    params.getPreviewFpsRange(&lastSetFpsRange[0], &lastSetFpsRange[1]);
+    lastSetFpsRange[0] /= kFpsToApiScale;
+    lastSetFpsRange[1] /= kFpsToApiScale;
 
-        /**
-         * 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 = params.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;
 
-    // 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]);
+    // 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]) {
 
+        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) {
@@ -1247,13 +1200,14 @@ status_t Parameters::set(const String8& paramString) {
         }
     }
 
-    // PREVIEW_FRAME_RATE Deprecated
-    // - Use only if the single FPS value was set later than the FPS range
-    if (fpsUseSingleValue) {
+    // 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) {
         int previewFps = newParams.getPreviewFrameRate();
-        ALOGV("%s: Preview FPS single value requested: %d",
-              __FUNCTION__, previewFps);
-        {
+        int lastSetPreviewFps = params.getPreviewFrameRate();
+        if (previewFps != lastSetPreviewFps || recordingHintChanged) {
             camera_metadata_ro_entry_t availableFrameRates =
                 staticInfo(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES);
             /**
@@ -1322,35 +1276,6 @@ status_t Parameters::set(const String8& paramString) {
         }
     }
 
-    /**
-     * 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)
-        validatedParams.params.setPreviewFrameRate(
-                fpsFromRange(/*min*/validatedParams.previewFpsRange[0],
-                             /*max*/validatedParams.previewFpsRange[1]));
-        validatedParams.params.setPreviewFpsRange(
-                validatedParams.previewFpsRange[0],
-                validatedParams.previewFpsRange[1]);
-    } else {
-        // Set fps range, then fps single (single wins)
-        validatedParams.params.setPreviewFpsRange(
-                validatedParams.previewFpsRange[0],
-                validatedParams.previewFpsRange[1]);
-        validatedParams.params.setPreviewFrameRate(
-                fpsFromRange(/*min*/validatedParams.previewFpsRange[0],
-                             /*max*/validatedParams.previewFpsRange[1]));
-    }
-
     // PICTURE_SIZE
     newParams.getPictureSize(&validatedParams.pictureWidth,
             &validatedParams.pictureHeight);