[libcamera-devel] [PATCH v3 08/10] android: camera_stream: Drop return value for process()
Jacopo Mondi
jacopo at jmondi.org
Tue Sep 21 13:33:42 CEST 2021
Hi Umang,
On Mon, Sep 20, 2021 at 11:07:50PM +0530, Umang Jain wrote:
> CameraStream::process() is invoked by CameraDevice::requestComplete()
> in case any post-processing is required for the camera stream. The
> failure or success is checked via the value returned by
> CameraStream::process().
>
> Now that the post-processor notifies about the post-processing
> completion operation and sets the ProcessStatus on the
> Camera3RequestDescriptor passed to it, we can drop the return value of
> CameraStream::process() in favour of reading the process status set on
> the descriptor in the PostProcessor::processComplete's slot.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 26 +++++++++++++-----------
> src/android/camera_stream.cpp | 12 +++++------
> src/android/camera_stream.h | 6 +++---
> src/android/jpeg/post_processor_jpeg.cpp | 12 +++++------
> src/android/jpeg/post_processor_jpeg.h | 6 +++---
> src/android/post_processor.h | 6 +++---
> src/android/yuv/post_processor_yuv.cpp | 14 ++++++-------
> src/android/yuv/post_processor_yuv.h | 6 +++---
> 8 files changed, 43 insertions(+), 45 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index e2de4012..4658e881 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1158,7 +1158,7 @@ void CameraDevice::requestComplete(Request *request)
>
> descriptor->status_ = Camera3RequestDescriptor::ProcessStatus::Processing;
>
> - int ret = cameraStream->process(src, *buffer.buffer, descriptor);
> + cameraStream->process(src, *buffer.buffer, descriptor);
>
> /*
> * Return the FrameBuffer to the CameraStream now that we're
> @@ -1166,12 +1166,6 @@ void CameraDevice::requestComplete(Request *request)
> */
> if (cameraStream->type() == CameraStream::Type::Internal)
> cameraStream->putBuffer(src);
> -
> - if (ret) {
> - buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
> - notifyError(descriptor->frameNumber_, buffer.stream,
> - CAMERA3_MSG_ERROR_BUFFER);
> - }
> }
>
> captureResult.result = descriptor->resultMetadata_->get();
> @@ -1182,12 +1176,20 @@ void CameraDevice::requestComplete(Request *request)
>
>
> void CameraDevice::streamProcessingComplete([[maybe_unused]] CameraStream *cameraStream,
> - [[maybe_unused]] Camera3RequestDescriptor *request)
> + Camera3RequestDescriptor *request)
> {
> - /*
> - * \todo Stream processing is completed hence, check for errors and
> - * if any, mark the corresponding buffer with CAMERA3_BUFFER_STATUS_ERROR.
> - */
> + if (request->status_ == Camera3RequestDescriptor::ProcessStatus::Error) {
> + for (camera3_stream_buffer_t &buffer : request->buffers_) {
> + CameraStream *cs = static_cast<CameraStream *>(buffer.stream->priv);
> +
> + if (cs->camera3Stream().format != HAL_PIXEL_FORMAT_BLOB)
> + continue;
> +
> + buffer.status = CAMERA3_BUFFER_STATUS_ERROR;
> + notifyError(request->frameNumber_, buffer.stream,
> + CAMERA3_MSG_ERROR_BUFFER);
> + }
> + }
I understand where this is going, but am I wrong or with this patch
post-processing results won't ever be sent to the camera service ?
It's not a big deal, but isn't it easier to introduce
CameraDevice::sendCaptureResult() before this change ?
> }
>
> std::string CameraDevice::logPrefix() const
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 70471959..c18c7041 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -101,14 +101,14 @@ int CameraStream::configure()
> return 0;
> }
>
> -int CameraStream::process(const FrameBuffer *source,
> - buffer_handle_t camera3Dest,
> - Camera3RequestDescriptor *request)
> +void CameraStream::process(const FrameBuffer *source,
> + buffer_handle_t camera3Dest,
> + Camera3RequestDescriptor *request)
> {
> if (!postProcessor_) {
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> handleProcessComplete(request);
> - return 0;
> + return;
> }
>
> /*
> @@ -122,10 +122,10 @@ int CameraStream::process(const FrameBuffer *source,
> LOG(HAL, Error) << "Failed to map android blob buffer";
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> handleProcessComplete(request);
> - return -EINVAL;
> + return;
> }
>
> - return postProcessor_->process(source, &dest, request);
> + postProcessor_->process(source, &dest, request);
> }
>
> void CameraStream::handleProcessComplete(Camera3RequestDescriptor *request)
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index f8ad6245..d4ec5c25 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -120,9 +120,9 @@ public:
> libcamera::Stream *stream() const;
>
> int configure();
> - int process(const libcamera::FrameBuffer *source,
> - buffer_handle_t camera3Dest,
> - Camera3RequestDescriptor *request);
> + void process(const libcamera::FrameBuffer *source,
> + buffer_handle_t camera3Dest,
> + Camera3RequestDescriptor *request);
> libcamera::FrameBuffer *getBuffer();
> void putBuffer(libcamera::FrameBuffer *buffer);
>
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 87252acd..19eb7983 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -97,14 +97,14 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer *source,
> }
> }
>
> -int PostProcessorJpeg::process(const FrameBuffer *source,
> - CameraBuffer *destination,
> - Camera3RequestDescriptor *request)
> +void PostProcessorJpeg::process(const FrameBuffer *source,
> + CameraBuffer *destination,
> + Camera3RequestDescriptor *request)
> {
> if (!encoder_) {
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> processComplete.emit(request);
> - return 0;
> + return;
> }
>
> ASSERT(destination->numPlanes() == 1);
> @@ -202,7 +202,7 @@ int PostProcessorJpeg::process(const FrameBuffer *source,
> LOG(JPEG, Error) << "Failed to encode stream image";
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> processComplete.emit(request);
> - return jpeg_size;
> + return;
> }
>
> /* Fill in the JPEG blob header. */
> @@ -218,6 +218,4 @@ int PostProcessorJpeg::process(const FrameBuffer *source,
>
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Success;
> processComplete.emit(request);
> -
> - return 0;
> }
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index d49c8d2b..e9938919 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -22,9 +22,9 @@ public:
>
> int configure(const libcamera::StreamConfiguration &incfg,
> const libcamera::StreamConfiguration &outcfg) override;
> - int process(const libcamera::FrameBuffer *source,
> - CameraBuffer *destination,
> - Camera3RequestDescriptor *request) override;
> + void process(const libcamera::FrameBuffer *source,
> + CameraBuffer *destination,
> + Camera3RequestDescriptor *request) override;
>
> private:
> void generateThumbnail(const libcamera::FrameBuffer *source,
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index f639b237..48ddd8ac 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -25,9 +25,9 @@ public:
>
> virtual int configure(const libcamera::StreamConfiguration &inCfg,
> const libcamera::StreamConfiguration &outCfg) = 0;
> - virtual int process(const libcamera::FrameBuffer *source,
> - CameraBuffer *destination,
> - Camera3RequestDescriptor *request) = 0;
> + virtual void process(const libcamera::FrameBuffer *source,
> + CameraBuffer *destination,
> + Camera3RequestDescriptor *request) = 0;
>
> libcamera::Signal<Camera3RequestDescriptor *> processComplete;
> };
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index b144649a..09838aa5 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -51,14 +51,14 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
> return 0;
> }
>
> -int PostProcessorYuv::process(const FrameBuffer *source,
> - CameraBuffer *destination,
> - Camera3RequestDescriptor *request)
> +void PostProcessorYuv::process(const FrameBuffer *source,
> + CameraBuffer *destination,
> + Camera3RequestDescriptor *request)
> {
> if (!isValidBuffers(source, *destination)) {
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> processComplete.emit(request);
> - return -EINVAL;
> + return;
> }
>
> const MappedFrameBuffer sourceMapped(source, MappedFrameBuffer::MapFlag::Read);
> @@ -66,7 +66,7 @@ int PostProcessorYuv::process(const FrameBuffer *source,
> LOG(YUV, Error) << "Failed to mmap camera frame buffer";
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> processComplete.emit(request);
> - return -EINVAL;
> + return;
> }
>
> int ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(),
> @@ -85,13 +85,11 @@ int PostProcessorYuv::process(const FrameBuffer *source,
> LOG(YUV, Error) << "Failed NV12 scaling: " << ret;
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Error;
> processComplete.emit(request);
> - return -EINVAL;
> + return;
> }
>
> request->status_ = Camera3RequestDescriptor::ProcessStatus::Success;
> processComplete.emit(request);
> -
> - return 0;
> }
>
> bool PostProcessorYuv::isValidBuffers(const FrameBuffer *source,
> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> index eddd1086..32e5b60d 100644
> --- a/src/android/yuv/post_processor_yuv.h
> +++ b/src/android/yuv/post_processor_yuv.h
> @@ -20,9 +20,9 @@ public:
>
> int configure(const libcamera::StreamConfiguration &incfg,
> const libcamera::StreamConfiguration &outcfg) override;
> - int process(const libcamera::FrameBuffer *source,
> - CameraBuffer *destination,
> - Camera3RequestDescriptor *context) override;
> + void process(const libcamera::FrameBuffer *source,
> + CameraBuffer *destination,
> + Camera3RequestDescriptor *context) override;
>
> private:
> bool isValidBuffers(const libcamera::FrameBuffer *source,
> --
> 2.31.1
>
More information about the libcamera-devel
mailing list