Camera2: Rework the FPS range vs. FPS single setting detection

Give up on current approach of writing out consistent FPS values
into parameters that will be read back by the app.

- Preserve app's latest set parameters exactly, and compare against
  them when detecting if a new FPS range or single FPS value has been
  selected.

- Since get() returns exactly what was set(), it doesn't matter if the
  app calls getParameters() before its next setParameters(), in terms
  of retriggering FPS selection logic. Before, the behavior varied
  depending on whether the app re-read the parameters.

- As before, if app changes both range and single FPS in a single set
  call, the range set wins. Otherwise, the value that has changed more
  recently is used.

Bug: 11570973
Change-Id: I72b5e60c3f60e88d55127dd1bda87e26eaf929c6

2 files changed, 20 insertions, 33 deletions

diff --git a/services/camera/libcameraservice/api1/client2/Parameters.cpp b/services/camera/libcameraservice/api1/client2/Parameters.cpp
index 1e425ba..08af566 100644
--- a/services/camera/libcameraservice/api1/client2/Parameters.cpp
+++ b/services/camera/libcameraservice/api1/client2/Parameters.cpp
@@ -183,8 +183,7 @@ status_t Parameters::initialize(const CameraMetadata *info) {
     // still have to do something sane for them
 
     // NOTE: Not scaled like FPS range values are.
-    previewFps = fpsFromRange(previewFpsRange[0], previewFpsRange[1]);
-    lastSetPreviewFps = previewFps;
+    int previewFps = fpsFromRange(previewFpsRange[0], previewFpsRange[1]);
     params.set(CameraParameters::KEY_PREVIEW_FRAME_RATE,
             previewFps);
 
@@ -1134,13 +1133,22 @@ status_t Parameters::set(const String8& paramString) {
 
     // PREVIEW_FPS_RANGE
     bool fpsRangeChanged = false;
+    int32_t lastSetFpsRange[2];
+
+    params.getPreviewFpsRange(&lastSetFpsRange[0], &lastSetFpsRange[1]);
+    lastSetFpsRange[0] /= kFpsToApiScale;
+    lastSetFpsRange[1] /= kFpsToApiScale;
+
     newParams.getPreviewFpsRange(&validatedParams.previewFpsRange[0],
             &validatedParams.previewFpsRange[1]);
     validatedParams.previewFpsRange[0] /= kFpsToApiScale;
     validatedParams.previewFpsRange[1] /= kFpsToApiScale;
 
-    if (validatedParams.previewFpsRange[0] != previewFpsRange[0] ||
-            validatedParams.previewFpsRange[1] != 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);
@@ -1158,16 +1166,6 @@ status_t Parameters::set(const String8& paramString) {
                     validatedParams.previewFpsRange[1]);
             return BAD_VALUE;
         }
-        validatedParams.previewFps =
-            fpsFromRange(validatedParams.previewFpsRange[0],
-                         validatedParams.previewFpsRange[1]);
-
-        // Update our last-seen single preview FPS, needed for disambiguating
-        // when the application is intending to use the deprecated single-FPS
-        // setting vs. the range FPS setting
-        validatedParams.lastSetPreviewFps = newParams.getPreviewFrameRate();
-
-        newParams.setPreviewFrameRate(validatedParams.previewFps);
     }
 
     // PREVIEW_FORMAT
@@ -1205,12 +1203,11 @@ status_t Parameters::set(const String8& paramString) {
     // 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 instead of the
-    // single FPS we may have synthesized from a range FPS set.
+    // previewFps, compare against their last-set preview FPS.
     if (!fpsRangeChanged) {
-        validatedParams.previewFps = newParams.getPreviewFrameRate();
-        if (validatedParams.previewFps != lastSetPreviewFps ||
-                recordingHintChanged) {
+        int previewFps = newParams.getPreviewFrameRate();
+        int lastSetPreviewFps = params.getPreviewFrameRate();
+        if (previewFps != lastSetPreviewFps || recordingHintChanged) {
             camera_metadata_ro_entry_t availableFrameRates =
                 staticInfo(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES);
             /**
@@ -1223,8 +1220,8 @@ status_t Parameters::set(const String8& paramString) {
               * Either way, in case of multiple ranges, break the tie by
               * selecting the smaller range.
               */
-            int targetFps = validatedParams.previewFps;
-            // all ranges which have targetFps
+
+            // all ranges which have previewFps
             Vector<Range> candidateRanges;
             for (i = 0; i < availableFrameRates.count; i+=2) {
                 Range r = {
@@ -1232,13 +1229,13 @@ status_t Parameters::set(const String8& paramString) {
                             availableFrameRates.data.i32[i+1]
                 };
 
-                if (r.min <= targetFps && targetFps <= r.max) {
+                if (r.min <= previewFps && previewFps <= r.max) {
                     candidateRanges.push(r);
                 }
             }
             if (candidateRanges.isEmpty()) {
                 ALOGE("%s: Requested preview frame rate %d is not supported",
-                        __FUNCTION__, validatedParams.previewFps);
+                        __FUNCTION__, previewFps);
                 return BAD_VALUE;
             }
             // most applicable range with targetFps
@@ -1277,14 +1274,6 @@ status_t Parameters::set(const String8& paramString) {
                 validatedParams.previewFpsRange[1],
                 validatedParams.recordingHint);
         }
-        newParams.set(CameraParameters::KEY_PREVIEW_FPS_RANGE,
-                String8::format("%d,%d",
-                        validatedParams.previewFpsRange[0] * kFpsToApiScale,
-                        validatedParams.previewFpsRange[1] * kFpsToApiScale));
-        // Update our last-seen single preview FPS, needed for disambiguating
-        // when the application is intending to use the deprecated single-FPS
-        // setting vs. the range FPS setting
-        validatedParams.lastSetPreviewFps = validatedParams.previewFps;
     }
 
     // PICTURE_SIZE
diff --git a/services/camera/libcameraservice/api1/client2/Parameters.h b/services/camera/libcameraservice/api1/client2/Parameters.h
index 93ab113..32dbd42 100644
--- a/services/camera/libcameraservice/api1/client2/Parameters.h
+++ b/services/camera/libcameraservice/api1/client2/Parameters.h
@@ -46,8 +46,6 @@ struct Parameters {
 
     int previewWidth, previewHeight;
     int32_t previewFpsRange[2];
-    int lastSetPreviewFps; // the last single FPS value seen in a set call
-    int previewFps; // deprecated, here only for tracking changes
     int previewFormat;
 
     int previewTransform; // set by CAMERA_CMD_SET_DISPLAY_ORIENTATION