| Message ID | 20211025203833.122460-6-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
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@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@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; > } >
Hi Umang, thank you for the patch. On Tue, Oct 26, 2021 at 6:58 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > 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@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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) { nit: [cameraDevice=cameraDevice_] Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > + 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
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; }
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@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(-)