[libcamera-devel] [PATCH v7 5/7] android: Track and notify post processing of streams
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 25 23:58:11 CEST 2021
Hi Umang,
Thank you for the patch.
On Tue, Oct 26, 2021 at 02:08:31AM +0530, Umang Jain wrote:
> Notify that the post processing for a request has been completed,
> via a signal. The signal is emitted with a context pointer along with
> status of the buffer. The function CameraDevice::streamProcessingComplete()
> will finally set the status on the request descriptor and complete the
> descriptor if all the streams requiring post processing are completed.
> If buffer status obtained is in error state, notify the status to the
> framework and set the overall error status on the descriptor via
> setBufferStatus().
>
> We need to track the number of streams requiring post-processing
> per Camera3RequestDescriptor (i.e. per capture request). Introduce
> a std::map<> to track the post-processing of streams. The nodes
> are dropped from the map when a particular stream post processing
> is completed (or on error paths). A std::map is selected for tracking
> post-processing requests, since we will move post-processing to be
> asynchronous in subsequent commits. A vector or queue will not be
> suitable then as the sequential order of post-processing completion
> of various requests won't be guaranteed then.
>
> A streamsProcessMutex_ has been introduced here as well, which will be
> applicable to guard access to descriptor's pendingStreamsToProcess_ when
> post-processing is moved to be asynchronous in subsequent commits.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 87 +++++++++++++++++-------
> src/android/camera_device.h | 4 ++
> src/android/camera_request.h | 6 ++
> src/android/camera_stream.cpp | 15 ++++
> src/android/jpeg/post_processor_jpeg.cpp | 2 +
> src/android/post_processor.h | 9 +++
> src/android/yuv/post_processor_yuv.cpp | 8 ++-
> 7 files changed, 106 insertions(+), 25 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 9155728a..3ded0f7e 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -926,6 +926,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> * Request.
> */
> LOG(HAL, Debug) << ss.str() << " (mapped)";
> +
> + descriptor->pendingStreamsToProcess_.insert(
> + { cameraStream, &buffer });
> continue;
>
> case CameraStream::Type::Direct:
> @@ -955,6 +958,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> frameBuffer = cameraStream->getBuffer();
> buffer.internalBuffer = frameBuffer;
> LOG(HAL, Debug) << ss.str() << " (internal)";
> +
> + descriptor->pendingStreamsToProcess_.insert(
> + { cameraStream, &buffer });
> break;
> }
>
> @@ -1118,42 +1124,42 @@ void CameraDevice::requestComplete(Request *request)
> }
>
> /* Handle post-processing. */
> - for (auto &buffer : descriptor->buffers_) {
> - CameraStream *stream = buffer.stream;
> -
> - if (stream->type() == CameraStream::Type::Direct)
> - continue;
> + /*
> + * \todo Protect the loop below with streamProcessMutex_ when post
> + * processor runs asynchronously.
> + */
> + auto iter = descriptor->pendingStreamsToProcess_.begin();
> + while (iter != descriptor->pendingStreamsToProcess_.end()) {
> + CameraStream *stream = iter->first;
> + Camera3RequestDescriptor::StreamBuffer *buffer = iter->second;
>
> FrameBuffer *src = request->findBuffer(stream->stream());
> if (!src) {
> LOG(HAL, Error) << "Failed to find a source stream buffer";
> - buffer.status = Camera3RequestDescriptor::Status::Error;
> - notifyError(descriptor->frameNumber_, stream->camera3Stream(),
> - CAMERA3_MSG_ERROR_BUFFER);
> - descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);
> + iter = descriptor->pendingStreamsToProcess_.erase(iter);
> continue;
> }
>
> - buffer.srcBuffer = src;
> -
> - int ret = stream->process(&buffer);
> -
> - /*
> - * If the framebuffer is internal to CameraStream return it back
> - * now that we're done processing it.
> - */
> - if (buffer.internalBuffer)
> - stream->putBuffer(buffer.internalBuffer);
> + buffer->srcBuffer = src;
>
> + ++iter;
> + int ret = stream->process(buffer);
> if (ret) {
> - buffer.status = Camera3RequestDescriptor::Status::Error;
> - notifyError(descriptor->frameNumber_, stream->camera3Stream(),
> - CAMERA3_MSG_ERROR_BUFFER);
> - descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);
> + descriptor->pendingStreamsToProcess_.erase(stream);
> +
> + /*
> + * If the framebuffer is internal to CameraStream return
> + * it back now that we're done processing it.
> + */
> + if (buffer->internalBuffer)
> + stream->putBuffer(buffer->internalBuffer);
> }
> }
>
> - completeDescriptor(descriptor);
> + if (descriptor->pendingStreamsToProcess_.empty())
> + completeDescriptor(descriptor);
> }
>
> void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)
> @@ -1208,6 +1214,39 @@ void CameraDevice::sendCaptureResults()
> }
> }
>
> +void CameraDevice::setBufferStatus(Camera3RequestDescriptor::StreamBuffer &streamBuffer,
> + Camera3RequestDescriptor::Status status)
> +{
> + streamBuffer.status = status;
> + if (status != Camera3RequestDescriptor::Status::Success) {
> + notifyError(streamBuffer.request->frameNumber_,
> + streamBuffer.stream->camera3Stream(),
> + CAMERA3_MSG_ERROR_BUFFER);
> +
> + /* Also set error status on entire request descriptor. */
> + streamBuffer.request->status_ =
> + Camera3RequestDescriptor::Status::Error;
> + }
> +}
> +
> +void CameraDevice::streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> + Camera3RequestDescriptor::Status status)
> +{
> + setBufferStatus(*streamBuffer, status);
> +
> + /*
> + * If the framebuffer is internal to CameraStream return it back now
> + * that we're done processing it.
> + */
> + if (streamBuffer->internalBuffer)
> + streamBuffer->stream->putBuffer(streamBuffer->internalBuffer);
> +
> + Camera3RequestDescriptor *request = streamBuffer->request;
> + MutexLocker locker(request->streamsProcessMutex_);
> +
> + request->pendingStreamsToProcess_.erase(streamBuffer->stream);
> +}
> +
> std::string CameraDevice::logPrefix() const
> {
> return "'" + camera_->id() + "'";
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index e544f2bd..2a414020 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -66,6 +66,8 @@ public:
> int configureStreams(camera3_stream_configuration_t *stream_list);
> int processCaptureRequest(camera3_capture_request_t *request);
> void requestComplete(libcamera::Request *request);
> + void streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *bufferStream,
> + Camera3RequestDescriptor::Status status);
>
> protected:
> std::string logPrefix() const override;
> @@ -95,6 +97,8 @@ private:
> int processControls(Camera3RequestDescriptor *descriptor);
> void completeDescriptor(Camera3RequestDescriptor *descriptor);
> void sendCaptureResults();
> + void setBufferStatus(Camera3RequestDescriptor::StreamBuffer &buffer,
> + Camera3RequestDescriptor::Status status);
> std::unique_ptr<CameraMetadata> getResultMetadata(
> const Camera3RequestDescriptor &descriptor) const;
>
> diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> index c7fda00d..c28f7942 100644
> --- a/src/android/camera_request.h
> +++ b/src/android/camera_request.h
> @@ -7,7 +7,9 @@
> #ifndef __ANDROID_CAMERA_REQUEST_H__
> #define __ANDROID_CAMERA_REQUEST_H__
>
> +#include <map>
> #include <memory>
> +#include <mutex>
> #include <vector>
>
> #include <libcamera/base/class.h>
> @@ -43,6 +45,10 @@ public:
> Camera3RequestDescriptor *request;
> };
>
> + /* Keeps track of streams requiring post-processing. */
> + std::map<CameraStream *, StreamBuffer *> pendingStreamsToProcess_;
> + std::mutex streamsProcessMutex_;
> +
> Camera3RequestDescriptor(libcamera::Camera *camera,
> const camera3_capture_request_t *camera3Request);
> ~Camera3RequestDescriptor();
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 282b19b0..dac216d6 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -22,6 +22,7 @@
> #include "camera_capabilities.h"
> #include "camera_device.h"
> #include "camera_metadata.h"
> +#include "post_processor.h"
>
> using namespace libcamera;
>
> @@ -97,6 +98,20 @@ int CameraStream::configure()
> int ret = postProcessor_->configure(configuration(), output);
> if (ret)
> return ret;
> +
> + postProcessor_->processComplete.connect(
> + this, [&](Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> + PostProcessor::Status status) {
> + Camera3RequestDescriptor::Status bufferStatus;
> +
> + if (status == PostProcessor::Status::Success)
> + bufferStatus = Camera3RequestDescriptor::Status::Success;
> + else
> + bufferStatus = Camera3RequestDescriptor::Status::Error;
> +
> + cameraDevice_->streamProcessingComplete(streamBuffer,
> + bufferStatus);
> + });
> }
>
> if (type_ == Type::Internal) {
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 240e29f6..5e8c61fc 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -198,6 +198,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf
> exif.data(), quality);
> if (jpeg_size < 0) {
> LOG(JPEG, Error) << "Failed to encode stream image";
> + processComplete.emit(streamBuffer, PostProcessor::Status::Error);
> return jpeg_size;
> }
>
> @@ -211,6 +212,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf
>
> /* Update the JPEG result Metadata. */
> resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size);
> + processComplete.emit(streamBuffer, PostProcessor::Status::Success);
>
> return 0;
> }
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index 128161c8..4ac74fcf 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>
>
> @@ -16,11 +18,18 @@
> class PostProcessor
> {
> public:
> + enum class Status {
> + Error,
> + Success
> + };
> +
> virtual ~PostProcessor() = default;
>
> virtual int configure(const libcamera::StreamConfiguration &inCfg,
> const libcamera::StreamConfiguration &outCfg) = 0;
> virtual int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0;
> +
> + libcamera::Signal<Camera3RequestDescriptor::StreamBuffer *, 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 70385ab3..05c4f1cf 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -54,12 +54,15 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff
> const FrameBuffer &source = *streamBuffer->srcBuffer;
> CameraBuffer *destination = streamBuffer->dstBuffer.get();
>
> - if (!isValidBuffers(source, *destination))
> + if (!isValidBuffers(source, *destination)) {
> + processComplete.emit(streamBuffer, 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(streamBuffer, PostProcessor::Status::Error);
> return -EINVAL;
> }
>
> @@ -77,9 +80,12 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff
> libyuv::FilterMode::kFilterBilinear);
> if (ret) {
> LOG(YUV, Error) << "Failed NV12 scaling: " << ret;
> + processComplete.emit(streamBuffer, PostProcessor::Status::Error);
> return -EINVAL;
> }
>
> + processComplete.emit(streamBuffer, PostProcessor::Status::Success);
> +
> return 0;
> }
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list