[libcamera-devel] [PATCH v3 07/10] android: post_processor: Notify post processing completion status
Jacopo Mondi
jacopo at jmondi.org
Tue Sep 21 13:29:28 CEST 2021
Hi Umang,
On Mon, Sep 20, 2021 at 11:07:49PM +0530, Umang Jain wrote:
> We should be able to know if post-processing has been completed
> successfully or encountered some errors. This commit introduces a
> Signal<> that will notify that the post-processing has been completed.
> If the post processing was successful, status on the request descriptor
> will be set to Camera3RequestDescriptor::ProcessStatus::Success.
> The signal will be required when the post-processor is meant to
> run asynchronously (in subsequent commits) and capture results need
> to be sent back to the framework from the signal's slot instead.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 18 ++++++++++++++++++
> src/android/camera_device.h | 11 +++++++++++
> src/android/camera_stream.cpp | 15 ++++++++++++++-
> src/android/camera_stream.h | 2 ++
> src/android/jpeg/post_processor_jpeg.cpp | 10 +++++++++-
> src/android/post_processor.h | 4 ++++
> src/android/yuv/post_processor_yuv.cpp | 16 ++++++++++++++--
> 7 files changed, 72 insertions(+), 4 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index fa462368..e2de4012 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -246,6 +246,12 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
> */
> request_ = std::make_unique<CaptureRequest>(camera,
> reinterpret_cast<uint64_t>(this));
> +
> + /*
> + * Denotes the post-processing status required by any stream. It is set
> + * to ProcessStatus::Success if processing is successful.
> + */
> + status_ = ProcessStatus::None;
Looking at how the status is used in CameraDevice, I would rather make
it a paramter of CameraDevice::streamProcessingComplete() and in the
following patch of sendCaptureResults().
This way you could avoid there
if (d->status_ == Camera3RequestDescriptor::ProcessStatus::Processing ||
d->status_ == Camera3RequestDescriptor::ProcessStatus::None)
return;
> }
>
> /*
> @@ -1150,6 +1156,8 @@ void CameraDevice::requestComplete(Request *request)
> continue;
> }
>
> + descriptor->status_ = Camera3RequestDescriptor::ProcessStatus::Processing;
> +
> int ret = cameraStream->process(src, *buffer.buffer, descriptor);
>
> /*
> @@ -1172,6 +1180,16 @@ void CameraDevice::requestComplete(Request *request)
> descriptors_.pop_front();
> }
>
> +
> +void CameraDevice::streamProcessingComplete([[maybe_unused]] CameraStream *cameraStream,
> + [[maybe_unused]] Camera3RequestDescriptor *request)
> +{
> + /*
> + * \todo Stream processing is completed hence, check for errors and
> + * if any, mark the corresponding buffer with CAMERA3_BUFFER_STATUS_ERROR.
> + */
> +}
> +
> std::string CameraDevice::logPrefix() const
> {
> return "'" + camera_->id() + "'";
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index b2871e52..60c134dc 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -36,6 +36,13 @@
> struct CameraConfigData;
>
> struct Camera3RequestDescriptor {
> + enum class ProcessStatus {
> + None,
> + Processing,
> + Success,
> + Error
> + };
> +
> Camera3RequestDescriptor() = default;
> ~Camera3RequestDescriptor() = default;
> Camera3RequestDescriptor(libcamera::Camera *camera,
> @@ -48,6 +55,8 @@ struct Camera3RequestDescriptor {
> CameraMetadata settings_;
> std::unique_ptr<CaptureRequest> request_;
> std::unique_ptr<CameraMetadata> resultMetadata_;
> +
> + ProcessStatus status_;
> };
>
> class CameraDevice : protected libcamera::Loggable
> @@ -79,6 +88,8 @@ 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);
>
> protected:
> std::string logPrefix() const override;
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index d494f5d5..70471959 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -81,6 +81,9 @@ int CameraStream::configure()
> int ret = postProcessor_->configure(configuration(), output);
> if (ret)
> return ret;
> +
> + postProcessor_->processComplete.connect(
> + this, &CameraStream::handleProcessComplete);
> }
>
> if (allocator_) {
> @@ -102,8 +105,11 @@ int CameraStream::process(const FrameBuffer *source,
> buffer_handle_t camera3Dest,
> Camera3RequestDescriptor *request)
> {
> - if (!postProcessor_)
> + if (!postProcessor_) {
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> + handleProcessComplete(request);
> return 0;
> + }
That was like this before, but I feel like this is a development
mistaken and should be better caught with Fatal
same for the below !encoder and the YUV post-processor
>
> /*
> * \todo Buffer mapping and processing should be moved to a
> @@ -114,12 +120,19 @@ int CameraStream::process(const FrameBuffer *source,
> PROT_READ | PROT_WRITE);
> if (!dest.isValid()) {
> LOG(HAL, Error) << "Failed to map android blob buffer";
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> + handleProcessComplete(request);
> return -EINVAL;
> }
>
> return postProcessor_->process(source, &dest, request);
> }
>
> +void CameraStream::handleProcessComplete(Camera3RequestDescriptor *request)
> +{
> + cameraDevice_->streamProcessingComplete(this, request);
> +}
Can't CameraStream connect CameraDevice::streamProcessingComplete with
the post-processor's processComplete signal ?
> +
> FrameBuffer *CameraStream::getBuffer()
> {
> if (!allocator_)
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 68789700..f8ad6245 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -127,6 +127,8 @@ public:
> void putBuffer(libcamera::FrameBuffer *buffer);
>
> private:
> + void handleProcessComplete(Camera3RequestDescriptor *request);
> +
> CameraDevice *const cameraDevice_;
> const libcamera::CameraConfiguration *config_;
> const Type type_;
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 31f68330..87252acd 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -101,8 +101,11 @@ int PostProcessorJpeg::process(const FrameBuffer *source,
> CameraBuffer *destination,
> Camera3RequestDescriptor *request)
> {
> - if (!encoder_)
> + if (!encoder_) {
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> + processComplete.emit(request);
> return 0;
> + }
>
> ASSERT(destination->numPlanes() == 1);
>
> @@ -197,6 +200,8 @@ int PostProcessorJpeg::process(const FrameBuffer *source,
> exif.data(), quality);
> if (jpeg_size < 0) {
> LOG(JPEG, Error) << "Failed to encode stream image";
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> + processComplete.emit(request);
> return jpeg_size;
> }
>
> @@ -211,5 +216,8 @@ int PostProcessorJpeg::process(const FrameBuffer *source,
> /* Update the JPEG result Metadata. */
> resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size);
>
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Success;
> + processComplete.emit(request);
> +
> return 0;
> }
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index bdd6afe7..f639b237 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>
>
> @@ -26,6 +28,8 @@ public:
> virtual int process(const libcamera::FrameBuffer *source,
> CameraBuffer *destination,
> Camera3RequestDescriptor *request) = 0;
> +
> + libcamera::Signal<Camera3RequestDescriptor *> 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 5e18caee..b144649a 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,19 @@ 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)) {
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> + processComplete.emit(request);
If my first suggestion will be considered, the post processor will
send the status in the signal, and you can reduce the number of status
to Success or Error maybe ?
> return -EINVAL;
> + }
>
> const MappedFrameBuffer sourceMapped(source, MappedFrameBuffer::MapFlag::Read);
> if (!sourceMapped.isValid()) {
> LOG(YUV, Error) << "Failed to mmap camera frame buffer";
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> + processComplete.emit(request);
> return -EINVAL;
> }
>
> @@ -76,9 +83,14 @@ int PostProcessorYuv::process(const FrameBuffer *source,
> libyuv::FilterMode::kFilterBilinear);
> if (ret) {
> LOG(YUV, Error) << "Failed NV12 scaling: " << ret;
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> + processComplete.emit(request);
> return -EINVAL;
> }
>
> + request->status_ = Camera3RequestDescriptor::ProcessStatus::Success;
> + processComplete.emit(request);
> +
> return 0;
> }
>
> --
> 2.31.1
>
More information about the libcamera-devel
mailing list