[{"id":20485,"web_url":"https://patchwork.libcamera.org/comment/20485/","msgid":"","date":"2021-10-25T21:58:11","subject":"Re: [libcamera-devel] [PATCH v7 5/7] android: Track and notify post\n\tprocessing of streams","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 Tue, Oct 26, 2021 at 02:08:31AM +0530, Umang Jain wrote:\n> Notify that the post processing for a request has been completed,\n> via a signal. The signal is emitted with a context pointer along with\n> status of the buffer. The function CameraDevice::streamProcessingComplete()\n> will finally set the status on the request descriptor and complete the\n> descriptor if all the streams requiring post processing are completed.\n> If buffer status obtained is in error state, notify the status to the\n> framework and set the overall error status on the descriptor via\n> setBufferStatus().\n> \n> We need to track the number of streams requiring post-processing\n> per Camera3RequestDescriptor (i.e. per capture request). Introduce\n> a std::map<> to track the post-processing of streams. The nodes\n> are dropped from the map when a particular stream post processing\n> is completed (or on error paths). A std::map is selected for tracking\n> post-processing requests, since we will move post-processing to be\n> asynchronous in subsequent commits. A vector or queue will not be\n> suitable then as the sequential order of post-processing completion\n> of various requests won't be guaranteed then.\n> \n> A streamsProcessMutex_ has been introduced here as well, which will be\n> applicable to guard access to descriptor's pendingStreamsToProcess_ when\n> post-processing is moved to be asynchronous in subsequent commits.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/android/camera_device.cpp | 87 +++++++++++++++++-------\n> src/android/camera_device.h | 4 ++\n> src/android/camera_request.h | 6 ++\n> src/android/camera_stream.cpp | 15 ++++\n> src/android/jpeg/post_processor_jpeg.cpp | 2 +\n> src/android/post_processor.h | 9 +++\n> src/android/yuv/post_processor_yuv.cpp | 8 ++-\n> 7 files changed, 106 insertions(+), 25 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 9155728a..3ded0f7e 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -926,6 +926,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t\t\t * Request.\n> \t\t\t */\n> \t\t\tLOG(HAL, Debug) << ss.str() << \" (mapped)\";\n> +\n> +\t\t\tdescriptor->pendingStreamsToProcess_.insert(\n> +\t\t\t\t{ cameraStream, &buffer });\n> \t\t\tcontinue;\n> \n> \t\tcase CameraStream::Type::Direct:\n> @@ -955,6 +958,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> \t\t\tframeBuffer = cameraStream->getBuffer();\n> \t\t\tbuffer.internalBuffer = frameBuffer;\n> \t\t\tLOG(HAL, Debug) << ss.str() << \" (internal)\";\n> +\n> +\t\t\tdescriptor->pendingStreamsToProcess_.insert(\n> +\t\t\t\t{ cameraStream, &buffer });\n> \t\t\tbreak;\n> \t\t}\n> \n> @@ -1118,42 +1124,42 @@ void CameraDevice::requestComplete(Request *request)\n> \t}\n> \n> \t/* Handle post-processing. */\n> -\tfor (auto &buffer : descriptor->buffers_) {\n> -\t\tCameraStream *stream = buffer.stream;\n> -\n> -\t\tif (stream->type() == CameraStream::Type::Direct)\n> -\t\t\tcontinue;\n> +\t/*\n> +\t * \\todo Protect the loop below with streamProcessMutex_ when post\n> +\t * processor runs asynchronously.\n> +\t */\n> +\tauto iter = descriptor->pendingStreamsToProcess_.begin();\n> +\twhile (iter != descriptor->pendingStreamsToProcess_.end()) {\n> +\t\tCameraStream *stream = iter->first;\n> +\t\tCamera3RequestDescriptor::StreamBuffer *buffer = iter->second;\n> \n> \t\tFrameBuffer *src = request->findBuffer(stream->stream());\n> \t\tif (!src) {\n> \t\t\tLOG(HAL, Error) << \"Failed to find a source stream buffer\";\n> -\t\t\tbuffer.status = Camera3RequestDescriptor::Status::Error;\n> -\t\t\tnotifyError(descriptor->frameNumber_, stream->camera3Stream(),\n> -\t\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n> -\t\t\tdescriptor->status_ = Camera3RequestDescriptor::Status::Error;\n> +\t\t\tsetBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);\n> +\t\t\titer = descriptor->pendingStreamsToProcess_.erase(iter);\n> \t\t\tcontinue;\n> \t\t}\n> \n> -\t\tbuffer.srcBuffer = src;\n> -\n> -\t\tint ret = stream->process(&buffer);\n> -\n> -\t\t/*\n> -\t\t * If the framebuffer is internal to CameraStream return it back\n> -\t\t * now that we're done processing it.\n> -\t\t */\n> -\t\tif (buffer.internalBuffer)\n> -\t\t\tstream->putBuffer(buffer.internalBuffer);\n> +\t\tbuffer->srcBuffer = src;\n> \n> +\t\t++iter;\n> +\t\tint ret = stream->process(buffer);\n> \t\tif (ret) {\n> -\t\t\tbuffer.status = Camera3RequestDescriptor::Status::Error;\n> -\t\t\tnotifyError(descriptor->frameNumber_, stream->camera3Stream(),\n> -\t\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n> -\t\t\tdescriptor->status_ = Camera3RequestDescriptor::Status::Error;\n> +\t\t\tsetBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);\n> +\t\t\tdescriptor->pendingStreamsToProcess_.erase(stream);\n> +\n> +\t\t\t/*\n> +\t\t\t * If the framebuffer is internal to CameraStream return\n> +\t\t\t * it back now that we're done processing it.\n> +\t\t\t */\n> +\t\t\tif (buffer->internalBuffer)\n> +\t\t\t\tstream->putBuffer(buffer->internalBuffer);\n> \t\t}\n> \t}\n> \n> -\tcompleteDescriptor(descriptor);\n> +\tif (descriptor->pendingStreamsToProcess_.empty())\n> +\t\tcompleteDescriptor(descriptor);\n> }\n> \n> void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)\n> @@ -1208,6 +1214,39 @@ void CameraDevice::sendCaptureResults()\n> \t}\n> }\n> \n> +void CameraDevice::setBufferStatus(Camera3RequestDescriptor::StreamBuffer &streamBuffer,\n> +\t\t\t\t Camera3RequestDescriptor::Status status)\n> +{\n> +\tstreamBuffer.status = status;\n> +\tif (status != Camera3RequestDescriptor::Status::Success) {\n> +\t\tnotifyError(streamBuffer.request->frameNumber_,\n> +\t\t\t streamBuffer.stream->camera3Stream(),\n> +\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n> +\n> +\t\t/* Also set error status on entire request descriptor. */\n> +\t\tstreamBuffer.request->status_ =\n> +\t\t\tCamera3RequestDescriptor::Status::Error;\n> +\t}\n> +}\n> +\n> +void CameraDevice::streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> +\t\t\t\t\t Camera3RequestDescriptor::Status status)\n> +{\n> +\tsetBufferStatus(*streamBuffer, status);\n> +\n> +\t/*\n> +\t * If the framebuffer is internal to CameraStream return it back now\n> +\t * that we're done processing it.\n> +\t */\n> +\tif (streamBuffer->internalBuffer)\n> +\t\tstreamBuffer->stream->putBuffer(streamBuffer->internalBuffer);\n> +\n> +\tCamera3RequestDescriptor *request = streamBuffer->request;\n> +\tMutexLocker locker(request->streamsProcessMutex_);\n> +\n> +\trequest->pendingStreamsToProcess_.erase(streamBuffer->stream);\n> +}\n> +\n> std::string CameraDevice::logPrefix() const\n> {\n> \treturn \"'\" + camera_->id() + \"'\";\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index e544f2bd..2a414020 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -66,6 +66,8 @@ public:\n> \tint configureStreams(camera3_stream_configuration_t *stream_list);\n> \tint processCaptureRequest(camera3_capture_request_t *request);\n> \tvoid requestComplete(libcamera::Request *request);\n> +\tvoid streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *bufferStream,\n> +\t\t\t\t Camera3RequestDescriptor::Status status);\n> \n> protected:\n> \tstd::string logPrefix() const override;\n> @@ -95,6 +97,8 @@ private:\n> \tint processControls(Camera3RequestDescriptor *descriptor);\n> \tvoid completeDescriptor(Camera3RequestDescriptor *descriptor);\n> \tvoid sendCaptureResults();\n> +\tvoid setBufferStatus(Camera3RequestDescriptor::StreamBuffer &buffer,\n> +\t\t\t Camera3RequestDescriptor::Status status);\n> \tstd::unique_ptr getResultMetadata(\n> \t\tconst Camera3RequestDescriptor &descriptor) const;\n> \n> diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> index c7fda00d..c28f7942 100644\n> --- a/src/android/camera_request.h\n> +++ b/src/android/camera_request.h\n> @@ -7,7 +7,9 @@\n> #ifndef __ANDROID_CAMERA_REQUEST_H__\n> #define __ANDROID_CAMERA_REQUEST_H__\n> \n> +#include \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -43,6 +45,10 @@ public:\n> \t\tCamera3RequestDescriptor *request;\n> \t};\n> \n> +\t/* Keeps track of streams requiring post-processing. */\n> +\tstd::map pendingStreamsToProcess_;\n> +\tstd::mutex streamsProcessMutex_;\n> +\n> \tCamera3RequestDescriptor(libcamera::Camera *camera,\n> \t\t\t\t const camera3_capture_request_t *camera3Request);\n> \t~Camera3RequestDescriptor();\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 282b19b0..dac216d6 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -22,6 +22,7 @@\n> #include \"camera_capabilities.h\"\n> #include \"camera_device.h\"\n> #include \"camera_metadata.h\"\n> +#include \"post_processor.h\"\n> \n> using namespace libcamera;\n> \n> @@ -97,6 +98,20 @@ int CameraStream::configure()\n> \t\tint ret = postProcessor_->configure(configuration(), output);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> +\n> +\t\tpostProcessor_->processComplete.connect(\n> +\t\t\tthis, [&](Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> +\t\t\t\t PostProcessor::Status status) {\n> +\t\t\t\tCamera3RequestDescriptor::Status bufferStatus;\n> +\n> +\t\t\t\tif (status == PostProcessor::Status::Success)\n> +\t\t\t\t\tbufferStatus = Camera3RequestDescriptor::Status::Success;\n> +\t\t\t\telse\n> +\t\t\t\t\tbufferStatus = Camera3RequestDescriptor::Status::Error;\n> +\n> +\t\t\t\tcameraDevice_->streamProcessingComplete(streamBuffer,\n> +\t\t\t\t\t\t\t\t\tbufferStatus);\n> +\t\t\t});\n> \t}\n> \n> \tif (type_ == Type::Internal) {\n> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> index 240e29f6..5e8c61fc 100644\n> --- a/src/android/jpeg/post_processor_jpeg.cpp\n> +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> @@ -198,6 +198,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf\n> \t\t\t\t\t exif.data(), quality);\n> \tif (jpeg_size < 0) {\n> \t\tLOG(JPEG, Error) << \"Failed to encode stream image\";\n> +\t\tprocessComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> \t\treturn jpeg_size;\n> \t}\n> \n> @@ -211,6 +212,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf\n> \n> \t/* Update the JPEG result Metadata. */\n> \tresultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size);\n> +\tprocessComplete.emit(streamBuffer, PostProcessor::Status::Success);\n> \n> \treturn 0;\n> }\n> diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> index 128161c8..4ac74fcf 100644\n> --- a/src/android/post_processor.h\n> +++ b/src/android/post_processor.h\n> @@ -7,6 +7,8 @@\n> #ifndef __ANDROID_POST_PROCESSOR_H__\n> #define __ANDROID_POST_PROCESSOR_H__\n> \n> +#include \n> +\n> #include \n> #include \n> \n> @@ -16,11 +18,18 @@\n> class PostProcessor\n> {\n> public:\n> +\tenum class Status {\n> +\t\tError,\n> +\t\tSuccess\n> +\t};\n> +\n> \tvirtual ~PostProcessor() = default;\n> \n> \tvirtual int configure(const libcamera::StreamConfiguration &inCfg,\n> \t\t\t const libcamera::StreamConfiguration &outCfg) = 0;\n> \tvirtual int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0;\n> +\n> +\tlibcamera::Signal processComplete;\n> };\n> \n> #endif /* __ANDROID_POST_PROCESSOR_H__ */\n> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp\n> index 70385ab3..05c4f1cf 100644\n> --- a/src/android/yuv/post_processor_yuv.cpp\n> +++ b/src/android/yuv/post_processor_yuv.cpp\n> @@ -54,12 +54,15 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff\n> \tconst FrameBuffer &source = *streamBuffer->srcBuffer;\n> \tCameraBuffer *destination = streamBuffer->dstBuffer.get();\n> \n> -\tif (!isValidBuffers(source, *destination))\n> +\tif (!isValidBuffers(source, *destination)) {\n> +\t\tprocessComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> \t\treturn -EINVAL;\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(streamBuffer, PostProcessor::Status::Error);\n> \t\treturn -EINVAL;\n> \t}\n> \n> @@ -77,9 +80,12 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff\n> \t\t\t\t libyuv::FilterMode::kFilterBilinear);\n> \tif (ret) {\n> \t\tLOG(YUV, Error) << \"Failed NV12 scaling: \" << ret;\n> +\t\tprocessComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\tprocessComplete.emit(streamBuffer, PostProcessor::Status::Success);\n> +\n> \treturn 0;\n> }\n>","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 EE083BDB1C\n\tfor ;\n\tMon, 25 Oct 2021 21:58:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B21CC6487A;\n\tMon, 25 Oct 2021 23:58:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A101760125\n\tfor ;\n\tMon, 25 Oct 2021 23:58:33 +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 38EC4E0A;\n\tMon, 25 Oct 2021 23:58:33 +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=\"HeU6Uuzh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635199113;\n\tbh=/4z+koQo2c+BN9ANPUpy1KvpjXUBiSL7PkxUlQZVg3s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HeU6UuzhGxtVRgvmAZJnQsqQzZhE+W5o8QqSEWHfEdwfK6giqBEqU5qsmC9AZjSVX\n\tVMH+xboX4FXOVNtXIazhsat12GHiH0rlzDcQ79N7wKgEpiXnallE7vrg4AxL87JePH\n\tryLA/pgPy2hxSEzq8H5Npfpok5HMzboq77lxedCg=","Date":"Tue, 26 Oct 2021 00:58:11 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20211025203833.122460-1-umang.jain@ideasonboard.com>\n\t<20211025203833.122460-6-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211025203833.122460-6-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v7 5/7] android: Track and notify post\n\tprocessing of streams","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\" "}},{"id":20490,"web_url":"https://patchwork.libcamera.org/comment/20490/","msgid":"","date":"2021-10-26T01:11:48","subject":"Re: [libcamera-devel] [PATCH v7 5/7] android: Track and notify post\n\tprocessing of streams","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Umang, thank you for the patch.\n\nOn Tue, Oct 26, 2021 at 6:58 AM Laurent Pinchart\n wrote:\n>\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Tue, Oct 26, 2021 at 02:08:31AM +0530, Umang Jain wrote:\n> > Notify that the post processing for a request has been completed,\n> > via a signal. The signal is emitted with a context pointer along with\n> > status of the buffer. The function CameraDevice::streamProcessingComplete()\n> > will finally set the status on the request descriptor and complete the\n> > descriptor if all the streams requiring post processing are completed.\n> > If buffer status obtained is in error state, notify the status to the\n> > framework and set the overall error status on the descriptor via\n> > setBufferStatus().\n> >\n> > We need to track the number of streams requiring post-processing\n> > per Camera3RequestDescriptor (i.e. per capture request). Introduce\n> > a std::map<> to track the post-processing of streams. The nodes\n> > are dropped from the map when a particular stream post processing\n> > is completed (or on error paths). A std::map is selected for tracking\n> > post-processing requests, since we will move post-processing to be\n> > asynchronous in subsequent commits. A vector or queue will not be\n> > suitable then as the sequential order of post-processing completion\n> > of various requests won't be guaranteed then.\n> >\n> > A streamsProcessMutex_ has been introduced here as well, which will be\n> > applicable to guard access to descriptor's pendingStreamsToProcess_ when\n> > post-processing is moved to be asynchronous in subsequent commits.\n> >\n> > Signed-off-by: Umang Jain \n>\n> Reviewed-by: Laurent Pinchart \n>\n> > ---\n> > src/android/camera_device.cpp | 87 +++++++++++++++++-------\n> > src/android/camera_device.h | 4 ++\n> > src/android/camera_request.h | 6 ++\n> > src/android/camera_stream.cpp | 15 ++++\n> > src/android/jpeg/post_processor_jpeg.cpp | 2 +\n> > src/android/post_processor.h | 9 +++\n> > src/android/yuv/post_processor_yuv.cpp | 8 ++-\n> > 7 files changed, 106 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 9155728a..3ded0f7e 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -926,6 +926,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > * Request.\n> > */\n> > LOG(HAL, Debug) << ss.str() << \" (mapped)\";\n> > +\n> > + descriptor->pendingStreamsToProcess_.insert(\n> > + { cameraStream, &buffer });\n> > continue;\n> >\n> > case CameraStream::Type::Direct:\n> > @@ -955,6 +958,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > frameBuffer = cameraStream->getBuffer();\n> > buffer.internalBuffer = frameBuffer;\n> > LOG(HAL, Debug) << ss.str() << \" (internal)\";\n> > +\n> > + descriptor->pendingStreamsToProcess_.insert(\n> > + { cameraStream, &buffer });\n> > break;\n> > }\n> >\n> > @@ -1118,42 +1124,42 @@ void CameraDevice::requestComplete(Request *request)\n> > }\n> >\n> > /* Handle post-processing. */\n> > - for (auto &buffer : descriptor->buffers_) {\n> > - CameraStream *stream = buffer.stream;\n> > -\n> > - if (stream->type() == CameraStream::Type::Direct)\n> > - continue;\n> > + /*\n> > + * \\todo Protect the loop below with streamProcessMutex_ when post\n> > + * processor runs asynchronously.\n> > + */\n> > + auto iter = descriptor->pendingStreamsToProcess_.begin();\n> > + while (iter != descriptor->pendingStreamsToProcess_.end()) {\n> > + CameraStream *stream = iter->first;\n> > + Camera3RequestDescriptor::StreamBuffer *buffer = iter->second;\n> >\n> > FrameBuffer *src = request->findBuffer(stream->stream());\n> > if (!src) {\n> > LOG(HAL, Error) << \"Failed to find a source stream buffer\";\n> > - buffer.status = Camera3RequestDescriptor::Status::Error;\n> > - notifyError(descriptor->frameNumber_, stream->camera3Stream(),\n> > - CAMERA3_MSG_ERROR_BUFFER);\n> > - descriptor->status_ = Camera3RequestDescriptor::Status::Error;\n> > + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);\n> > + iter = descriptor->pendingStreamsToProcess_.erase(iter);\n> > continue;\n> > }\n> >\n> > - buffer.srcBuffer = src;\n> > -\n> > - int ret = stream->process(&buffer);\n> > -\n> > - /*\n> > - * If the framebuffer is internal to CameraStream return it back\n> > - * now that we're done processing it.\n> > - */\n> > - if (buffer.internalBuffer)\n> > - stream->putBuffer(buffer.internalBuffer);\n> > + buffer->srcBuffer = src;\n> >\n> > + ++iter;\n> > + int ret = stream->process(buffer);\n> > if (ret) {\n> > - buffer.status = Camera3RequestDescriptor::Status::Error;\n> > - notifyError(descriptor->frameNumber_, stream->camera3Stream(),\n> > - CAMERA3_MSG_ERROR_BUFFER);\n> > - descriptor->status_ = Camera3RequestDescriptor::Status::Error;\n> > + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);\n> > + descriptor->pendingStreamsToProcess_.erase(stream);\n> > +\n> > + /*\n> > + * If the framebuffer is internal to CameraStream return\n> > + * it back now that we're done processing it.\n> > + */\n> > + if (buffer->internalBuffer)\n> > + stream->putBuffer(buffer->internalBuffer);\n> > }\n> > }\n> >\n> > - completeDescriptor(descriptor);\n> > + if (descriptor->pendingStreamsToProcess_.empty())\n> > + completeDescriptor(descriptor);\n> > }\n> >\n> > void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)\n> > @@ -1208,6 +1214,39 @@ void CameraDevice::sendCaptureResults()\n> > }\n> > }\n> >\n> > +void CameraDevice::setBufferStatus(Camera3RequestDescriptor::StreamBuffer &streamBuffer,\n> > + Camera3RequestDescriptor::Status status)\n> > +{\n> > + streamBuffer.status = status;\n> > + if (status != Camera3RequestDescriptor::Status::Success) {\n> > + notifyError(streamBuffer.request->frameNumber_,\n> > + streamBuffer.stream->camera3Stream(),\n> > + CAMERA3_MSG_ERROR_BUFFER);\n> > +\n> > + /* Also set error status on entire request descriptor. */\n> > + streamBuffer.request->status_ =\n> > + Camera3RequestDescriptor::Status::Error;\n> > + }\n> > +}\n> > +\n> > +void CameraDevice::streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> > + Camera3RequestDescriptor::Status status)\n> > +{\n> > + setBufferStatus(*streamBuffer, status);\n> > +\n> > + /*\n> > + * If the framebuffer is internal to CameraStream return it back now\n> > + * that we're done processing it.\n> > + */\n> > + if (streamBuffer->internalBuffer)\n> > + streamBuffer->stream->putBuffer(streamBuffer->internalBuffer);\n> > +\n> > + Camera3RequestDescriptor *request = streamBuffer->request;\n> > + MutexLocker locker(request->streamsProcessMutex_);\n> > +\n> > + request->pendingStreamsToProcess_.erase(streamBuffer->stream);\n> > +}\n> > +\n> > std::string CameraDevice::logPrefix() const\n> > {\n> > return \"'\" + camera_->id() + \"'\";\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index e544f2bd..2a414020 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -66,6 +66,8 @@ public:\n> > int configureStreams(camera3_stream_configuration_t *stream_list);\n> > int processCaptureRequest(camera3_capture_request_t *request);\n> > void requestComplete(libcamera::Request *request);\n> > + void streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *bufferStream,\n> > + Camera3RequestDescriptor::Status status);\n> >\n> > protected:\n> > std::string logPrefix() const override;\n> > @@ -95,6 +97,8 @@ private:\n> > int processControls(Camera3RequestDescriptor *descriptor);\n> > void completeDescriptor(Camera3RequestDescriptor *descriptor);\n> > void sendCaptureResults();\n> > + void setBufferStatus(Camera3RequestDescriptor::StreamBuffer &buffer,\n> > + Camera3RequestDescriptor::Status status);\n> > std::unique_ptr getResultMetadata(\n> > const Camera3RequestDescriptor &descriptor) const;\n> >\n> > diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> > index c7fda00d..c28f7942 100644\n> > --- a/src/android/camera_request.h\n> > +++ b/src/android/camera_request.h\n> > @@ -7,7 +7,9 @@\n> > #ifndef __ANDROID_CAMERA_REQUEST_H__\n> > #define __ANDROID_CAMERA_REQUEST_H__\n> >\n> > +#include \n> > #include \n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -43,6 +45,10 @@ public:\n> > Camera3RequestDescriptor *request;\n> > };\n> >\n> > + /* Keeps track of streams requiring post-processing. */\n> > + std::map pendingStreamsToProcess_;\n> > + std::mutex streamsProcessMutex_;\n> > +\n> > Camera3RequestDescriptor(libcamera::Camera *camera,\n> > const camera3_capture_request_t *camera3Request);\n> > ~Camera3RequestDescriptor();\n> > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> > index 282b19b0..dac216d6 100644\n> > --- a/src/android/camera_stream.cpp\n> > +++ b/src/android/camera_stream.cpp\n> > @@ -22,6 +22,7 @@\n> > #include \"camera_capabilities.h\"\n> > #include \"camera_device.h\"\n> > #include \"camera_metadata.h\"\n> > +#include \"post_processor.h\"\n> >\n> > using namespace libcamera;\n> >\n> > @@ -97,6 +98,20 @@ int CameraStream::configure()\n> > int ret = postProcessor_->configure(configuration(), output);\n> > if (ret)\n> > return ret;\n> > +\n> > + postProcessor_->processComplete.connect(\n> > + this, [&](Camera3RequestDescriptor::StreamBuffer *streamBuffer,\n> > + PostProcessor::Status status) {\nnit: [cameraDevice=cameraDevice_]\n\nReviewed-by: Hirokazu Honda \n> > + Camera3RequestDescriptor::Status bufferStatus;\n> > +\n> > + if (status == PostProcessor::Status::Success)\n> > + bufferStatus = Camera3RequestDescriptor::Status::Success;\n> > + else\n> > + bufferStatus = Camera3RequestDescriptor::Status::Error;\n> > +\n> > + cameraDevice_->streamProcessingComplete(streamBuffer,\n> > + bufferStatus);\n> > + });\n> > }\n> >\n> > if (type_ == Type::Internal) {\n> > diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp\n> > index 240e29f6..5e8c61fc 100644\n> > --- a/src/android/jpeg/post_processor_jpeg.cpp\n> > +++ b/src/android/jpeg/post_processor_jpeg.cpp\n> > @@ -198,6 +198,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf\n> > exif.data(), quality);\n> > if (jpeg_size < 0) {\n> > LOG(JPEG, Error) << \"Failed to encode stream image\";\n> > + processComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> > return jpeg_size;\n> > }\n> >\n> > @@ -211,6 +212,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf\n> >\n> > /* Update the JPEG result Metadata. */\n> > resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size);\n> > + processComplete.emit(streamBuffer, PostProcessor::Status::Success);\n> >\n> > return 0;\n> > }\n> > diff --git a/src/android/post_processor.h b/src/android/post_processor.h\n> > index 128161c8..4ac74fcf 100644\n> > --- a/src/android/post_processor.h\n> > +++ b/src/android/post_processor.h\n> > @@ -7,6 +7,8 @@\n> > #ifndef __ANDROID_POST_PROCESSOR_H__\n> > #define __ANDROID_POST_PROCESSOR_H__\n> >\n> > +#include \n> > +\n> > #include \n> > #include \n> >\n> > @@ -16,11 +18,18 @@\n> > class PostProcessor\n> > {\n> > public:\n> > + enum class Status {\n> > + Error,\n> > + Success\n> > + };\n> > +\n> > virtual ~PostProcessor() = default;\n> >\n> > virtual int configure(const libcamera::StreamConfiguration &inCfg,\n> > const libcamera::StreamConfiguration &outCfg) = 0;\n> > virtual int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0;\n> > +\n> > + libcamera::Signal processComplete;\n> > };\n> >\n> > #endif /* __ANDROID_POST_PROCESSOR_H__ */\n> > diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp\n> > index 70385ab3..05c4f1cf 100644\n> > --- a/src/android/yuv/post_processor_yuv.cpp\n> > +++ b/src/android/yuv/post_processor_yuv.cpp\n> > @@ -54,12 +54,15 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff\n> > const FrameBuffer &source = *streamBuffer->srcBuffer;\n> > CameraBuffer *destination = streamBuffer->dstBuffer.get();\n> >\n> > - if (!isValidBuffers(source, *destination))\n> > + if (!isValidBuffers(source, *destination)) {\n> > + processComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> > return -EINVAL;\n> > + }\n> >\n> > const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read);\n> > if (!sourceMapped.isValid()) {\n> > LOG(YUV, Error) << \"Failed to mmap camera frame buffer\";\n> > + processComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> > return -EINVAL;\n> > }\n> >\n> > @@ -77,9 +80,12 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff\n> > libyuv::FilterMode::kFilterBilinear);\n> > if (ret) {\n> > LOG(YUV, Error) << \"Failed NV12 scaling: \" << ret;\n> > + processComplete.emit(streamBuffer, PostProcessor::Status::Error);\n> > return -EINVAL;\n> > }\n> >\n> > + processComplete.emit(streamBuffer, PostProcessor::Status::Success);\n> > +\n> > return 0;\n> > }\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 EEE6CBDB1C\n\tfor ;\n\tTue, 26 Oct 2021 01:12:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3320564878;\n\tTue, 26 Oct 2021 03:12:01 +0200 (CEST)","from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com\n\t[IPv6:2a00:1450:4864:20::52e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 685C26486D\n\tfor ;\n\tTue, 26 Oct 2021 03:11:59 +0200 (CEST)","by mail-ed1-x52e.google.com with SMTP id w15so6515780edc.9\n\tfor ;\n\tMon, 25 Oct 2021 18:11:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"IwUG5cey\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=70ILVnD+yyO2tF+viNizkxh2I9bUpHrvNDXz6Dg89vU=;\n\tb=IwUG5ceyGw2sfuM9y4Onb0j1EHMZIOtAreItNKs1JQUtHIxixwcZ8iJqt0hoNfaRLs\n\tEJEyLLa8k4n/kv4mg+zEt3TgSiLTJMdHQvyuaThyxY220wCy3IEEEYpL7oKuHsjfADqc\n\tAxT6oTdxgT9BoobUCT/rykiNE/sbGhSlz4ZLk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=70ILVnD+yyO2tF+viNizkxh2I9bUpHrvNDXz6Dg89vU=;\n\tb=W6QFv86uGDxSugUROrsSMT9vty7E08cjk8nEs2W/gMZxJYLlQkRWKv68fm01k2qwcK\n\tpm2q4eyHs84ldOWOl8K26M5p4wwYSnQQJG5qxUq5o3fhS1riotMZAJV0YpOAnaXcj/WD\n\tO45QWlM5ZSuY7/hQ0Ogzf/LmULKOj83iJBWnXgX/EeURkqm/6yX22dkPcmgD+25JFepS\n\t1eGBVgng8AhEfSfkJinnP1cYSczGsMkIIS/sbBznfANGUPLZGuMxpbdENWeYaLXMfhQq\n\tX9N1WI+3hEHwyLdbgNAT3QI8TfWOwRKUOozyOxH7dLhK3WzXL9h63wXvgzaXpGw7acyp\n\tsAog==","X-Gm-Message-State":"AOAM531lUXO2PnYRxxJYU5UT+uIzsL62KHEaqdX1wIryhOSKxSnRTD6t\n\t4EE/BI1ogznTWN6W7C7OJamFhLjYObXNHPzYe0Wx38Kl","X-Google-Smtp-Source":"ABdhPJyDzL0HL2pEX0LmiEyJ5BxSbML88WDrI9LsGNa0d8dVTUlySsL59h36PUvhr19xlrutJWqgXysDk2puB6xMvmI=","X-Received":"by 2002:a05:6402:3554:: with SMTP id\n\tf20mr31104539edd.354.1635210718785; \n\tMon, 25 Oct 2021 18:11:58 -0700 (PDT)","MIME-Version":"1.0","References":"<20211025203833.122460-1-umang.jain@ideasonboard.com>\n\t<20211025203833.122460-6-umang.jain@ideasonboard.com>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Tue, 26 Oct 2021 10:11:48 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v7 5/7] android: Track and notify post\n\tprocessing of streams","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]