[libcamera-devel] [PATCH v2 5/9] android: camera_stream: Query post-processing status

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Fri, Sep 10, 2021 at 12:36:34PM +0530, Umang Jain wrote:
> CameraStream takes care of any post-processing required via the
> CameraStream::process(). Since the post-processor will be moved
> to a separate thread (in subsequent commits), the caller of
> PostProcessor::process() will notify the associated CameraStream
> instance about the post-processing completion via processComplete
> signal. The CameraStream class should handle this event to query
> the status and take next steps of reporting it back to upper layers
> (i.e. CameraDevice).
> 
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/camera_stream.cpp | 13 +++++++++++++
>  src/android/camera_stream.h   | 13 ++++++++++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index d1c54797..996779c4 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -81,6 +81,9 @@ int CameraStream::configure()
>  		int ret = postProcessor_->configure(configuration(), output);
>  		if (ret)
>  			return ret;
> +
> +		postProcessor_->processComplete.connect(
> +			this, &CameraStream::postProcessingComplete);
>  	}
>  
>  	if (allocator_) {
> @@ -123,6 +126,16 @@ int CameraStream::process(const FrameBuffer *source,
>  				       resultMetadata, context);
>  }
>  
> +void CameraStream::postProcessingComplete(PostProcessor::Status status,
> +					  const Camera3RequestDescriptor *context)
> +{
> +	ProcessStatus processStatus;
> +	if (status == PostProcessor::Status::Succeeded)
> +		processStatus = ProcessStatus::Succeeded;
> +	else
> +		processStatus = ProcessStatus::Failed;
> +}

I'd squash this patch with 6/9, it doesn't do much on its own, and fails
to compile due to the unused processStatus variable.

> +
>  FrameBuffer *CameraStream::getBuffer()
>  {
>  	if (!allocator_)
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index d589f699..be076995 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -13,15 +13,18 @@
>  
>  #include <hardware/camera3.h>
>  
> +#include <libcamera/base/signal.h>

I don't think you need the signal header here.

> +
>  #include <libcamera/camera.h>
>  #include <libcamera/framebuffer.h>
>  #include <libcamera/framebuffer_allocator.h>
>  #include <libcamera/geometry.h>
>  #include <libcamera/pixel_format.h>
>  
> +#include "post_processor.h"
> +
>  class CameraDevice;
>  class CameraMetadata;
> -class PostProcessor;
>  
>  struct Camera3RequestDescriptor;
>  
> @@ -128,7 +131,15 @@ public:
>  	libcamera::FrameBuffer *getBuffer();
>  	void putBuffer(libcamera::FrameBuffer *buffer);
>  
> +	enum ProcessStatus {
> +		Succeeded,
> +		Failed,
> +	};

Do we need this enum, could we use PostProcessor::Status everywhere ?

> +
>  private:
> +	void postProcessingComplete(PostProcessor::Status status,
> +				    const Camera3RequestDescriptor *context);
> +
>  	CameraDevice *const cameraDevice_;
>  	const libcamera::CameraConfiguration *config_;
>  	const Type type_;

-- 
Regards,

Laurent Pinchart