[libcamera-devel] [PATCH v4 3/7] android: Notify post processing completion via a signal
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Oct 12 05:31:03 CEST 2021
Hi Umang,
Thank you for the patch.
On Mon, Oct 11, 2021 at 01:05:01PM +0530, Umang Jain wrote:
> Notify that the post processing for a request has been completed,
> via a signal. A pointer to the descriptor which is tracking the
> capture request is emitted along with the status of post processing.
> The function CameraDevice::streamProcessingComplete() will finally set
> the status on the descriptor by reading the status of the post-processor
> completion (passed as an argument to the signal) and call
> sendCaptureResults().
>
> We also need to save a pointer to any internal buffers that might have
> been allocated by CameraStream. The buffer should be returned back to
> CameraStream just before capture results are sent.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 49 +++++++++++++++++-------
> src/android/camera_device.h | 5 +++
> src/android/camera_stream.cpp | 5 +++
> src/android/jpeg/post_processor_jpeg.cpp | 2 +
> src/android/post_processor.h | 9 +++++
> src/android/yuv/post_processor_yuv.cpp | 12 +++++-
> 6 files changed, 67 insertions(+), 15 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index b52bdc8f..9f26c36d 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -8,7 +8,6 @@
> #include "camera_device.h"
> #include "camera_hal_config.h"
> #include "camera_ops.h"
> -#include "post_processor.h"
>
> #include <algorithm>
> #include <fstream>
> @@ -1226,20 +1225,12 @@ void CameraDevice::requestComplete(Request *request)
> continue;
> }
>
> - int ret = cameraStream->process(*src, buffer, descriptor);
> -
> - /*
> - * Return the FrameBuffer to the CameraStream now that we're
> - * done processing it.
> - */
> if (cameraStream->type() == CameraStream::Type::Internal)
> - cameraStream->putBuffer(src);
> + descriptor->internalBuffer_ = src;
>
> - if (ret) {
> - buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
> - notifyError(descriptor->frameNumber_, buffer.stream,
> - CAMERA3_MSG_ERROR_BUFFER);
> - }
> + int ret = cameraStream->process(*src, buffer, descriptor);
> +
> + return;
> }
>
> captureResult.result = descriptor->resultMetadata_->get();
> @@ -1266,6 +1257,38 @@ void CameraDevice::sendCaptureResults()
> }
> }
>
> +void CameraDevice::streamProcessingComplete(CameraStream *cameraStream,
> + Camera3RequestDescriptor *request,
> + PostProcessor::Status status)
> +{
You'll have a problem here if multiple streams need to be produced with
post-processing, the first stream to complete will complete the request.
> + if (status == PostProcessor::Status::Error) {
> + for (camera3_stream_buffer_t &buffer : request->buffers_) {
> + CameraStream *cs = static_cast<CameraStream *>(buffer.stream->priv);
> +
> + if (cs->type() == CameraStream::Type::Direct)
> + continue;
> +
> + buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
> + notifyError(request->frameNumber_, buffer.stream,
> + CAMERA3_MSG_ERROR_BUFFER);
> + }
> + }
This is right but shouldn't be done on all buffers in the request, only
the buffer for the stream that has completed.
> +
> + /*
> + * Return the FrameBuffer to the CameraStream now that we're
> + * done processing it.
> + */
> + if (cameraStream->type() == CameraStream::Type::Internal)
> + cameraStream->putBuffer(request->internalBuffer_);
Same here, and note how you could have one internal buffer per stream,
so it has to be stored accordingly, not as a single pointer in the
request.
> +
> + if (status == PostProcessor::Status::Success)
> + request->status_ = Camera3RequestDescriptor::Status::Success;
> + else
> + request->status_ = Camera3RequestDescriptor::Status::Error;
> +
> + sendCaptureResults();
Then you need to check if all post-processors have completed, and only
complete the request at that point.
> +}
> +
> std::string CameraDevice::logPrefix() const
> {
> return "'" + camera_->id() + "'";
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 3da8dffa..eee97516 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -32,6 +32,7 @@
> #include "camera_stream.h"
> #include "camera_worker.h"
> #include "jpeg/encoder.h"
> +#include "post_processor.h"
>
> struct CameraConfigData;
>
> @@ -56,6 +57,7 @@ struct Camera3RequestDescriptor {
> CameraMetadata settings_;
> std::unique_ptr<CaptureRequest> request_;
> std::unique_ptr<CameraMetadata> resultMetadata_;
> + libcamera::FrameBuffer *internalBuffer_;
>
> camera3_capture_result_t captureResult_ = {};
> Status status_ = Status::Pending;
> @@ -91,6 +93,9 @@ public:
> int configureStreams(camera3_stream_configuration_t *stream_list);
> int processCaptureRequest(camera3_capture_request_t *request);
> void requestComplete(libcamera::Request *request);
> + void streamProcessingComplete(CameraStream *cameraStream,
> + Camera3RequestDescriptor *request,
> + PostProcessor::Status status);
>
> protected:
> std::string logPrefix() const override;
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 8f47e4d8..d91e7dee 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -93,6 +93,11 @@ int CameraStream::configure()
> int ret = postProcessor_->configure(configuration(), output);
> if (ret)
> return ret;
> +
> + postProcessor_->processComplete.connect(
> + this, [&](Camera3RequestDescriptor *request, PostProcessor::Status status) {
> + cameraDevice_->streamProcessingComplete(this, request, status);
> + });
> }
>
> if (type_ == Type::Internal) {
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 656c48b2..81d1efe6 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -197,6 +197,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source,
> exif.data(), quality);
> if (jpeg_size < 0) {
> LOG(JPEG, Error) << "Failed to encode stream image";
> + processComplete.emit(request, PostProcessor::Status::Error);
> return jpeg_size;
> }
>
> @@ -210,6 +211,7 @@ 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/post_processor.h b/src/android/post_processor.h
> index fa13c7e0..6e67bcba 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -7,6 +7,8 @@
> #ifndef __ANDROID_POST_PROCESSOR_H__
> #define __ANDROID_POST_PROCESSOR_H__
>
> +#include <libcamera/base/signal.h>
> +
> #include <libcamera/framebuffer.h>
> #include <libcamera/stream.h>
>
> @@ -19,6 +21,11 @@ struct Camera3RequestDescriptor;
> class PostProcessor
> {
> public:
> + enum class Status {
> + Error,
> + Success
> + };
> +
> virtual ~PostProcessor() = default;
>
> virtual int configure(const libcamera::StreamConfiguration &inCfg,
> @@ -26,6 +33,8 @@ public:
> virtual int process(const libcamera::FrameBuffer &source,
> CameraBuffer *destination,
> Camera3RequestDescriptor *request) = 0;
> +
> + libcamera::Signal<Camera3RequestDescriptor *, Status> processComplete;
> };
>
> #endif /* __ANDROID_POST_PROCESSOR_H__ */
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index 8110a1f1..b34b389d 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -18,6 +18,8 @@
> #include "libcamera/internal/formats.h"
> #include "libcamera/internal/mapped_framebuffer.h"
>
> +#include "../camera_device.h"
> +
> using namespace libcamera;
>
> LOG_DEFINE_CATEGORY(YUV)
> @@ -51,14 +53,17 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
>
> int PostProcessorYuv::process(const FrameBuffer &source,
> CameraBuffer *destination,
> - [[maybe_unused]] Camera3RequestDescriptor *request)
> + Camera3RequestDescriptor *request)
> {
> - if (!isValidBuffers(source, *destination))
> + if (!isValidBuffers(source, *destination)) {
> + processComplete.emit(request, PostProcessor::Status::Error);
> return -EINVAL;
> + }
>
> 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;
> }
>
> @@ -76,9 +81,12 @@ int PostProcessorYuv::process(const FrameBuffer &source,
> libyuv::FilterMode::kFilterBilinear);
> if (ret) {
> LOG(YUV, Error) << "Failed NV12 scaling: " << ret;
> + processComplete.emit(request, PostProcessor::Status::Error);
> return -EINVAL;
> }
>
> + processComplete.emit(request, PostProcessor::Status::Success);
> +
> return 0;
> }
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list