{"id":12206,"url":"https://patchwork.libcamera.org/api/patches/12206/?format=json","web_url":"https://patchwork.libcamera.org/patch/12206/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210506104150.572045-1-paul.elder@ideasonboard.com>","date":"2021-05-06T10:41:50","name":"[libcamera-devel,RFC] android: camera_metadata: Auto-resize CameraMetadata","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"0503074b8b9d1f348b3c13624cdbef9165075b4c","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/?format=json","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"delegate":{"id":17,"url":"https://patchwork.libcamera.org/api/users/17/?format=json","username":"epaul","first_name":"Paul","last_name":"Elder","email":"paul.elder@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/patch/12206/mbox/","series":[{"id":2002,"url":"https://patchwork.libcamera.org/api/series/2002/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2002","date":"2021-05-06T10:41:50","name":"[libcamera-devel,RFC] android: camera_metadata: Auto-resize CameraMetadata","version":1,"mbox":"https://patchwork.libcamera.org/series/2002/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12206/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12206/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CFEB0BF81D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  6 May 2021 10:42:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 472C068915;\n\tThu,  6 May 2021 12:42:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6185868901\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  6 May 2021 12:42:25 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D9F2289D;\n\tThu,  6 May 2021 12:42:23 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jjqYUkCm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620297745;\n\tbh=ttSIaJKR3J09p4vGqWIlEvN1ANdm07Rc5ZwNM/rc1ok=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=jjqYUkCmqL6bjkcFzXG7mAeGr/5PGiEzxpH7KqQBcVG5jfBszEjIPWfSlBE35h7Vt\n\t7phVXiyhm5Q7OCiVC62vkJwOXBFjB2tyOlKKaHvXbpvCMk8l9YuitWBEokM5EH0ZUl\n\t9TeKTJOvVPrAfY9Hugz5IfYAzh/4X9KJnL79KuiI=","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Thu,  6 May 2021 19:41:50 +0900","Message-Id":"<20210506104150.572045-1-paul.elder@ideasonboard.com>","X-Mailer":"git-send-email 2.27.0","MIME-Version":"1.0","Subject":"[libcamera-devel] [RFC PATCH] android: camera_metadata: Auto-resize\n\tCameraMetadata","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"This patch depends on the series \"FULL hardware level fixes\".\n\nPreviously we had to manually declare the size of CameraMetadata on\nallocation, and its count not be changed after construction. Change\nCameraMetadata's behavior so that the user can simply add entries, and\nthe CameraMetadata will auto-resize (double the size) as necessary. At\nthe same time, make addEntry() also serve the purpose of updateEntry(),\nand remove updateEntry(). Also remove everything involved with\ncalculating the initial size for any CameraMetadata instances.\n\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\n\n---\n1 - What do you (plural) think about merging updateEntry() into\n    addEntry()? I thought that with the dynamic allocation it would be\n    convenient to use one function. Is there any reason that we should\n    keep them separate, or is it fine to merge them?\n2 - How can I get logging working in the CameraMetadata header file? The\n    last time I did that was in ipa_data_serializer.h and that wasn't very\n    pretty either...\n\n(I haven't tested it on CTS yet; this is just RFC for the API and\nimplementation)\n---\n src/android/camera_device.cpp   | 79 +++++----------------------------\n src/android/camera_device.h     |  1 -\n src/android/camera_metadata.cpp | 61 +++++++++----------------\n src/android/camera_metadata.h   | 39 +++++++++++++++-\n 4 files changed, 71 insertions(+), 109 deletions(-)","diff":"diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 114348a6..426e3fcd 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -772,53 +772,6 @@ void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks)\n \tcallbacks_ = callbacks;\n }\n \n-std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()\n-{\n-\t/*\n-\t * \\todo Keep this in sync with the actual number of entries.\n-\t * Currently: 63 entries, 1014 bytes of static metadata\n-\t */\n-\tuint32_t numEntries = 63;\n-\tuint32_t byteSize = 1014;\n-\n-\t// do i need to add for entries in the available keys?\n-\t// +1, +4 for EDGE_AVAILABLE_EDGE_MODES\n-\t// +1, +4 for LENS_INFO_AVAILABLE_FILTER_DENSITIES\n-\t// +1, +4 for BLACK_LEVEL_PATTERN\n-\t// +1, +4 for TONEMAP_AVAILABLE_TONE_MAP_MODES\n-\t// +1, +4 for TONEMAP_MAX_CURVE_POINTS\n-\t// +4x9 = 36 for the new result tags\n-\n-\t// +36 for new request keys\n-\n-\t/*\n-\t * Calculate space occupation in bytes for dynamically built metadata\n-\t * entries.\n-\t *\n-\t * Each stream configuration entry requires 48 bytes:\n-\t * 4 32bits integers for ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS\n-\t * 4 64bits integers for ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS\n-\t */\n-\tbyteSize += streamConfigurations_.size() * 48;\n-\n-\t/*\n-\t * 2 32bits integers for each HAL_PIXEL_FORMAT_BLOB for thumbnail sizes\n-\t * 2 32bits integers for the (0, 0) thumbnail size\n-\t *\n-\t * This is a worst case estimates as different configurations with the\n-\t * same aspect ratio will generate the same size.\n-\t */\n-\tfor (const auto &entry : streamConfigurations_) {\n-\t\tif (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB)\n-\t\t\tcontinue;\n-\n-\t\tbyteSize += 8;\n-\t}\n-\tbyteSize += 8;\n-\n-\treturn std::make_tuple(numEntries, byteSize);\n-}\n-\n /*\n  * Return static information for the camera.\n  */\n@@ -827,15 +780,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n \tif (staticMetadata_)\n \t\treturn staticMetadata_->get();\n \n-\t/*\n-\t * The here reported metadata are enough to implement a basic capture\n-\t * example application, but a real camera implementation will require\n-\t * more.\n-\t */\n-\tuint32_t numEntries;\n-\tuint32_t byteSize;\n-\tstd::tie(numEntries, byteSize) = calculateStaticMetadataSize();\n-\tstaticMetadata_ = std::make_unique<CameraMetadata>(numEntries, byteSize);\n+\tstaticMetadata_ = std::make_unique<CameraMetadata>(32, 512);\n \tif (!staticMetadata_->isValid()) {\n \t\tLOG(HAL, Error) << \"Failed to allocate static metadata\";\n \t\tstaticMetadata_.reset();\n@@ -1757,13 +1702,13 @@ std::unique_ptr<CameraMetadata> CameraDevice::requestTemplateVideo()\n \t\t\t\t  &entry);\n \n \tuint8_t edgeMode = ANDROID_EDGE_MODE_FAST;\n-\tpreviewTemplate->updateEntry(ANDROID_EDGE_MODE, &edgeMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_EDGE_MODE, &edgeMode, 1);\n \n \t/*\n \t * Assume the AE_AVAILABLE_TARGET_FPS_RANGE static metadata\n \t * has been assembled as {{min, max} {max, max}}.\n \t */\n-\tpreviewTemplate->updateEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n+\tpreviewTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,\n \t\t\t\t     entry.data.i32 + 2, 2);\n \n \treturn previewTemplate;\n@@ -1780,17 +1725,17 @@ std::unique_ptr<CameraMetadata> CameraDevice::requestTemplateStill()\n \t * HIGH_QUALITY.\n \t */\n \tuint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY;\n-\tpreviewTemplate->updateEntry(ANDROID_NOISE_REDUCTION_MODE,\n+\tpreviewTemplate->addEntry(ANDROID_NOISE_REDUCTION_MODE,\n \t\t\t\t     &noiseReduction, 1);\n \n \tuint8_t edgeMode = ANDROID_EDGE_MODE_HIGH_QUALITY;\n-\tpreviewTemplate->updateEntry(ANDROID_EDGE_MODE, &edgeMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_EDGE_MODE, &edgeMode, 1);\n \n \tuint8_t shadingMode = ANDROID_SHADING_MODE_HIGH_QUALITY;\n-\tpreviewTemplate->updateEntry(ANDROID_SHADING_MODE, &shadingMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_SHADING_MODE, &shadingMode, 1);\n \n \tuint8_t tonemapMode = ANDROID_TONEMAP_MODE_HIGH_QUALITY;\n-\tpreviewTemplate->updateEntry(ANDROID_TONEMAP_MODE, &tonemapMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_TONEMAP_MODE, &tonemapMode, 1);\n \n \treturn previewTemplate;\n }\n@@ -1802,16 +1747,16 @@ std::unique_ptr<CameraMetadata> CameraDevice::requestTemplateManual()\n \t\treturn nullptr;\n \n \tuint8_t controlMode = ANDROID_CONTROL_MODE_OFF;\n-\tpreviewTemplate->updateEntry(ANDROID_CONTROL_MODE, &controlMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_CONTROL_MODE, &controlMode, 1);\n \n \tuint8_t aeMode = ANDROID_CONTROL_AE_MODE_OFF;\n-\tpreviewTemplate->updateEntry(ANDROID_CONTROL_AE_MODE, &aeMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_CONTROL_AE_MODE, &aeMode, 1);\n \n \tuint8_t awbMode = ANDROID_CONTROL_AWB_MODE_OFF;\n-\tpreviewTemplate->updateEntry(ANDROID_CONTROL_AWB_MODE, &awbMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_CONTROL_AWB_MODE, &awbMode, 1);\n \n \tuint8_t edgeMode = ANDROID_EDGE_MODE_OFF;\n-\tpreviewTemplate->updateEntry(ANDROID_EDGE_MODE, &edgeMode, 1);\n+\tpreviewTemplate->addEntry(ANDROID_EDGE_MODE, &edgeMode, 1);\n \n \t/* \\todo get this from available filter densities */\n \tfloat filterDensity = 0.0f;\n@@ -1867,7 +1812,7 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)\n \t\treturn nullptr;\n \t}\n \n-\trequestTemplate->updateEntry(ANDROID_CONTROL_CAPTURE_INTENT,\n+\trequestTemplate->addEntry(ANDROID_CONTROL_CAPTURE_INTENT,\n \t\t\t\t     &captureIntent, 1);\n \n \trequestTemplates_[type] = std::move(requestTemplate);\ndiff --git a/src/android/camera_device.h b/src/android/camera_device.h\nindex 8edbcdfd..88aab012 100644\n--- a/src/android/camera_device.h\n+++ b/src/android/camera_device.h\n@@ -98,7 +98,6 @@ private:\n \tstd::vector<libcamera::Size>\n \tgetRawResolutions(const libcamera::PixelFormat &pixelFormat);\n \n-\tstd::tuple<uint32_t, uint32_t> calculateStaticMetadataSize();\n \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);\n \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream);\ndiff --git a/src/android/camera_metadata.cpp b/src/android/camera_metadata.cpp\nindex 6f1bcdbe..e0b314ee 100644\n--- a/src/android/camera_metadata.cpp\n+++ b/src/android/camera_metadata.cpp\n@@ -63,49 +63,32 @@ bool CameraMetadata::getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry) c\n \treturn true;\n }\n \n-bool CameraMetadata::addEntry(uint32_t tag, const void *data, size_t count)\n+bool CameraMetadata::resize(size_t count, size_t size)\n {\n \tif (!valid_)\n \t\treturn false;\n \n-\tif (!add_camera_metadata_entry(metadata_, tag, data, count))\n-\t\treturn true;\n-\n-\tconst char *name = get_camera_metadata_tag_name(tag);\n-\tif (name)\n-\t\tLOG(CameraMetadata, Error)\n-\t\t\t<< \"Failed to add tag \" << name;\n-\telse\n-\t\tLOG(CameraMetadata, Error)\n-\t\t\t<< \"Failed to add unknown tag \" << tag;\n-\n-\tvalid_ = false;\n-\n-\treturn false;\n-}\n-\n-bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count)\n-{\n-\tif (!valid_)\n-\t\treturn false;\n-\n-\tcamera_metadata_entry_t entry;\n-\tint ret = find_camera_metadata_entry(metadata_, tag, &entry);\n-\tif (ret) {\n-\t\tconst char *name = get_camera_metadata_tag_name(tag);\n-\t\tLOG(CameraMetadata, Error)\n-\t\t\t<< \"Failed to update tag \"\n-\t\t\t<< (name ? name : \"<unknown>\") << \": not present\";\n-\t\treturn false;\n-\t}\n-\n-\tret = update_camera_metadata_entry(metadata_, entry.index, data,\n-\t\t\t\t\t   count, nullptr);\n-\tif (ret) {\n-\t\tconst char *name = get_camera_metadata_tag_name(tag);\n-\t\tLOG(CameraMetadata, Error)\n-\t\t\t<< \"Failed to update tag \" << (name ? name : \"<unknown>\");\n-\t\treturn false;\n+\tsize_t currentEntryCount = get_camera_metadata_entry_count(metadata_);\n+\tsize_t currentEntryCapacity = get_camera_metadata_entry_capacity(metadata_);\n+\tsize_t newEntryCapacity = currentEntryCapacity < currentEntryCount + count ?\n+\t\t\t\t  currentEntryCapacity * 2 : currentEntryCapacity;\n+\n+\tsize_t currentDataCount = get_camera_metadata_data_count(metadata_);\n+\tsize_t currentDataCapacity = get_camera_metadata_data_capacity(metadata_);\n+\tsize_t newDataCapacity = currentDataCapacity < currentDataCount + size ?\n+\t\t\t\t currentDataCapacity * 2 : currentDataCapacity;\n+\n+\tif (newEntryCapacity > currentEntryCapacity ||\n+\t    newDataCapacity > currentDataCapacity) {\n+\t\tcamera_metadata_t *oldMetadata = metadata_;\n+\t\tmetadata_ = allocate_camera_metadata(newEntryCapacity, newDataCapacity);\n+\t\tif (!metadata_) {\n+\t\t\tmetadata_ = oldMetadata;\n+\t\t\treturn false;\n+\t\t}\n+\n+\t\tappend_camera_metadata(metadata_, oldMetadata);\n+\t\tfree_camera_metadata(oldMetadata);\n \t}\n \n \treturn true;\ndiff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h\nindex d653e2f0..c35ea1b7 100644\n--- a/src/android/camera_metadata.h\n+++ b/src/android/camera_metadata.h\n@@ -7,6 +7,7 @@\n #ifndef __ANDROID_CAMERA_METADATA_H__\n #define __ANDROID_CAMERA_METADATA_H__\n \n+#include <iostream>\n #include <stdint.h>\n \n #include <system/camera_metadata.h>\n@@ -23,9 +24,43 @@ public:\n \tCameraMetadata &operator=(const CameraMetadata &other);\n \n \tbool isValid() const { return valid_; }\n+\tbool resize(size_t count, size_t size);\n \tbool getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry) const;\n-\tbool addEntry(uint32_t tag, const void *data, size_t data_count);\n-\tbool updateEntry(uint32_t tag, const void *data, size_t data_count);\n+\n+\ttemplate<typename T>\n+\tbool addEntry(uint32_t tag, const T *data, size_t count)\n+\t{\n+\t\tif (!valid_)\n+\t\t\treturn false;\n+\n+\t\tcamera_metadata_entry_t entry;\n+\t\tint ret = find_camera_metadata_entry(metadata_, tag, &entry);\n+\t\tif (ret) {\n+\t\t\tif (!resize(1, count * sizeof(T))) {\n+\t\t\t\tstd::cerr << \"Failed to resize\";\n+\t\t\t\treturn false;\n+\t\t\t}\n+\n+\t\t\tif (!add_camera_metadata_entry(metadata_, tag, data, count))\n+\t\t\t\treturn true;\n+\n+\t\t\tconst char *name = get_camera_metadata_tag_name(tag);\n+\t\t\tstd::cerr << \"Failed to add tag \" << (name ? name : \"<unknown>\");\n+\n+\t\t\tvalid_ = false;\n+\n+\t\t\treturn false;\n+\t\t}\n+\n+\t\tif (!update_camera_metadata_entry(metadata_, entry.index, data,\n+\t\t\t\t\t\t  count, nullptr))\n+\t\t\treturn true;\n+\n+\t\tconst char *name = get_camera_metadata_tag_name(tag);\n+\t\tstd::cerr << \"Failed to update tag \" << (name ? name : \"<unknown>\");\n+\n+\t\treturn false;\n+\t}\n \n \tcamera_metadata_t *get();\n \tconst camera_metadata_t *get() const;\n","prefixes":["libcamera-devel","RFC"]}