modules: camera: Metadata refactor

Add* functions const to protect data being added.
Initiailze metadata object from a camera_metadata_t.
Remove template-specific constructor.
Add convenience "add1" method to abvoid stack-allocated data.
Construct templates manually (in ExampleCamera) from base.

Change-Id: I3201f61e9d32841ea7a769d69242b40d8727c02e

6 files changed, 197 insertions, 203 deletions

diff --git a/modules/camera/Camera.cpp b/modules/camera/Camera.cpp
index c7ca64a..a3164e5 100644
--- a/modules/camera/Camera.cpp
+++ b/modules/camera/Camera.cpp
@@ -509,20 +509,47 @@ void Camera::dump(int fd)
     pthread_mutex_unlock(&mMutex);
 }
 
-void Camera::setTemplate(int type, camera_metadata_t *settings)
+const char* Camera::templateToString(int type)
+{
+    switch (type) {
+    case CAMERA3_TEMPLATE_PREVIEW:
+        return "CAMERA3_TEMPLATE_PREVIEW";
+    case CAMERA3_TEMPLATE_STILL_CAPTURE:
+        return "CAMERA3_TEMPLATE_STILL_CAPTURE";
+    case CAMERA3_TEMPLATE_VIDEO_RECORD:
+        return "CAMERA3_TEMPLATE_VIDEO_RECORD";
+    case CAMERA3_TEMPLATE_VIDEO_SNAPSHOT:
+        return "CAMERA3_TEMPLATE_VIDEO_SNAPSHOT";
+    case CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG:
+        return "CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG";
+    }
+    // TODO: support vendor templates
+    return "Invalid template type!";
+}
+
+int Camera::setTemplate(int type, camera_metadata_t *settings)
 {
     if (!isValidTemplateType(type)) {
         ALOGE("%s:%d: Invalid template request type: %d", __func__, mId, type);
-        return;
+        return -EINVAL;
     }
     pthread_mutex_lock(&mMutex);
     if (mTemplates[type] != NULL) {
-        ALOGE("%s:%d: Setting already constructed template type %d: %p to %p!",
-                __func__, mId, type, mStaticInfo, settings);
-        free_camera_metadata(mTemplates[type]);
+        ALOGE("%s:%d: Setting already constructed template type %s(%d)",
+                __func__, mId, templateToString(type), type);
+        pthread_mutex_unlock(&mMutex);
+        return -EINVAL;
     }
+    // Make a durable copy of the underlying metadata
     mTemplates[type] = clone_camera_metadata(settings);
+    if (mTemplates[type] == NULL) {
+        ALOGE("%s:%d: Failed to clone metadata %p for template type %s(%d)",
+                __func__, mId, settings, templateToString(type), type);
+        pthread_mutex_unlock(&mMutex);
+        return -EINVAL;
+    }
     pthread_mutex_unlock(&mMutex);
+    return 0;
 }
 
 extern "C" {
diff --git a/modules/camera/Camera.h b/modules/camera/Camera.h
index 53672ae..4c869bb 100644
--- a/modules/camera/Camera.h
+++ b/modules/camera/Camera.h
@@ -59,8 +59,10 @@ class Camera {
         virtual bool isValidCaptureSettings(const camera_metadata_t *) = 0;
         // Separate initialization method for individual devices when opened
         virtual int initDevice() = 0;
-        // Accessor used by initDevice()
-        void setTemplate(int type, camera_metadata_t *static_info);
+        // Accessor used by initDevice() to set the templates' metadata
+        int setTemplate(int type, camera_metadata_t *static_info);
+        // Prettyprint template names
+        const char* templateToString(int type);
 
     private:
         // Camera device handle returned to framework for use
diff --git a/modules/camera/ExampleCamera.cpp b/modules/camera/ExampleCamera.cpp
index 8cf2ef0..16e1e02 100644
--- a/modules/camera/ExampleCamera.cpp
+++ b/modules/camera/ExampleCamera.cpp
@@ -174,31 +174,84 @@ camera_metadata_t *ExampleCamera::initStaticInfo()
 
     /* End of static camera characteristics */
 
-    return clone_camera_metadata(m.generate());
+    return clone_camera_metadata(m.get());
 }
 
 int ExampleCamera::initDevice()
 {
-    // Create standard settings templates
-    Metadata preview = Metadata(ANDROID_CONTROL_MODE_OFF,
-            ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW);
-    setTemplate(CAMERA3_TEMPLATE_PREVIEW, preview.generate());
-    Metadata capture = Metadata(ANDROID_CONTROL_MODE_OFF,
-            ANDROID_CONTROL_CAPTURE_INTENT_STILL_CAPTURE);
-    setTemplate(CAMERA3_TEMPLATE_STILL_CAPTURE, capture.generate());
-    Metadata record = Metadata(ANDROID_CONTROL_MODE_OFF,
-            ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_RECORD);
-    setTemplate(CAMERA3_TEMPLATE_VIDEO_RECORD, record.generate());
-    Metadata snapshot = Metadata(ANDROID_CONTROL_MODE_OFF,
-            ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_SNAPSHOT);
-    setTemplate(CAMERA3_TEMPLATE_VIDEO_SNAPSHOT, snapshot.generate());
-    Metadata zsl = Metadata(ANDROID_CONTROL_MODE_OFF,
-            ANDROID_CONTROL_CAPTURE_INTENT_ZERO_SHUTTER_LAG);
-    setTemplate(CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG, zsl.generate());
+    int res;
+    Metadata base;
+
+    // Create standard settings templates from copies of base metadata
+    // TODO: use vendor tags in base metadata
+    if (res = base.add1UInt8(ANDROID_CONTROL_MODE, ANDROID_CONTROL_MODE_OFF))
+        return res;
+
+    // Use base settings to create all other templates and set them
+    if (res = setPreviewTemplate(base)) return res;
+    if (res = setStillTemplate(base)) return res;
+    if (res = setRecordTemplate(base)) return res;
+    if (res = setSnapshotTemplate(base)) return res;
+    if (res = setZslTemplate(base)) return res;
 
     return 0;
 }
 
+int ExampleCamera::setPreviewTemplate(Metadata m)
+{
+    int res;
+    // Setup default preview controls
+    if (res = m.add1UInt8(ANDROID_CONTROL_CAPTURE_INTENT,
+            ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW))
+        return res;
+    // TODO: set fast auto-focus, auto-whitebalance, auto-exposure, auto flash
+    return setTemplate(CAMERA3_TEMPLATE_PREVIEW, m.get());
+}
+
+int ExampleCamera::setStillTemplate(Metadata m)
+{
+    int res;
+    // Setup default still capture controls
+    if (res = m.add1UInt8(ANDROID_CONTROL_CAPTURE_INTENT,
+            ANDROID_CONTROL_CAPTURE_INTENT_STILL_CAPTURE))
+        return res;
+    // TODO: set fast auto-focus, auto-whitebalance, auto-exposure, auto flash
+    return setTemplate(CAMERA3_TEMPLATE_STILL_CAPTURE, m.get());
+}
+
+int ExampleCamera::setRecordTemplate(Metadata m)
+{
+    int res;
+    // Setup default video record controls
+    if (res = m.add1UInt8(ANDROID_CONTROL_CAPTURE_INTENT,
+            ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_RECORD))
+        return res;
+    // TODO: set slow auto-focus, auto-whitebalance, auto-exposure, flash off
+    return setTemplate(CAMERA3_TEMPLATE_VIDEO_RECORD, m.get());
+}
+
+int ExampleCamera::setSnapshotTemplate(Metadata m)
+{
+    int res;
+    // Setup default video snapshot controls
+    if (res = m.add1UInt8(ANDROID_CONTROL_CAPTURE_INTENT,
+            ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_SNAPSHOT))
+        return res;
+    // TODO: set slow auto-focus, auto-whitebalance, auto-exposure, flash off 
+    return setTemplate(CAMERA3_TEMPLATE_VIDEO_SNAPSHOT, m.get());
+}
+
+int ExampleCamera::setZslTemplate(Metadata m)
+{
+    int res;
+    // Setup default zero shutter lag controls
+    if (res = m.add1UInt8(ANDROID_CONTROL_CAPTURE_INTENT,
+            ANDROID_CONTROL_CAPTURE_INTENT_ZERO_SHUTTER_LAG))
+        return res;
+    // TODO: set reprocessing parameters for zsl input queue
+    return setTemplate(CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG, m.get());
+}
+
 bool ExampleCamera::isValidCaptureSettings(const camera_metadata_t* settings)
 {
     // TODO: reject settings that cannot be captured
diff --git a/modules/camera/ExampleCamera.h b/modules/camera/ExampleCamera.h
index 9d6a9c0..45c4a94 100644
--- a/modules/camera/ExampleCamera.h
+++ b/modules/camera/ExampleCamera.h
@@ -35,6 +35,12 @@ class ExampleCamera : public Camera {
         camera_metadata_t *initStaticInfo();
         // Initialize whole device (templates/etc) when opened
         int initDevice();
+        // Initialize each template metadata controls
+        int setPreviewTemplate(Metadata m);
+        int setStillTemplate(Metadata m);
+        int setRecordTemplate(Metadata m);
+        int setSnapshotTemplate(Metadata m);
+        int setZslTemplate(Metadata m);
         // Verify settings are valid for a capture with this device
         bool isValidCaptureSettings(const camera_metadata_t* settings);
 };
diff --git a/modules/camera/Metadata.cpp b/modules/camera/Metadata.cpp
index ba7ed9c..72314d5 100644
--- a/modules/camera/Metadata.cpp
+++ b/modules/camera/Metadata.cpp
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-#include <pthread.h>
 #include <system/camera_metadata.h>
 
 //#define LOG_NDEBUG 0
@@ -28,95 +27,79 @@
 
 namespace default_camera_hal {
 
-Metadata::Metadata()
-  : mHead(NULL),
-    mTail(NULL),
-    mEntryCount(0),
-    mDataCount(0),
-    mGenerated(NULL),
-    mDirty(true)
+Metadata::Metadata():
+    mData(NULL)
 {
-    // NULL (default) pthread mutex attributes
-    pthread_mutex_init(&mMutex, NULL);
 }
 
 Metadata::~Metadata()
 {
-    Entry *current = mHead;
+    replace(NULL);
+}
 
-    while (current != NULL) {
-        Entry *tmp = current;
-        current = current->mNext;
-        delete tmp;
+void Metadata::replace(camera_metadata_t *m)
+{
+    if (m == mData) {
+        ALOGE("%s: Replacing metadata with itself?!", __func__);
+        return;
     }
-
-    if (mGenerated != NULL)
-        free_camera_metadata(mGenerated);
-
-    pthread_mutex_destroy(&mMutex);
+    if (mData)
+        free_camera_metadata(mData);
+    mData = m;
 }
 
-Metadata::Metadata(uint8_t mode, uint8_t intent)
-  : mHead(NULL),
-    mTail(NULL),
-    mEntryCount(0),
-    mDataCount(0),
-    mGenerated(NULL),
-    mDirty(true)
+int Metadata::init(const camera_metadata_t *metadata)
 {
-    pthread_mutex_init(&mMutex, NULL);
-
-    if (validate(ANDROID_CONTROL_MODE, TYPE_BYTE, 1)) {
-        int res = add(ANDROID_CONTROL_MODE, 1, &mode);
-        if (res != 0) {
-            ALOGE("%s: Unable to add mode to template!", __func__);
-        }
-    } else {
-        ALOGE("%s: Invalid mode constructing template!", __func__);
-    }
+    camera_metadata_t* tmp;
 
-    if (validate(ANDROID_CONTROL_CAPTURE_INTENT, TYPE_BYTE, 1)) {
-        int res = add(ANDROID_CONTROL_CAPTURE_INTENT, 1, &intent);
-        if (res != 0) {
-            ALOGE("%s: Unable to add capture intent to template!", __func__);
-        }
-    } else {
-        ALOGE("%s: Invalid capture intent constructing template!", __func__);
-    }
+    if (!validate_camera_metadata_structure(metadata, NULL))
+        return -EINVAL;
+
+    tmp = clone_camera_metadata(metadata);
+    if (tmp == NULL)
+        return -EINVAL;
+
+    replace(tmp);
+    return 0;
 }
 
-int Metadata::addUInt8(uint32_t tag, int count, uint8_t *data)
+int Metadata::addUInt8(uint32_t tag, int count, const uint8_t *data)
 {
     if (!validate(tag, TYPE_BYTE, count)) return -EINVAL;
     return add(tag, count, data);
 }
 
-int Metadata::addInt32(uint32_t tag, int count, int32_t *data)
+int Metadata::add1UInt8(uint32_t tag, const uint8_t data)
+{
+    return addUInt8(tag, 1, &data);
+}
+
+int Metadata::addInt32(uint32_t tag, int count, const int32_t *data)
 {
     if (!validate(tag, TYPE_INT32, count)) return -EINVAL;
     return add(tag, count, data);
 }
 
-int Metadata::addFloat(uint32_t tag, int count, float *data)
+int Metadata::addFloat(uint32_t tag, int count, const float *data)
 {
     if (!validate(tag, TYPE_FLOAT, count)) return -EINVAL;
     return add(tag, count, data);
 }
 
-int Metadata::addInt64(uint32_t tag, int count, int64_t *data)
+int Metadata::addInt64(uint32_t tag, int count, const int64_t *data)
 {
     if (!validate(tag, TYPE_INT64, count)) return -EINVAL;
     return add(tag, count, data);
 }
 
-int Metadata::addDouble(uint32_t tag, int count, double *data)
+int Metadata::addDouble(uint32_t tag, int count, const double *data)
 {
     if (!validate(tag, TYPE_DOUBLE, count)) return -EINVAL;
     return add(tag, count, data);
 }
 
 int Metadata::addRational(uint32_t tag, int count,
-        camera_metadata_rational_t *data)
+        const camera_metadata_rational_t *data)
 {
     if (!validate(tag, TYPE_RATIONAL, count)) return -EINVAL;
     return add(tag, count, data);
@@ -144,102 +127,46 @@ bool Metadata::validate(uint32_t tag, int tag_type, int count)
     return true;
 }
 
-int Metadata::add(uint32_t tag, int count, void *tag_data)
+int Metadata::add(uint32_t tag, int count, const void *tag_data)
 {
+    int res;
+    camera_metadata_t* tmp;
     int tag_type = get_camera_metadata_tag_type(tag);
-    size_t type_sz = camera_metadata_type_size[tag_type];
-
-    // Allocate array to hold new metadata
-    void *data = malloc(count * type_sz);
-    if (data == NULL)
+    size_t size = calculate_camera_metadata_entry_data_size(tag_type, count);
+    size_t entry_capacity = get_camera_metadata_entry_count(mData) + 1;
+    size_t data_capacity = get_camera_metadata_data_count(mData) + size;
+
+    // Opportunistically attempt to add if metadata has room for it
+    if (!add_camera_metadata_entry(mData, tag, tag_data, count))
+        return 0;
+
+    // Double new dimensions to minimize future reallocations
+    tmp = allocate_camera_metadata(entry_capacity * 2, data_capacity * 2);
+    if (tmp == NULL) {
+        ALOGE("%s: Failed to allocate new metadata with %d entries, %d data",
+                __func__, entry_capacity, data_capacity);
         return -ENOMEM;
-    memcpy(data, tag_data, count * type_sz);
-
-    pthread_mutex_lock(&mMutex);
-    mEntryCount++;
-    mDataCount += calculate_camera_metadata_entry_data_size(tag_type, count);
-    push(new Entry(tag, data, count));
-    mDirty = true;
-    pthread_mutex_unlock(&mMutex);
-    return 0;
-}
-
-camera_metadata_t* Metadata::generate()
-{
-    pthread_mutex_lock(&mMutex);
-    // Reuse if old generated metadata still valid
-    if (!mDirty && mGenerated != NULL) {
-        ALOGV("%s: Reusing generated metadata at %p", __func__, mGenerated);
-        goto out;
     }
-    // Destroy old metadata
-    if (mGenerated != NULL) {
-        ALOGV("%s: Freeing generated metadata at %p", __func__, mGenerated);
-        free_camera_metadata(mGenerated);
-        mGenerated = NULL;
+    // Append the current metadata to the new (empty) metadata
+    if (res = append_camera_metadata(tmp, mData)) {
+        ALOGE("%s: Failed to append old metadata %p to new %p",
+                __func__, mData, tmp);
+        return res;
     }
-    // Generate new metadata structure
-    ALOGV("%s: Generating new camera metadata structure, Entries:%d Data:%d",
-            __func__, mEntryCount, mDataCount);
-    mGenerated = allocate_camera_metadata(mEntryCount, mDataCount);
-    if (mGenerated == NULL) {
-        ALOGE("%s: Failed to allocate metadata (%d entries %d data)",
-                __func__, mEntryCount, mDataCount);
-        goto out;
+    // Add the remaining new item
+    if (res = add_camera_metadata_entry(tmp, tag, tag_data, count)) {
+        ALOGE("%s: Failed to add new entry (%d, %p, %d) to metadata %p",
+                __func__, tag, tag_data, count, tmp);
+        return res;
     }
-    // Walk list of entries adding each one to newly allocated metadata
-    for (Entry *current = mHead; current != NULL; current = current->mNext) {
-        int res = add_camera_metadata_entry(mGenerated, current->mTag,
-                current->mData, current->mCount);
-        if (res != 0) {
-            ALOGE("%s: Failed to add camera metadata: %d", __func__, res);
-            free_camera_metadata(mGenerated);
-            mGenerated = NULL;
-            goto out;
-        }
-    }
-
-out:
-    pthread_mutex_unlock(&mMutex);
-    return mGenerated;
-}
-
-Metadata::Entry::Entry(uint32_t tag, void *data, int count)
-  : mNext(NULL),
-    mPrev(NULL),
-    mTag(tag),
-    mData(data),
-    mCount(count)
-{
-}
 
-void Metadata::push(Entry *e)
-{
-    if (mHead == NULL) {
-        mHead = mTail = e;
-    } else {
-        mTail->insertAfter(e);
-        mTail = e;
-    }
-}
-
-Metadata::Entry::~Entry()
-{
-    if (mNext != NULL)
-        mNext->mPrev = mPrev;
-    if (mPrev != NULL)
-        mPrev->mNext = mNext;
+    replace(tmp);
+    return 0;
 }
 
-void Metadata::Entry::insertAfter(Entry *e)
+camera_metadata_t* Metadata::get()
 {
-    if (e == NULL)
-        return;
-    if (mNext != NULL)
-        mNext->mPrev = e;
-    e->mNext = mNext;
-    e->mPrev = this;
-    mNext = e;
+    return mData;
 }
 
 } // namespace default_camera_hal
diff --git a/modules/camera/Metadata.h b/modules/camera/Metadata.h
index c0d9492..f432d04 100644
--- a/modules/camera/Metadata.h
+++ b/modules/camera/Metadata.h
@@ -17,11 +17,9 @@
 #ifndef METADATA_H_
 #define METADATA_H_
 
-#include <pthread.h>
+#include <stdint.h>
 #include <hardware/camera3.h>
-#include <hardware/gralloc.h>
 #include <system/camera_metadata.h>
-#include <system/graphics.h>
 
 namespace default_camera_hal {
 // Metadata is a convenience class for dealing with libcamera_metadata
@@ -29,51 +27,32 @@ class Metadata {
     public:
         Metadata();
         ~Metadata();
-        // Constructor used for request metadata templates
-        Metadata(uint8_t mode, uint8_t intent);
+        // Initialize with framework metadata
+        int init(const camera_metadata_t *metadata);
 
-        // Parse and add an entry
-        int addUInt8(uint32_t tag, int count, uint8_t *data);
-        int addInt32(uint32_t tag, int count, int32_t *data);
-        int addFloat(uint32_t tag, int count, float *data);
-        int addInt64(uint32_t tag, int count, int64_t *data);
-        int addDouble(uint32_t tag, int count, double *data);
+        // Parse and add an entry. Allocates and copies new storage for *data.
+        int addUInt8(uint32_t tag, int count, const uint8_t *data);
+        int add1UInt8(uint32_t tag, const uint8_t data);
+        int addInt32(uint32_t tag, int count, const int32_t *data);
+        int addFloat(uint32_t tag, int count, const float *data);
+        int addInt64(uint32_t tag, int count, const int64_t *data);
+        int addDouble(uint32_t tag, int count, const double *data);
         int addRational(uint32_t tag, int count,
-                camera_metadata_rational_t *data);
-        // Generate a camera_metadata structure and fill it with internal data
-        camera_metadata_t *generate();
+                const camera_metadata_rational_t *data);
+
+        // Get a handle to the current metadata
+        // This is not a durable handle, and may be destroyed by add*/init
+        camera_metadata_t* get();
 
     private:
+        // Actual internal storage
+        camera_metadata_t* mData;
+        // Destroy old metadata and replace with new
+        void replace(camera_metadata_t *m);
         // Validate the tag, type and count for a metadata entry
         bool validate(uint32_t tag, int tag_type, int count);
-        // Add a verified tag with data to this Metadata structure
-        int add(uint32_t tag, int count, void *tag_data);
-
-        class Entry {
-            public:
-                Entry(uint32_t tag, void *data, int count);
-                ~Entry();
-                Entry *mNext;
-                Entry *mPrev;
-                const uint32_t mTag;
-                const void *mData;
-                const int mCount;
-                void insertAfter(Entry *e);
-        };
-        // List ends
-        Entry *mHead;
-        Entry *mTail;
-        // Append entry to list
-        void push(Entry *e);
-        // Total of entries and entry data size
-        int mEntryCount;
-        int mDataCount;
-        // Save generated metadata, invalidated on update
-        camera_metadata_t *mGenerated;
-        // Flag to force metadata regeneration
-        bool mDirty;
-        // Lock protecting the Metadata object for modifications
-        pthread_mutex_t mMutex;
+        // Add a verified tag with data
+        int add(uint32_t tag, int count, const void *tag_data);
 };
 } // namespace default_camera_hal