[libcamera-devel] [PATCH v3 08/10] android: camera_stream: Drop return value for process()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Sep 22 00:33:56 CEST 2021
Hello,
On Tue, Sep 21, 2021 at 01:33:42PM +0200, Jacopo Mondi wrote:
> 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 ?
That's my understanding too, this will break bisection. We should try to
avoid it.
> > }
> >
> > 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,
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list