diff options
author | Igor Murashkin <iam@google.com> | 2014-03-18 18:15:23 -0700 |
---|---|---|
committer | Igor Murashkin <iam@google.com> | 2014-03-20 00:21:25 +0000 |
commit | 9078a1b3b9f9c0c48046ade0e8e18b0d79a659db (patch) | |
tree | b621ec3b2db6b4dcb12df7d201eb474a09183386 | |
parent | b4a7a2df4c28c3f32b5d877b54831d2cc5d78f81 (diff) | |
download | frameworks_av-9078a1b3b9f9c0c48046ade0e8e18b0d79a659db.zip frameworks_av-9078a1b3b9f9c0c48046ade0e8e18b0d79a659db.tar.gz frameworks_av-9078a1b3b9f9c0c48046ade0e8e18b0d79a659db.tar.bz2 |
camera: Fix setParameters for Preview FPS single/range values
Bug: 12609188
Change-Id: I82ea6f5de2183dd046d4bf5683600c97f37ab4db
-rw-r--r-- | camera/CameraParameters.cpp | 53 | ||||
-rw-r--r-- | include/camera/CameraParameters.h | 104 | ||||
-rw-r--r-- | services/camera/libcameraservice/api1/client2/Parameters.cpp | 157 |
3 files changed, 267 insertions, 47 deletions
diff --git a/camera/CameraParameters.cpp b/camera/CameraParameters.cpp index af091f4..99e5df4 100644 --- a/camera/CameraParameters.cpp +++ b/camera/CameraParameters.cpp @@ -16,6 +16,7 @@ */ #define LOG_TAG "CameraParams" +// #define LOG_NDEBUG 0 #include <utils/Log.h> #include <string.h> @@ -198,6 +199,8 @@ String8 CameraParameters::flatten() const flattened += ";"; } + ALOGV("%s: Flattened params = %s", __FUNCTION__, flattened.string()); + return flattened; } @@ -247,7 +250,9 @@ void CameraParameters::set(const char *key, const char *value) return; } - mMap.replaceValueFor(String8(key), String8(value)); + // 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 CameraParameters::set(const char *key, int value) @@ -266,10 +271,12 @@ void CameraParameters::setFloat(const char *key, float value) const char *CameraParameters::get(const char *key) const { - String8 v = mMap.valueFor(String8(key)); - if (v.length() == 0) - return 0; - return v.string(); + ssize_t idx = mMap.indexOfKey(String8(key)); + if (idx < 0) { + return NULL; + } else { + return mMap.valueAt(idx).string(); + } } int CameraParameters::getInt(const char *key) const @@ -287,6 +294,36 @@ 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)); @@ -412,6 +449,12 @@ 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 d521543..833ba76 100644 --- a/include/camera/CameraParameters.h +++ b/include/camera/CameraParameters.h @@ -18,6 +18,7 @@ #define ANDROID_HARDWARE_CAMERA_PARAMETERS_H #include <utils/KeyedVector.h> +#include <utils/Vector.h> #include <utils/String8.h> namespace android { @@ -50,10 +51,26 @@ 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); @@ -91,6 +108,7 @@ 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); @@ -675,7 +693,91 @@ public: static const char LIGHTFX_HDR[]; private: - DefaultKeyedVector<String8,String8> mMap; + + // 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 diff --git a/services/camera/libcameraservice/api1/client2/Parameters.cpp b/services/camera/libcameraservice/api1/client2/Parameters.cpp index 2cf0d29..75ec426 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 @@ 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 @@ -179,6 +159,9 @@ 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 @@ -187,6 +170,27 @@ 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; @@ -1127,29 +1131,72 @@ status_t Parameters::set(const String8& paramString) { // 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 = params.get(CameraParameters::KEY_PREVIEW_FRAME_RATE); + fpsSingle = params.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 = 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; - // 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 +1247,13 @@ 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. - 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 +1322,35 @@ 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); |