[libcamera-devel] [PATCH 1/2] android: CameraDevice: Mark getResultMetadata() const function

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Thu, Mar 25, 2021 at 08:13:56PM +0900, Hirokazu Honda wrote:
> CameraDevice::getResultMetadata() doesn't change either
> |descriptor| and member variables. It should be marked as a
> const function and |descriptor| should be passed with const
> lvalue reference.

The patch itself looks good to me. I'd however like Jacopo's feedback,
as he has designed these classes, regarding whether he foresees a need
to modify the descriptor in that function in the near future.

If Jacopo is fine with the patch, you can also add my

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/android/camera_device.cpp | 10 +++++-----
>  src/android/camera_device.h   |  3 ++-
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 5fbf6f82..ae693664 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1924,7 +1924,7 @@ void CameraDevice::requestComplete(Request *request)
>  	 * pipeline handlers) timestamp in the Request itself.
>  	 */
>  	uint64_t timestamp = buffers.begin()->second->metadata().timestamp;
> -	resultMetadata = getResultMetadata(descriptor, timestamp);
> +	resultMetadata = getResultMetadata(*descriptor, timestamp);
> 
>  	/* Handle any JPEG compression. */
>  	for (camera3_stream_buffer_t &buffer : descriptor->buffers_) {
> @@ -2030,11 +2030,11 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream)
>   * Produce a set of fixed result metadata.
>   */
>  std::unique_ptr<CameraMetadata>
> -CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,
> -				int64_t timestamp)
> +CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor,
> +				int64_t timestamp) const
>  {
> -	const ControlList &metadata = descriptor->request_->metadata();
> -	const CameraMetadata &settings = descriptor->settings_;
> +	const ControlList &metadata = descriptor.request_->metadata();
> +	const CameraMetadata &settings = descriptor.settings_;
>  	camera_metadata_ro_entry_t entry;
>  	bool found;
> 
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 09c395ff..11bdfec8 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -102,7 +102,8 @@ private:
>  	libcamera::PixelFormat toPixelFormat(int format) const;
>  	int processControls(Camera3RequestDescriptor *descriptor);
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
> -		Camera3RequestDescriptor *descriptor, int64_t timestamp);
> +		const Camera3RequestDescriptor &descriptor,
> +		int64_t timestamp) const;
> 
>  	unsigned int id_;
>  	camera3_device_t camera3Device_;

-- 
Regards,

Laurent Pinchart