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

Jacopo Mondi jacopo at jmondi.org
Thu Mar 25 17:04:39 CET 2021


Hi Laurent,

On Thu, Mar 25, 2021 at 04:52:23PM +0200, Laurent Pinchart wrote:
> 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.

As far as I can tell this should not happen.
In any case we can rollback if that need arise in future.

>
> If Jacopo is fine with the patch, you can also add my
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>
> > 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
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel


More information about the libcamera-devel mailing list