[libcamera-devel] [PATCH v5 2/4] android: post_processor: Drop return value for process()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Oct 20 23:46:21 CEST 2021
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 at 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 at 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,
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list