[libcamera-devel] [PATCH 01/11] libcamera: pipeline_handler: Remove Camera argument from request handling
Jacopo Mondi
jacopo at jmondi.org
Mon Nov 9 10:55:47 CET 2020
Hi Niklas,
On Thu, Nov 05, 2020 at 01:15:36AM +0100, Niklas Söderlund wrote:
> There is no need to pass the Camera pointer to queueRequest(),
> completeBuffer() and completeRequest() as the Request also passed
> contains the same information. Remove the Camera argument to avoid
> situations where the information in the Request and the argument differ.
>
> There is no functional change and no public API change as the interface
> is only used between the Camera and PipelineHandler.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Thanks
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/pipeline_handler.h | 7 +++----
> src/libcamera/camera.cpp | 2 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++----
> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 8 ++++----
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++---
> src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++--
> src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--
> src/libcamera/pipeline_handler.cpp | 14 +++++++-------
> 9 files changed, 31 insertions(+), 33 deletions(-)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c12c8904858e7536..ca1997bc0771b139 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -81,11 +81,10 @@ public:
> virtual int start(Camera *camera) = 0;
> virtual void stop(Camera *camera) = 0;
>
> - int queueRequest(Camera *camera, Request *request);
> + int queueRequest(Request *request);
>
> - bool completeBuffer(Camera *camera, Request *request,
> - FrameBuffer *buffer);
> - void completeRequest(Camera *camera, Request *request);
> + bool completeBuffer(Request *request, FrameBuffer *buffer);
> + void completeRequest(Request *request);
>
> const char *name() const { return name_; }
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 9590ab7249d39e2f..f868161f2a620264 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -914,7 +914,7 @@ int Camera::queueRequest(Request *request)
> }
>
> return p_->pipe_->invokeMethod(&PipelineHandler::queueRequest,
> - ConnectionTypeQueued, this, request);
> + ConnectionTypeQueued, request);
> }
>
> /**
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 5a6ee1a83e45ff75..3f0232bc1eaad048 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -836,13 +836,13 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
> {
> Request *request = buffer->request();
>
> - if (!pipe_->completeBuffer(camera_, request, buffer))
> + if (!pipe_->completeBuffer(request, buffer))
> /* Request not completed yet, return here. */
> return;
>
> /* Mark the request as complete. */
> request->metadata().set(controls::draft::PipelineDepth, 3);
> - pipe_->completeRequest(camera_, request);
> + pipe_->completeRequest(request);
> }
>
> /**
> @@ -865,10 +865,10 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> * now as there's no need for ImgU processing.
> */
> if (request->findBuffer(&rawStream_)) {
> - bool isComplete = pipe_->completeBuffer(camera_, request, buffer);
> + bool isComplete = pipe_->completeBuffer(request, buffer);
> if (isComplete) {
> request->metadata().set(controls::draft::PipelineDepth, 2);
> - pipe_->completeRequest(camera_, request);
> + pipe_->completeRequest(request);
> return;
> }
> }
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 220b9c51849662f3..630b876985459d2f 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1463,10 +1463,10 @@ void RPiCameraData::clearIncompleteRequests()
> * request? If not, do so now.
> */
> if (buffer && buffer->request())
> - pipe_->completeBuffer(camera_, request, buffer);
> + pipe_->completeBuffer(request, buffer);
> }
>
> - pipe_->completeRequest(camera_, request);
> + pipe_->completeRequest(request);
> requestQueue_.pop_front();
> }
> }
> @@ -1490,7 +1490,7 @@ void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)
> * Tag the buffer as completed, returning it to the
> * application.
> */
> - pipe_->completeBuffer(camera_, request, buffer);
> + pipe_->completeBuffer(request, buffer);
> } else {
> /*
> * This buffer was not part of the Request, or there is no
> @@ -1554,7 +1554,7 @@ void RPiCameraData::checkRequestCompleted()
> if (state_ != State::IpaComplete)
> return;
>
> - pipe_->completeRequest(camera_, request);
> + pipe_->completeRequest(request);
> requestQueue_.pop_front();
> requestCompleted = true;
> }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 424099d087528fb1..70a10853bf83b6a5 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1042,15 +1042,14 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
>
> data->frameInfo_.destroy(info->frame);
>
> - completeRequest(activeCamera_, request);
> + completeRequest(request);
> }
>
> void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
> {
> - ASSERT(activeCamera_);
> Request *request = buffer->request();
>
> - completeBuffer(activeCamera_, request, buffer);
> + completeBuffer(request, buffer);
> tryCompleteRequest(request);
> }
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 3d2039f3f26951bf..dc55304fc6ebb202 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -903,8 +903,8 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
> }
>
> Request *request = buffer->request();
> - completeBuffer(activeCamera_, request, buffer);
> - completeRequest(activeCamera_, request);
> + completeBuffer(request, buffer);
> + completeRequest(request);
> return;
> }
>
> @@ -928,8 +928,8 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
>
> /* Otherwise simply complete the request. */
> Request *request = buffer->request();
> - completeBuffer(activeCamera_, request, buffer);
> - completeRequest(activeCamera_, request);
> + completeBuffer(request, buffer);
> + completeRequest(request);
> }
>
> void SimplePipelineHandler::converterDone(FrameBuffer *input,
> @@ -940,8 +940,8 @@ void SimplePipelineHandler::converterDone(FrameBuffer *input,
>
> /* Complete the request. */
> Request *request = output->request();
> - completeBuffer(activeCamera_, request, output);
> - completeRequest(activeCamera_, request);
> + completeBuffer(request, output);
> + completeRequest(request);
>
> /* Queue the input buffer back for capture. */
> data->video_->queueBuffer(input);
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 3862631b7bedf604..272e4c6bf50e82e4 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -650,8 +650,8 @@ void UVCCameraData::bufferReady(FrameBuffer *buffer)
> {
> Request *request = buffer->request();
>
> - pipe_->completeBuffer(camera_, request, buffer);
> - pipe_->completeRequest(camera_, request);
> + pipe_->completeBuffer(request, buffer);
> + pipe_->completeRequest(request);
> }
>
> REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC)
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 7416c37c6f5a243c..99b0fa2742269147 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -529,8 +529,8 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)
> {
> Request *request = buffer->request();
>
> - pipe_->completeBuffer(camera_, request, buffer);
> - pipe_->completeRequest(camera_, request);
> + pipe_->completeBuffer(request, buffer);
> + pipe_->completeRequest(request);
> }
>
> REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc)
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 894200ee4e936735..e60b962d66183e19 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -376,7 +376,6 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
> /**
> * \fn PipelineHandler::queueRequest()
> * \brief Queue a request to the camera
> - * \param[in] camera The camera to queue the request to
> * \param[in] request The request to queue
> *
> * This method queues a capture request to the pipeline handler for processing.
> @@ -391,8 +390,9 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
> *
> * \return 0 on success or a negative error code otherwise
> */
> -int PipelineHandler::queueRequest(Camera *camera, Request *request)
> +int PipelineHandler::queueRequest(Request *request)
> {
> + Camera *camera = request->camera_;
> CameraData *data = cameraData(camera);
> data->queuedRequests_.push_back(request);
>
> @@ -422,7 +422,6 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)
>
> /**
> * \brief Complete a buffer for a request
> - * \param[in] camera The camera the request belongs to
> * \param[in] request The request the buffer belongs to
> * \param[in] buffer The buffer that has completed
> *
> @@ -438,16 +437,15 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)
> * \return True if all buffers contained in the request have completed, false
> * otherwise
> */
> -bool PipelineHandler::completeBuffer(Camera *camera, Request *request,
> - FrameBuffer *buffer)
> +bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)
> {
> + Camera *camera = request->camera_;
> camera->bufferCompleted.emit(request, buffer);
> return request->completeBuffer(buffer);
> }
>
> /**
> * \brief Signal request completion
> - * \param[in] camera The camera that the request belongs to
> * \param[in] request The request that has completed
> *
> * The pipeline handler shall call this method to notify the \a camera that the
> @@ -460,8 +458,10 @@ bool PipelineHandler::completeBuffer(Camera *camera, Request *request,
> *
> * \context This function shall be called from the CameraManager thread.
> */
> -void PipelineHandler::completeRequest(Camera *camera, Request *request)
> +void PipelineHandler::completeRequest(Request *request)
> {
> + Camera *camera = request->camera_;
> +
> request->complete();
>
> CameraData *data = cameraData(camera);
> --
> 2.29.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list