[libcamera-devel,v5,2/4] android: post_processor: Drop return value for process()
diff mbox series

Message ID 20211020104212.121743-3-umang.jain@ideasonboard.com
State Superseded
Headers show
Series
  • Async Post Processor
Related show

Commit Message

Umang Jain Oct. 20, 2021, 10:42 a.m. UTC 
PostProcessor::process() is invoked by CameraStream class
in case any post-processing is required for the camera stream.
The failure or success is checked via the value returned by
CameraStream::process().

Now that the post-processor notifies about the post-processing
completion operation, we can drop the return value of
PostProcessor::process(). The status of post-processing is passed
to CameraDevice::streamProcessingComplete() by the
PostProcessor::processComplete signal's slot.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/android/camera_stream.cpp            |  4 +++-
 src/android/jpeg/post_processor_jpeg.cpp | 12 +++++-------
 src/android/jpeg/post_processor_jpeg.h   |  6 +++---
 src/android/post_processor.h             |  6 +++---
 src/android/yuv/post_processor_yuv.cpp   | 14 ++++++--------
 src/android/yuv/post_processor_yuv.h     |  6 +++---
 6 files changed, 23 insertions(+), 25 deletions(-)

Comments

Laurent Pinchart Oct. 20, 2021, 9:46 p.m. UTC | #1 
Hi Umang,

Thank you for the patch.

On Wed, Oct 20, 2021 at 04:12:10PM +0530, Umang Jain wrote:
> PostProcessor::process() is invoked by CameraStream class
> in case any post-processing is required for the camera stream.
> The failure or success is checked via the value returned by
> CameraStream::process().
> 
> Now that the post-processor notifies about the post-processing
> completion operation, we can drop the return value of
> PostProcessor::process(). The status of post-processing is passed
> to CameraDevice::streamProcessingComplete() by the
> PostProcessor::processComplete signal's slot.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/android/camera_stream.cpp            |  4 +++-
>  src/android/jpeg/post_processor_jpeg.cpp | 12 +++++-------
>  src/android/jpeg/post_processor_jpeg.h   |  6 +++---
>  src/android/post_processor.h             |  6 +++---
>  src/android/yuv/post_processor_yuv.cpp   | 14 ++++++--------
>  src/android/yuv/post_processor_yuv.h     |  6 +++---
>  6 files changed, 23 insertions(+), 25 deletions(-)
> 
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 04cbef8c..b3cb06a2 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -191,7 +191,9 @@ int CameraStream::process(const FrameBuffer &source,
>  		return -EINVAL;
>  	}
>  
> -	return postProcessor_->process(source, &destBuffer, request);
> +	postProcessor_->process(source, &destBuffer, request);
> +
> +	return 0;
>  }
>  
>  FrameBuffer *CameraStream::getBuffer()
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index a001fede..955ea21d 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -98,12 +98,12 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
>  	}
>  }
>  
> -int PostProcessorJpeg::process(const FrameBuffer &source,
> -			       CameraBuffer *destination,
> -			       Camera3RequestDescriptor *request)
> +void PostProcessorJpeg::process(const FrameBuffer &source,
> +				CameraBuffer *destination,
> +				Camera3RequestDescriptor *request)
>  {
>  	if (!encoder_)
> -		return 0;
> +		return;

If this happens, the request will be lost. As it's not supposed to
happen, I'd replace the check with

	ASSERT(encoder_);

in a patch before 1/4.

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

>  
>  	ASSERT(destination->numPlanes() == 1);
>  
> @@ -199,7 +199,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  	if (jpeg_size < 0) {
>  		LOG(JPEG, Error) << "Failed to encode stream image";
>  		processComplete.emit(request, PostProcessor::Status::Error);
> -		return jpeg_size;
> +		return;
>  	}
>  
>  	/* Fill in the JPEG blob header. */
> @@ -213,6 +213,4 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
>  	/* Update the JPEG result Metadata. */
>  	resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size);
>  	processComplete.emit(request, PostProcessor::Status::Success);
> -
> -	return 0;
>  }
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index 0184d77e..5dab14e1 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -22,9 +22,9 @@ public:
>  
>  	int configure(const libcamera::StreamConfiguration &incfg,
>  		      const libcamera::StreamConfiguration &outcfg) override;
> -	int process(const libcamera::FrameBuffer &source,
> -		    CameraBuffer *destination,
> -		    Camera3RequestDescriptor *request) override;
> +	void process(const libcamera::FrameBuffer &source,
> +		     CameraBuffer *destination,
> +		     Camera3RequestDescriptor *request) override;
>  
>  private:
>  	void generateThumbnail(const libcamera::FrameBuffer &source,
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index 14f5e8c7..7e78cfec 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -28,9 +28,9 @@ public:
>  
>  	virtual int configure(const libcamera::StreamConfiguration &inCfg,
>  			      const libcamera::StreamConfiguration &outCfg) = 0;
> -	virtual int process(const libcamera::FrameBuffer &source,
> -			    CameraBuffer *destination,
> -			    Camera3RequestDescriptor *request) = 0;
> +	virtual void process(const libcamera::FrameBuffer &source,
> +			     CameraBuffer *destination,
> +			     Camera3RequestDescriptor *request) = 0;
>  
>  	libcamera::Signal<Camera3RequestDescriptor *, Status> processComplete;
>  };
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index fd364741..b167f057 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -49,20 +49,20 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
>  	return 0;
>  }
>  
> -int PostProcessorYuv::process(const FrameBuffer &source,
> -			      CameraBuffer *destination,
> -			      Camera3RequestDescriptor *request)
> +void PostProcessorYuv::process(const FrameBuffer &source,
> +			       CameraBuffer *destination,
> +			       Camera3RequestDescriptor *request)
>  {
>  	if (!isValidBuffers(source, *destination)) {
>  		processComplete.emit(request, PostProcessor::Status::Error);
> -		return -EINVAL;
> +		return;
>  	}
>  
>  	const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read);
>  	if (!sourceMapped.isValid()) {
>  		LOG(YUV, Error) << "Failed to mmap camera frame buffer";
>  		processComplete.emit(request, PostProcessor::Status::Error);
> -		return -EINVAL;
> +		return;
>  	}
>  
>  	int ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(),
> @@ -80,12 +80,10 @@ int PostProcessorYuv::process(const FrameBuffer &source,
>  	if (ret) {
>  		LOG(YUV, Error) << "Failed NV12 scaling: " << ret;
>  		processComplete.emit(request, PostProcessor::Status::Error);
> -		return -EINVAL;
> +		return;
>  	}
>  
>  	processComplete.emit(request, PostProcessor::Status::Success);
> -
> -	return 0;
>  }
>  
>  bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> index a4e0ff5d..1278843b 100644
> --- a/src/android/yuv/post_processor_yuv.h
> +++ b/src/android/yuv/post_processor_yuv.h
> @@ -18,9 +18,9 @@ public:
>  
>  	int configure(const libcamera::StreamConfiguration &incfg,
>  		      const libcamera::StreamConfiguration &outcfg) override;
> -	int process(const libcamera::FrameBuffer &source,
> -		    CameraBuffer *destination,
> -		    Camera3RequestDescriptor *request) override;
> +	void process(const libcamera::FrameBuffer &source,
> +		     CameraBuffer *destination,
> +		     Camera3RequestDescriptor *request) override;
>  
>  private:
>  	bool isValidBuffers(const libcamera::FrameBuffer &source,

Patch
diff mbox series 

diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 04cbef8c..b3cb06a2 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -191,7 +191,9 @@  int CameraStream::process(const FrameBuffer &source,
 		return -EINVAL;
 	}
 
-	return postProcessor_->process(source, &destBuffer, request);
+	postProcessor_->process(source, &destBuffer, request);
+
+	return 0;
 }
 
 FrameBuffer *CameraStream::getBuffer()
diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
index a001fede..955ea21d 100644
--- a/src/android/jpeg/post_processor_jpeg.cpp
+++ b/src/android/jpeg/post_processor_jpeg.cpp
@@ -98,12 +98,12 @@  void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
 	}
 }
 
-int PostProcessorJpeg::process(const FrameBuffer &source,
-			       CameraBuffer *destination,
-			       Camera3RequestDescriptor *request)
+void PostProcessorJpeg::process(const FrameBuffer &source,
+				CameraBuffer *destination,
+				Camera3RequestDescriptor *request)
 {
 	if (!encoder_)
-		return 0;
+		return;
 
 	ASSERT(destination->numPlanes() == 1);
 
@@ -199,7 +199,7 @@  int PostProcessorJpeg::process(const FrameBuffer &source,
 	if (jpeg_size < 0) {
 		LOG(JPEG, Error) << "Failed to encode stream image";
 		processComplete.emit(request, PostProcessor::Status::Error);
-		return jpeg_size;
+		return;
 	}
 
 	/* Fill in the JPEG blob header. */
@@ -213,6 +213,4 @@  int PostProcessorJpeg::process(const FrameBuffer &source,
 	/* Update the JPEG result Metadata. */
 	resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size);
 	processComplete.emit(request, PostProcessor::Status::Success);
-
-	return 0;
 }
diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
index 0184d77e..5dab14e1 100644
--- a/src/android/jpeg/post_processor_jpeg.h
+++ b/src/android/jpeg/post_processor_jpeg.h
@@ -22,9 +22,9 @@  public:
 
 	int configure(const libcamera::StreamConfiguration &incfg,
 		      const libcamera::StreamConfiguration &outcfg) override;
-	int process(const libcamera::FrameBuffer &source,
-		    CameraBuffer *destination,
-		    Camera3RequestDescriptor *request) override;
+	void process(const libcamera::FrameBuffer &source,
+		     CameraBuffer *destination,
+		     Camera3RequestDescriptor *request) override;
 
 private:
 	void generateThumbnail(const libcamera::FrameBuffer &source,
diff --git a/src/android/post_processor.h b/src/android/post_processor.h
index 14f5e8c7..7e78cfec 100644
--- a/src/android/post_processor.h
+++ b/src/android/post_processor.h
@@ -28,9 +28,9 @@  public:
 
 	virtual int configure(const libcamera::StreamConfiguration &inCfg,
 			      const libcamera::StreamConfiguration &outCfg) = 0;
-	virtual int process(const libcamera::FrameBuffer &source,
-			    CameraBuffer *destination,
-			    Camera3RequestDescriptor *request) = 0;
+	virtual void process(const libcamera::FrameBuffer &source,
+			     CameraBuffer *destination,
+			     Camera3RequestDescriptor *request) = 0;
 
 	libcamera::Signal<Camera3RequestDescriptor *, Status> processComplete;
 };
diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
index fd364741..b167f057 100644
--- a/src/android/yuv/post_processor_yuv.cpp
+++ b/src/android/yuv/post_processor_yuv.cpp
@@ -49,20 +49,20 @@  int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
 	return 0;
 }
 
-int PostProcessorYuv::process(const FrameBuffer &source,
-			      CameraBuffer *destination,
-			      Camera3RequestDescriptor *request)
+void PostProcessorYuv::process(const FrameBuffer &source,
+			       CameraBuffer *destination,
+			       Camera3RequestDescriptor *request)
 {
 	if (!isValidBuffers(source, *destination)) {
 		processComplete.emit(request, PostProcessor::Status::Error);
-		return -EINVAL;
+		return;
 	}
 
 	const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read);
 	if (!sourceMapped.isValid()) {
 		LOG(YUV, Error) << "Failed to mmap camera frame buffer";
 		processComplete.emit(request, PostProcessor::Status::Error);
-		return -EINVAL;
+		return;
 	}
 
 	int ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(),
@@ -80,12 +80,10 @@  int PostProcessorYuv::process(const FrameBuffer &source,
 	if (ret) {
 		LOG(YUV, Error) << "Failed NV12 scaling: " << ret;
 		processComplete.emit(request, PostProcessor::Status::Error);
-		return -EINVAL;
+		return;
 	}
 
 	processComplete.emit(request, PostProcessor::Status::Success);
-
-	return 0;
 }
 
 bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
index a4e0ff5d..1278843b 100644
--- a/src/android/yuv/post_processor_yuv.h
+++ b/src/android/yuv/post_processor_yuv.h
@@ -18,9 +18,9 @@  public:
 
 	int configure(const libcamera::StreamConfiguration &incfg,
 		      const libcamera::StreamConfiguration &outcfg) override;
-	int process(const libcamera::FrameBuffer &source,
-		    CameraBuffer *destination,
-		    Camera3RequestDescriptor *request) override;
+	void process(const libcamera::FrameBuffer &source,
+		     CameraBuffer *destination,
+		     Camera3RequestDescriptor *request) override;
 
 private:
 	bool isValidBuffers(const libcamera::FrameBuffer &source,