[libcamera-devel] [PATCH 8/8] android: CameraDevice: Manage requestTemplates_ with std::unique_ptr
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Mar 23 03:15:58 CET 2021
Hi Hiro,
Thank you for the patch.
On Tue, Mar 23, 2021 at 10:42:26AM +0900, Hirokazu Honda wrote:
A commit message would be nice here too.
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
>
> ---
> src/android/camera_device.cpp | 22 ++++++++--------------
> src/android/camera_device.h | 6 +++---
> 2 files changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 696f7676..a9a29c76 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -338,11 +338,7 @@ CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)
> }
> }
>
> -CameraDevice::~CameraDevice()
> -{
> - for (auto &it : requestTemplates_)
> - delete it.second;
> -}
> +CameraDevice::~CameraDevice() = default;
>
> std::unique_ptr<CameraDevice> CameraDevice::create(unsigned int id,
> std::shared_ptr<Camera> cam)
> @@ -1358,15 +1354,14 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> return staticMetadata_->get();
> }
>
> -CameraMetadata *CameraDevice::requestTemplatePreview()
> +std::unique_ptr<CameraMetadata> CameraDevice::requestTemplatePreview()
> {
> /*
> * \todo Keep this in sync with the actual number of entries.
> * Currently: 20 entries, 35 bytes
> */
> - CameraMetadata *requestTemplate = new CameraMetadata(21, 36);
> + auto requestTemplate = std::make_unique<CameraMetadata>(21, 36);
> if (!requestTemplate->isValid()) {
> - delete requestTemplate;
> return nullptr;
> }
>
> @@ -1454,9 +1449,9 @@ CameraMetadata *CameraDevice::requestTemplatePreview()
> return requestTemplate;
> }
>
> -CameraMetadata *CameraDevice::requestTemplateVideo()
> +std::unique_ptr<CameraMetadata> CameraDevice::requestTemplateVideo()
> {
> - CameraMetadata *previewTemplate = requestTemplatePreview();
> + auto previewTemplate = requestTemplatePreview();
We tend to spell out types explicitly when it doesn't make lines overly
long:
std::unique_ptr<CameraMetadata> previewTemplate = requestTemplatePreview();
Could you do so here ? The reason is that it makes code easier to read
by showing types explicitly. The use of 'auto' above is fine, as
auto requestTemplate = std::make_unique<CameraMetadata>(21, 36);
shows the type through the right-hand side, while with
auto previewTemplate = requestTemplatePreview();
one would have to look up the declaration of requestTemplatePreview().
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> if (!previewTemplate)
> return nullptr;
>
> @@ -1488,7 +1483,7 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)
> return it->second->get();
>
> /* Use the capture intent matching the requested template type. */
> - CameraMetadata *requestTemplate;
> + std::unique_ptr<CameraMetadata> requestTemplate;
> uint8_t captureIntent;
> switch (type) {
> case CAMERA3_TEMPLATE_PREVIEW:
> @@ -1521,15 +1516,14 @@ const camera_metadata_t *CameraDevice::constructDefaultRequestSettings(int type)
>
> if (!requestTemplate || !requestTemplate->isValid()) {
> LOG(HAL, Error) << "Failed to construct request template";
> - delete requestTemplate;
> return nullptr;
> }
>
> requestTemplate->updateEntry(ANDROID_CONTROL_CAPTURE_INTENT,
> &captureIntent, 1);
>
> - requestTemplates_[type] = requestTemplate;
> - return requestTemplate->get();
> + requestTemplates_[type] = std::move(requestTemplate);
> + return requestTemplates_[type]->get();
> }
>
> PixelFormat CameraDevice::toPixelFormat(int format) const
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index cea82b24..09c395ff 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -97,8 +97,8 @@ private:
> 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);
> - CameraMetadata *requestTemplatePreview();
> - CameraMetadata *requestTemplateVideo();
> + std::unique_ptr<CameraMetadata> requestTemplatePreview();
> + std::unique_ptr<CameraMetadata> requestTemplateVideo();
> libcamera::PixelFormat toPixelFormat(int format) const;
> int processControls(Camera3RequestDescriptor *descriptor);
> std::unique_ptr<CameraMetadata> getResultMetadata(
> @@ -114,7 +114,7 @@ private:
> std::unique_ptr<libcamera::CameraConfiguration> config_;
>
> std::unique_ptr<CameraMetadata> staticMetadata_;
> - std::map<unsigned int, const CameraMetadata *> requestTemplates_;
> + std::map<unsigned int, std::unique_ptr<CameraMetadata>> requestTemplates_;
> const camera3_callback_ops_t *callbacks_;
>
> std::vector<Camera3StreamConfiguration> streamConfigurations_;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list