[libcamera-devel] [PATCH v2 3/9] android: post_processor: Plumb down the process() with context

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Fri, Sep 10, 2021 at 12:36:32PM +0530, Umang Jain wrote:
> The context required for post processing of a camera request is saved
> via Camera3RequestDescriptor. The context needs to be plumbed to
> PostProcessor::process() which should be passed a const pointer to
> Camera3RequestDescriptor. In subsequent commits, we will prepare
> the post-processor to run asynchrnoously hence, it will require the
> context pointer to be emitted back to the upper layers, to identify for
> which request the post-processing has been completed.
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/camera_device.cpp            | 3 ++-
>  src/android/camera_stream.cpp            | 6 ++++--
>  src/android/camera_stream.h              | 5 ++++-
>  src/android/jpeg/post_processor_jpeg.cpp | 3 ++-
>  src/android/jpeg/post_processor_jpeg.h   | 3 ++-
>  src/android/post_processor.h             | 5 ++++-
>  src/android/yuv/post_processor_yuv.cpp   | 3 ++-
>  src/android/yuv/post_processor_yuv.h     | 3 ++-
>  8 files changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 0840c056..84549d04 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1150,7 +1150,8 @@ void CameraDevice::requestComplete(Request *request)
>  
>  		int ret = cameraStream->process(src, *buffer.buffer,
>  						descriptor.settings_,

Now that the request metadata can be accessed through the descriptor
pointer, could you please drop this argument ?

> -						resultMetadata.get());
> +						resultMetadata.get(),
> +						&descriptor);

I'd move the descriptor parameter before the metadata. I think it would
also make sense to store the result metadata in
Camera3RequestDescriptor, but that can go to another patch.

>  		/*
>  		 * Return the FrameBuffer to the CameraStream now that we're
>  		 * done processing it.
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 0fed5382..d1c54797 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -101,7 +101,8 @@ int CameraStream::configure()
>  int CameraStream::process(const FrameBuffer *source,
>  			  buffer_handle_t camera3Dest,
>  			  const CameraMetadata &requestMetadata,
> -			  CameraMetadata *resultMetadata)
> +			  CameraMetadata *resultMetadata,
> +			  const Camera3RequestDescriptor *context)
>  {
>  	if (!postProcessor_)
>  		return 0;
> @@ -118,7 +119,8 @@ int CameraStream::process(const FrameBuffer *source,
>  		return -EINVAL;
>  	}
>  
> -	return postProcessor_->process(source, &dest, requestMetadata, resultMetadata);
> +	return postProcessor_->process(source, &dest, requestMetadata,
> +				       resultMetadata, context);
>  }
>  
>  FrameBuffer *CameraStream::getBuffer()
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 5c232cb6..d589f699 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -23,6 +23,8 @@ class CameraDevice;
>  class CameraMetadata;
>  class PostProcessor;
>  
> +struct Camera3RequestDescriptor;
> +
>  class CameraStream
>  {
>  public:
> @@ -121,7 +123,8 @@ public:
>  	int process(const libcamera::FrameBuffer *source,
>  		    buffer_handle_t camera3Dest,
>  		    const CameraMetadata &requestMetadata,
> -		    CameraMetadata *resultMetadata);
> +		    CameraMetadata *resultMetadata,
> +		    const Camera3RequestDescriptor *context);
>  	libcamera::FrameBuffer *getBuffer();
>  	void putBuffer(libcamera::FrameBuffer *buffer);
>  
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index cb45f86b..741eeb21 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -100,7 +100,8 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer *source,
>  int PostProcessorJpeg::process(const FrameBuffer *source,
>  			       CameraBuffer *destination,
>  			       const CameraMetadata &requestMetadata,
> -			       CameraMetadata *resultMetadata)
> +			       CameraMetadata *resultMetadata,
> +			       const Camera3RequestDescriptor *context)
>  {
>  	if (!encoder_)
>  		return 0;
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index c4b2e9ef..72c38fdb 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -25,7 +25,8 @@ public:
>  	int process(const libcamera::FrameBuffer *source,
>  		    CameraBuffer *destination,
>  		    const CameraMetadata &requestMetadata,
> -		    CameraMetadata *resultMetadata) override;
> +		    CameraMetadata *resultMetadata,
> +		    const Camera3RequestDescriptor *context) override;
>  
>  private:
>  	void generateThumbnail(const libcamera::FrameBuffer *source,
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index 61dfb6d4..1213e7e6 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -14,6 +14,8 @@
>  
>  class CameraMetadata;
>  
> +struct Camera3RequestDescriptor;
> +
>  class PostProcessor
>  {
>  public:
> @@ -24,7 +26,8 @@ public:
>  	virtual int process(const libcamera::FrameBuffer *source,
>  			    CameraBuffer *destination,
>  			    const CameraMetadata &requestMetadata,
> -			    CameraMetadata *resultMetadata) = 0;
> +			    CameraMetadata *resultMetadata,
> +			    const Camera3RequestDescriptor *context) = 0;
>  };
>  
>  #endif /* __ANDROID_POST_PROCESSOR_H__ */
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index 0a874886..13a78abe 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -52,7 +52,8 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
>  int PostProcessorYuv::process(const FrameBuffer *source,
>  			      CameraBuffer *destination,
>  			      [[maybe_unused]] const CameraMetadata &requestMetadata,
> -			      [[maybe_unused]] CameraMetadata *metadata)
> +			      [[maybe_unused]] CameraMetadata *metadata,
> +			      [[maybe_unused]] const Camera3RequestDescriptor *context)
>  {
>  	if (!isValidBuffers(source, *destination))
>  		return -EINVAL;
> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> index 44a04113..38a30f91 100644
> --- a/src/android/yuv/post_processor_yuv.h
> +++ b/src/android/yuv/post_processor_yuv.h
> @@ -23,7 +23,8 @@ public:
>  	int process(const libcamera::FrameBuffer *source,
>  		    CameraBuffer *destination,
>  		    const CameraMetadata &requestMetadata,
> -		    CameraMetadata *metadata) override;
> +		    CameraMetadata *metadata,
> +		    const Camera3RequestDescriptor *context) override;

I'd name the parameter "request" instead of "context", "context" is more
vague.

With those changes,

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

>  
>  private:
>  	bool isValidBuffers(const libcamera::FrameBuffer *source,

-- 
Regards,

Laurent Pinchart