From patchwork Fri May 7 11:00:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 12227 X-Patchwork-Delegate: paul.elder@ideasonboard.com Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 68B2FBF829 for ; Fri, 7 May 2021 11:00:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 01AD968919; Fri, 7 May 2021 13:00:49 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="YSOQVlac"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EE5B3602BB for ; Fri, 7 May 2021 13:00:47 +0200 (CEST) Received: from pyrite.rasen.tech (unknown [IPv6:2400:4051:61:600:2c71:1b79:d06d:5032]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7E8A73F2; Fri, 7 May 2021 13:00:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1620385247; bh=Y+Gx5a6MKVHNCRw5V63+Dq1/aVhBjA++zjW2SpR5Mpw=; h=From:To:Cc:Subject:Date:From; b=YSOQVlacCNbrTEVpj2i/CCqP7i7eIelXyPmlm2eyGEipVJ2UZJBg/SabqK276A+1M V4139a5gA2cAFTztqAtqhTh/8TXqWh1G51ANaYKf8hGHD0mdI0/dmD5g0tpB6oLy6j bZNQ5mvEFuEDAq02Jb+lqhHC1tKePo1J3mVVxm1U= From: Paul Elder To: libcamera-devel@lists.libcamera.org Date: Fri, 7 May 2021 20:00:34 +0900 Message-Id: <20210507110035.594433-1-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH v2] android: camera_metadata: Auto-resize CameraMetadata X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" This patch depends on the series "FULL hardware level fixes". Previously we had to manually declare the size of CameraMetadata on allocation, and its count could not be changed after construction. Change CameraMetadata's behavior so that the user can simply add entries, and the CameraMetadata will auto-resize (double the size) as necessary. At the same time, make addEntry() also serve the purpose of updateEntry(), and remove updateEntry(). Also remove everything involved with calculating the initial size for any CameraMetadata instances. Signed-off-by: Paul Elder --- Changes in v2: - add overloading of addEntry - better logging with the separation of core code vs template code - addEntry failure makes the metadata invalid 1 - What do you (plural) think about merging updateEntry() into addEntry()? I thought that with the dynamic allocation it would be convenient to use one function. Is there any reason that we should keep them separate, or is it fine to merge them? - v2: I'm leaning towards merging them. The performance difference is binary/linear search for determining add vs update. Which search depends on if the metadata is sorted or not; it seems it's sorted until a metadata is appended to a non-empty metadata, which I don't think would happen since resize is always appending a non-empty metadata to an empty one. 2 - valid_ used to be synonymous with metadata_ == nullptr. In v2 I've made it so that failure to addEntry makes the instance invalid. This is because with 150 calls in camera_device.cpp, checking every call doesn't seem too nice. Maybe the valid_ flag should be replaced with a status flag? And that way we can just initialize an empty instance and addEntry into it? Or maybe that would never happen because we'd always suggest a size? I'm planning to rename addEntry to setEntry. Just "set" seems nice too but maybe it's too short? Or maybe [] operator would be nice too... Anyway I'm keeping it as addEntry to reduce clutter in the RFC. The main feature of v2 is the overloading of addEntry. I have three overloads and the main one: - tag, single piece of data (reference) - tag, span - tag, data pointer, count - tag, data pointer, count, size of one instance of the data The last one is the main one, and is not templated. The first two are nice and convenient, because now we can do things like: uint32_t orientation = 0; resultMetadata->addEntry(ANDROID_JPEG_ORIENTATION, orientation); and std::vector availableStates = { SOME_STATE_1, SOME_STATE_2 }; metadata->addEntry(ANDROID_AVAILABLE_STATES, availableStates); The third is for when android metadata is copied directly from another piece of android metadata. This is very common, and wasn't very nice having to wrap everything in a Span. Everything in camera_metadata.cpp works as-is with this overload (I only changed post_process_jpeg.cpp as a sample of the above two overloads). todos: - test on CTS - track resize and print if it occurred, and what the size change was --- src/android/camera_device.cpp | 79 ++++-------------------- src/android/camera_device.h | 1 - src/android/camera_metadata.cpp | 79 +++++++++++++++--------- src/android/camera_metadata.h | 25 +++++++- src/android/jpeg/post_processor_jpeg.cpp | 16 ++--- 5 files changed, 94 insertions(+), 106 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 114348a6..426e3fcd 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -772,53 +772,6 @@ void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks) callbacks_ = callbacks; } -std::tuple CameraDevice::calculateStaticMetadataSize() -{ - /* - * \todo Keep this in sync with the actual number of entries. - * Currently: 63 entries, 1014 bytes of static metadata - */ - uint32_t numEntries = 63; - uint32_t byteSize = 1014; - - // do i need to add for entries in the available keys? - // +1, +4 for EDGE_AVAILABLE_EDGE_MODES - // +1, +4 for LENS_INFO_AVAILABLE_FILTER_DENSITIES - // +1, +4 for BLACK_LEVEL_PATTERN - // +1, +4 for TONEMAP_AVAILABLE_TONE_MAP_MODES - // +1, +4 for TONEMAP_MAX_CURVE_POINTS - // +4x9 = 36 for the new result tags - - // +36 for new request keys - - /* - * Calculate space occupation in bytes for dynamically built metadata - * entries. - * - * Each stream configuration entry requires 48 bytes: - * 4 32bits integers for ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS - * 4 64bits integers for ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS - */ - byteSize += streamConfigurations_.size() * 48; - - /* - * 2 32bits integers for each HAL_PIXEL_FORMAT_BLOB for thumbnail sizes - * 2 32bits integers for the (0, 0) thumbnail size - * - * This is a worst case estimates as different configurations with the - * same aspect ratio will generate the same size. - */ - for (const auto &entry : streamConfigurations_) { - if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB) - continue; - - byteSize += 8; - } - byteSize += 8; - - return std::make_tuple(numEntries, byteSize); -} - /* * Return static information for the camera. */ @@ -827,15 +780,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata() if (staticMetadata_) return staticMetadata_->get(); - /* - * The here reported metadata are enough to implement a basic capture - * example application, but a real camera implementation will require - * more. - */ - uint32_t numEntries; - uint32_t byteSize; - std::tie(numEntries, byteSize) = calculateStaticMetadataSize(); - staticMetadata_ = std::make_unique(numEntries, byteSize); + staticMetadata_ = std::make_unique(32, 512); if (!staticMetadata_->isValid()) { LOG(HAL, Error) << "Failed to allocate static metadata"; staticMetadata_.reset(); @@ -1757,13 +1702,13 @@ std::unique_ptr CameraDevice::requestTemplateVideo() &entry); uint8_t edgeMode = ANDROID_EDGE_MODE_FAST; - previewTemplate->updateEntry(ANDROID_EDGE_MODE, &edgeMode, 1); + previewTemplate->addEntry(ANDROID_EDGE_MODE, &edgeMode, 1); /* * Assume the AE_AVAILABLE_TARGET_FPS_RANGE static metadata * has been assembled as {{min, max} {max, max}}. */ - previewTemplate->updateEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, + previewTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, entry.data.i32 + 2, 2); return previewTemplate; @@ -1780,17 +1725,17 @@ std::unique_ptr CameraDevice::requestTemplateStill() * HIGH_QUALITY. */ uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY; - previewTemplate->updateEntry(ANDROID_NOISE_REDUCTION_MODE, + previewTemplate->addEntry(ANDROID_NOISE_REDUCTION_MODE, &noiseReduction, 1); uint8_t edgeMode = ANDROID_EDGE_MODE_HIGH_QUALITY; - previewTemplate->updateEntry(ANDROID_EDGE_MODE, &edgeMode, 1); + previewTemplate->addEntry(ANDROID_EDGE_MODE, &edgeMode, 1); uint8_t shadingMode = ANDROID_SHADING_MODE_HIGH_QUALITY; - previewTemplate->updateEntry(ANDROID_SHADING_MODE, &shadingMode, 1); + previewTemplate->addEntry(ANDROID_SHADING_MODE, &shadingMode, 1); uint8_t tonemapMode = ANDROID_TONEMAP_MODE_HIGH_QUALITY; - previewTemplate->updateEntry(ANDROID_TONEMAP_MODE, &tonemapMode, 1); + previewTemplate->addEntry(ANDROID_TONEMAP_MODE, &tonemapMode, 1); return previewTemplate; } @@ -1802,16 +1747,16 @@ std::unique_ptr CameraDevice::requestTemplateManual() return nullptr; uint8_t controlMode = ANDROID_CONTROL_MODE_OFF; - previewTemplate->updateEntry(ANDROID_CONTROL_MODE, &controlMode, 1); + previewTemplate->addEntry(ANDROID_CONTROL_MODE, &controlMode, 1); uint8_t aeMode = ANDROID_CONTROL_AE_MODE_OFF; - previewTemplate->updateEntry(ANDROID_CONTROL_AE_MODE, &aeMode, 1); + previewTemplate->addEntry(ANDROID_CONTROL_AE_MODE, &aeMode, 1); uint8_t awbMode = ANDROID_CONTROL_AWB_MODE_OFF; - previewTemplate->updateEntry(ANDROID_CONTROL_AWB_MODE, &awbMode, 1); + previewTemplate->addEntry(ANDROID_CONTROL_AWB_MODE, &awbMode, 1); uint8_t edgeMode = ANDROID_EDGE_MODE_OFF; - previewTemplate->updateEntry(ANDROID_EDGE_MODE, &edgeMode, 1); + previewTemplate->addEntry(ANDROID_EDGE_MODE, &edgeMode, 1); /* \todo get this from available filter densities */ float filterDensity = 0.0f; @@ -1867,7 +1812,7 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type) return nullptr; } - requestTemplate->updateEntry(ANDROID_CONTROL_CAPTURE_INTENT, + requestTemplate->addEntry(ANDROID_CONTROL_CAPTURE_INTENT, &captureIntent, 1); requestTemplates_[type] = std::move(requestTemplate); diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 8edbcdfd..88aab012 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -98,7 +98,6 @@ private: std::vector getRawResolutions(const libcamera::PixelFormat &pixelFormat); - std::tuple calculateStaticMetadataSize(); libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer); void notifyShutter(uint32_t frameNumber, uint64_t timestamp); void notifyError(uint32_t frameNumber, camera3_stream_t *stream); diff --git a/src/android/camera_metadata.cpp b/src/android/camera_metadata.cpp index 6f1bcdbe..b7abea96 100644 --- a/src/android/camera_metadata.cpp +++ b/src/android/camera_metadata.cpp @@ -63,52 +63,73 @@ bool CameraMetadata::getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry) c return true; } -bool CameraMetadata::addEntry(uint32_t tag, const void *data, size_t count) +bool CameraMetadata::resize(size_t count, size_t size) { if (!valid_) return false; - if (!add_camera_metadata_entry(metadata_, tag, data, count)) - return true; - - const char *name = get_camera_metadata_tag_name(tag); - if (name) - LOG(CameraMetadata, Error) - << "Failed to add tag " << name; - else - LOG(CameraMetadata, Error) - << "Failed to add unknown tag " << tag; - - valid_ = false; + size_t currentEntryCount = get_camera_metadata_entry_count(metadata_); + size_t currentEntryCapacity = get_camera_metadata_entry_capacity(metadata_); + size_t newEntryCapacity = currentEntryCapacity < currentEntryCount + count ? + currentEntryCapacity * 2 : currentEntryCapacity; + + size_t currentDataCount = get_camera_metadata_data_count(metadata_); + size_t currentDataCapacity = get_camera_metadata_data_capacity(metadata_); + size_t newDataCapacity = currentDataCapacity < currentDataCount + size ? + currentDataCapacity * 2 : currentDataCapacity; + + if (newEntryCapacity > currentEntryCapacity || + newDataCapacity > currentDataCapacity) { + camera_metadata_t *oldMetadata = metadata_; + metadata_ = allocate_camera_metadata(newEntryCapacity, newDataCapacity); + if (!metadata_) { + metadata_ = oldMetadata; + return false; + } + + append_camera_metadata(metadata_, oldMetadata); + free_camera_metadata(oldMetadata); + } - return false; + return true; } -bool CameraMetadata::updateEntry(uint32_t tag, const void *data, size_t count) +void CameraMetadata::addEntry(uint32_t tag, const void *data, size_t count, + size_t sizeofT) { if (!valid_) - return false; + return; camera_metadata_entry_t entry; int ret = find_camera_metadata_entry(metadata_, tag, &entry); if (ret) { - const char *name = get_camera_metadata_tag_name(tag); - LOG(CameraMetadata, Error) - << "Failed to update tag " - << (name ? name : "") << ": not present"; - return false; - } + if (!resize(1, count * sizeofT)) { + LOG(CameraMetadata, Error) << "Failed to resize"; + valid_ = false; + return; + } + + if (!add_camera_metadata_entry(metadata_, tag, data, count)) + return; - ret = update_camera_metadata_entry(metadata_, entry.index, data, - count, nullptr); - if (ret) { const char *name = get_camera_metadata_tag_name(tag); - LOG(CameraMetadata, Error) - << "Failed to update tag " << (name ? name : ""); - return false; + LOG(CameraMetadata, Error) << "Failed to add tag " << (name ? name : ""); + + valid_ = false; + + return; } - return true; + if (!update_camera_metadata_entry(metadata_, entry.index, data, + count, nullptr)) + return; + + const char *name = get_camera_metadata_tag_name(tag); + LOG(CameraMetadata, Error) << "Failed to update tag " << (name ? name : ""); + + valid_ = false; + + return; } camera_metadata_t *CameraMetadata::get() diff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h index d653e2f0..c4c91112 100644 --- a/src/android/camera_metadata.h +++ b/src/android/camera_metadata.h @@ -11,6 +11,8 @@ #include +#include + class CameraMetadata { public: @@ -23,9 +25,28 @@ public: CameraMetadata &operator=(const CameraMetadata &other); bool isValid() const { return valid_; } + bool resize(size_t count, size_t size); bool getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry) const; - bool addEntry(uint32_t tag, const void *data, size_t data_count); - bool updateEntry(uint32_t tag, const void *data, size_t data_count); + + template + void addEntry(uint32_t tag, const T &data) + { + addEntry(tag, &data, 1, sizeof(T)); + } + + template + void addEntry(uint32_t tag, libcamera::Span &data) + { + addEntry(tag, data.data(), data.size(), sizeof(T)); + } + + template + void addEntry(uint32_t tag, const T *data, size_t count) + { + addEntry(tag, data, count, sizeof(T)); + } + + void addEntry(uint32_t tag, const void *data, size_t count, size_t sizeofT); camera_metadata_t *get(); const camera_metadata_t *get() const; diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 237fb318..4c6e5eb2 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -15,6 +15,7 @@ #include "exif.h" #include +#include #include "libcamera/internal/log.h" @@ -103,7 +104,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, ret = requestMetadata.getEntry(ANDROID_JPEG_ORIENTATION, &entry); const uint32_t jpegOrientation = ret ? *entry.data.i32 : 0; - resultMetadata->addEntry(ANDROID_JPEG_ORIENTATION, &jpegOrientation, 1); + resultMetadata->addEntry(ANDROID_JPEG_ORIENTATION, jpegOrientation); exif.setOrientation(jpegOrientation); exif.setSize(streamSize_); @@ -132,7 +133,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, if (ret) { exif.setGPSDateTimestamp(*entry.data.i64); resultMetadata->addEntry(ANDROID_JPEG_GPS_TIMESTAMP, - entry.data.i64, 1); + *entry.data.i64); } ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_SIZE, &entry); @@ -143,7 +144,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, ret = requestMetadata.getEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &entry); uint8_t quality = ret ? *entry.data.u8 : 95; - resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, &quality, 1); + resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_QUALITY, quality); if (thumbnailSize != Size(0, 0)) { std::vector thumbnail; @@ -152,14 +153,15 @@ int PostProcessorJpeg::process(const FrameBuffer &source, exif.setThumbnail(thumbnail, Exif::Compression::JPEG); } - resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, data, 2); + resultMetadata->addEntry(ANDROID_JPEG_THUMBNAIL_SIZE, + Span{data, 2}); } ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_COORDINATES, &entry); if (ret) { exif.setGPSLocation(entry.data.d); resultMetadata->addEntry(ANDROID_JPEG_GPS_COORDINATES, - entry.data.d, 3); + Span{entry.data.d, 3}); } ret = requestMetadata.getEntry(ANDROID_JPEG_GPS_PROCESSING_METHOD, &entry); @@ -175,7 +177,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, ret = requestMetadata.getEntry(ANDROID_JPEG_QUALITY, &entry); const uint8_t quality = ret ? *entry.data.u8 : 95; - resultMetadata->addEntry(ANDROID_JPEG_QUALITY, &quality, 1); + resultMetadata->addEntry(ANDROID_JPEG_QUALITY, quality); int jpeg_size = encoder_->encode(source, destination->plane(0), exif.data(), quality); @@ -193,7 +195,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, blob->jpeg_size = jpeg_size; /* Update the JPEG result Metadata. */ - resultMetadata->addEntry(ANDROID_JPEG_SIZE, &jpeg_size, 1); + resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size); return 0; }