[{"id":20334,"web_url":"https://patchwork.libcamera.org/comment/20334/","msgid":"","date":"2021-10-20T21:46:21","subject":"Re: [libcamera-devel] [PATCH v5 2/4] android: post_processor: Drop\n\treturn value for process()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Wed, Oct 20, 2021 at 04:12:10PM +0530, Umang Jain wrote:\n> PostProcessor::process() is invoked by CameraStream class\n> in case any post-processing is required for the camera stream.\n> The failure or success is checked via the value returned by\n> CameraStream::process().\n> \n> Now that the post-processor notifies about the post-processing\n> completion operation, we can drop the return value of\n> PostProcessor::process(). The status of post-processing is passed\n> to CameraDevice::streamProcessingComplete() by the\n> PostProcessor::processComplete signal's slot.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/android/camera_stream.cpp | 4 +++-\n> src/android/jpeg/post_processor_jpeg.cpp | 12 +++++-------\n> src/android/jpeg/post_processor_jpeg.h | 6 +++---\n> src/android/post_processor.h | 6 +++---\n> src/android/yuv/post_processor_yuv.cpp | 14 ++++++--------\n> src/android/yuv/post_processor_yuv.h | 6 +++---\n> 6 files changed, 23 insertions(+), 25 deletions(-)\n> \n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 04cbef8c..b3cb06a2 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -191,7 +191,9 @@ int CameraStream::process(const FrameBuffer &source,\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\treturn postProcessor_->process(source, &destBuffer, request);\n> +\tpostProcessor_->process(source, &destBuffer, request);\n> +\n> +\treturn 0;\n> }\n> \n> FrameBuffer *CameraStream::getBuffer()\n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index a001fede..955ea21d 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -98,12 +98,12 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,\n> \t}\n> }\n> \n> -int PostProcessorJpeg::process(const FrameBuffer &source,\n> -\t\t\t CameraBuffer *destination,\n> -\t\t\t Camera3RequestDescriptor *request)\n> +void PostProcessorJpeg::process(const FrameBuffer &source,\n> +\t\t\t\tCameraBuffer *destination,\n> +\t\t\t\tCamera3RequestDescriptor *request)\n> {\n> \tif (!encoder_)\n> -\t\treturn 0;\n> +\t\treturn;\n\nIf this happens, the request will be lost. As it's not supposed to\nhappen, I'd replace the check with\n\n\tASSERT(encoder_);\n\nin a patch before 1/4.\n\nReviewed-by: Laurent Pinchart \n\n> \n> \tASSERT(destination->numPlanes() == 1);\n> \n> @@ -199,7 +199,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> \tif (jpeg_size < 0) {\n> \t\tLOG(JPEG, Error) << \"Failed to encode stream image\";\n> \t\tprocessComplete.emit(request, PostProcessor::Status::Error);\n> -\t\treturn jpeg_size;\n> +\t\treturn;\n> \t}\n> \n> \t/* Fill in the JPEG blob header. */\n> @@ -213,6 +213,4 @@ int PostProcessorJpeg::process(const FrameBuffer &source,\n> \t/* Update the JPEG result Metadata. */\n> \tresultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size);\n> \tprocessComplete.emit(request, PostProcessor::Status::Success);\n> -\n> -\treturn 0;\n> }\n> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h\n> index 0184d77e..5dab14e1 100644\n> --- a/src/android/jpeg/post_processor_jpeg.h\n> +++ b/src/android/jpeg/post_processor_jpeg.h\n> @@ -22,9 +22,9 @@ public:\n> \n> \tint configure(const libcamera::StreamConfiguration &incfg,\n> \t\t const libcamera::StreamConfiguration &outcfg) override;\n> -\tint process(const libcamera::FrameBuffer &source,\n> -\t\t CameraBuffer *destination,\n> -\t\t Camera3RequestDescriptor *request) override;\n> +\tvoid process(const libcamera::FrameBuffer &source,\n> +\t\t CameraBuffer *destination,\n> +\t\t Camera3RequestDescriptor *request) override;\n> \n> private:\n> \tvoid generateThumbnail(const libcamera::FrameBuffer &source,\n> diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> index 14f5e8c7..7e78cfec 100644\n> --- a/src/android/post_processor.h\n> +++ b/src/android/post_processor.h\n> @@ -28,9 +28,9 @@ public:\n> \n> \tvirtual int configure(const libcamera::StreamConfiguration &inCfg,\n> \t\t\t const libcamera::StreamConfiguration &outCfg) = 0;\n> -\tvirtual int process(const libcamera::FrameBuffer &source,\n> -\t\t\t CameraBuffer *destination,\n> -\t\t\t Camera3RequestDescriptor *request) = 0;\n> +\tvirtual void process(const libcamera::FrameBuffer &source,\n> +\t\t\t CameraBuffer *destination,\n> +\t\t\t Camera3RequestDescriptor *request) = 0;\n> \n> \tlibcamera::Signal processComplete;\n> };\n> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp\n> index fd364741..b167f057 100644\n> --- a/src/android/yuv/post_processor_yuv.cpp\n> +++ b/src/android/yuv/post_processor_yuv.cpp\n> @@ -49,20 +49,20 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,\n> \treturn 0;\n> }\n> \n> -int PostProcessorYuv::process(const FrameBuffer &source,\n> -\t\t\t CameraBuffer *destination,\n> -\t\t\t Camera3RequestDescriptor *request)\n> +void PostProcessorYuv::process(const FrameBuffer &source,\n> +\t\t\t CameraBuffer *destination,\n> +\t\t\t Camera3RequestDescriptor *request)\n> {\n> \tif (!isValidBuffers(source, *destination)) {\n> \t\tprocessComplete.emit(request, PostProcessor::Status::Error);\n> -\t\treturn -EINVAL;\n> +\t\treturn;\n> \t}\n> \n> \tconst MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read);\n> \tif (!sourceMapped.isValid()) {\n> \t\tLOG(YUV, Error) << \"Failed to mmap camera frame buffer\";\n> \t\tprocessComplete.emit(request, PostProcessor::Status::Error);\n> -\t\treturn -EINVAL;\n> +\t\treturn;\n> \t}\n> \n> \tint ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(),\n> @@ -80,12 +80,10 @@ int PostProcessorYuv::process(const FrameBuffer &source,\n> \tif (ret) {\n> \t\tLOG(YUV, Error) << \"Failed NV12 scaling: \" << ret;\n> \t\tprocessComplete.emit(request, PostProcessor::Status::Error);\n> -\t\treturn -EINVAL;\n> +\t\treturn;\n> \t}\n> \n> \tprocessComplete.emit(request, PostProcessor::Status::Success);\n> -\n> -\treturn 0;\n> }\n> \n> bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,\n> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h\n> index a4e0ff5d..1278843b 100644\n> --- a/src/android/yuv/post_processor_yuv.h\n> +++ b/src/android/yuv/post_processor_yuv.h\n> @@ -18,9 +18,9 @@ public:\n> \n> \tint configure(const libcamera::StreamConfiguration &incfg,\n> \t\t const libcamera::StreamConfiguration &outcfg) override;\n> -\tint process(const libcamera::FrameBuffer &source,\n> -\t\t CameraBuffer *destination,\n> -\t\t Camera3RequestDescriptor *request) override;\n> +\tvoid process(const libcamera::FrameBuffer &source,\n> +\t\t CameraBuffer *destination,\n> +\t\t Camera3RequestDescriptor *request) override;\n> \n> private:\n> \tbool isValidBuffers(const libcamera::FrameBuffer &source,","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 55275BDB1C\n\tfor ;\n\tWed, 20 Oct 2021 21:46:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B7F0E68F59;\n\tWed, 20 Oct 2021 23:46:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E98C6023A\n\tfor ;\n\tWed, 20 Oct 2021 23:46:41 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BFD762A5;\n\tWed, 20 Oct 2021 23:46:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qTpbhTPe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634766401;\n\tbh=flZAaASez16RtstaZAYTfNQqYy+C5U8/1qsasVBU59o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qTpbhTPeuS33bP+ck3wGDo7Lnj/r2CqgsFP93dkMfB6g9Dqhks8LAskBb3YxOynLw\n\tYBi2lEBQd9B319szAD4weTWv8lvMTZcF3NvJz0dK3MxE7hoqcBeRtAsFamd+h2IkXc\n\tbSddRCkNc+7LTG9BC+aZau8tczay3mjLquQvsnVM=","Date":"Thu, 21 Oct 2021 00:46:21 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20211020104212.121743-1-umang.jain@ideasonboard.com>\n\t<20211020104212.121743-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211020104212.121743-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/4] android: post_processor: Drop\n\treturn value for process()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]