| Message ID | 20211011073505.243864-4-umang.jain@ideasonboard.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
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@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; > } >
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) +{ + 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); + } + } + + /* + * Return the FrameBuffer to the CameraStream now that we're + * done processing it. + */ + if (cameraStream->type() == CameraStream::Type::Internal) + cameraStream->putBuffer(request->internalBuffer_); + + if (status == PostProcessor::Status::Success) + request->status_ = Camera3RequestDescriptor::Status::Success; + else + request->status_ = Camera3RequestDescriptor::Status::Error; + + sendCaptureResults(); +} + 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; }
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@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(-)