[libcamera-devel] [PATCH v4 08/11] libcamera: pipeline: Introduce stopDevice()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Dec 8 12:45:26 CET 2021
Hi Jacopo,
Thank you for the patch.
On Wed, Dec 01, 2021 at 03:29:33PM +0100, Jacopo Mondi wrote:
> Since a queue of waiting Requests has been introduced, not all Requests
> queued to the PipelineHandler are immediately queued to the device.
>
> As a Camera can be stopped at any time, it is required to complete the
> waiting requests after the ones queued to the device had been completed.
>
> Introduce a pure virtual PipelineHandler::stopDevice() function to be
> implemented by pipeline handlers and make the PipelineHandler::stop()
> function call it before completing pending requests.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/pipeline_handler.h | 3 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +--
> .../pipeline/raspberrypi/raspberrypi.cpp | 4 +--
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +--
> src/libcamera/pipeline/simple/simple.cpp | 4 +--
> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 +--
> src/libcamera/pipeline/vimc/vimc.cpp | 4 +--
> src/libcamera/pipeline_handler.cpp | 30 +++++++++++++++++--
> 8 files changed, 42 insertions(+), 15 deletions(-)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 6aa3378547ba..ec986a518b5c 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -55,7 +55,7 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
>
> virtual int start(Camera *camera, const ControlList *controls) = 0;
> - virtual void stop(Camera *camera) = 0;
> + void stop(Camera *camera);
> bool hasPendingRequests(const Camera *camera) const;
>
> void queueRequest(Request *request);
> @@ -70,6 +70,7 @@ protected:
> void hotplugMediaDevice(MediaDevice *media);
>
> virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
> + virtual void stopDevice(Camera *camera) = 0;
>
> CameraManager *manager_;
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index c65afdb2912a..8da1cd93c4dc 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -134,7 +134,7 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>
> int start(Camera *camera, const ControlList *controls) override;
> - void stop(Camera *camera) override;
> + void stopDevice(Camera *camera) override;
>
> int queueRequestDevice(Camera *camera, Request *request) override;
>
> @@ -792,7 +792,7 @@ error:
> return ret;
> }
>
> -void PipelineHandlerIPU3::stop(Camera *camera)
> +void PipelineHandlerIPU3::stopDevice(Camera *camera)
> {
> IPU3CameraData *data = cameraData(camera);
> int ret = 0;
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index fa2848533b29..cb6ba1c2db18 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -303,7 +303,7 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>
> int start(Camera *camera, const ControlList *controls) override;
> - void stop(Camera *camera) override;
> + void stopDevice(Camera *camera) override;
>
> int queueRequestDevice(Camera *camera, Request *request) override;
>
> @@ -924,7 +924,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
> return 0;
> }
>
> -void PipelineHandlerRPi::stop(Camera *camera)
> +void PipelineHandlerRPi::stopDevice(Camera *camera)
> {
> RPiCameraData *data = cameraData(camera);
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 36ef6a02ae90..8cca8a15a3c3 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -146,7 +146,7 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>
> int start(Camera *camera, const ControlList *controls) override;
> - void stop(Camera *camera) override;
> + void stopDevice(Camera *camera) override;
>
> int queueRequestDevice(Camera *camera, Request *request) override;
>
> @@ -827,7 +827,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL
> return ret;
> }
>
> -void PipelineHandlerRkISP1::stop(Camera *camera)
> +void PipelineHandlerRkISP1::stopDevice(Camera *camera)
> {
> RkISP1CameraData *data = cameraData(camera);
> int ret;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 701fb4be0b71..fdff4ebd5134 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -279,7 +279,7 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>
> int start(Camera *camera, const ControlList *controls) override;
> - void stop(Camera *camera) override;
> + void stopDevice(Camera *camera) override;
>
> bool match(DeviceEnumerator *enumerator) override;
>
> @@ -1036,7 +1036,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
> return 0;
> }
>
> -void SimplePipelineHandler::stop(Camera *camera)
> +void SimplePipelineHandler::stopDevice(Camera *camera)
> {
> SimpleCameraData *data = cameraData(camera);
> V4L2VideoDevice *video = data->video_;
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 264f5370cf32..40654a0bc40c 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -74,7 +74,7 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>
> int start(Camera *camera, const ControlList *controls) override;
> - void stop(Camera *camera) override;
> + void stopDevice(Camera *camera) override;
>
> int queueRequestDevice(Camera *camera, Request *request) override;
>
> @@ -250,7 +250,7 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList
> return 0;
> }
>
> -void PipelineHandlerUVC::stop(Camera *camera)
> +void PipelineHandlerUVC::stopDevice(Camera *camera)
> {
> UVCCameraData *data = cameraData(camera);
> data->video_->streamOff();
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index e453091da4b2..29960fe3f038 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -91,7 +91,7 @@ public:
> std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
>
> int start(Camera *camera, const ControlList *controls) override;
> - void stop(Camera *camera) override;
> + void stopDevice(Camera *camera) override;
>
> int queueRequestDevice(Camera *camera, Request *request) override;
>
> @@ -359,7 +359,7 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] const ControlLis
> return 0;
> }
>
> -void PipelineHandlerVimc::stop(Camera *camera)
> +void PipelineHandlerVimc::stopDevice(Camera *camera)
> {
> VimcCameraData *data = cameraData(camera);
> data->video_->streamOff();
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 2374c2891c64..863d206a7ddd 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -267,8 +267,7 @@ void PipelineHandler::unlock()
> */
>
> /**
> - * \fn PipelineHandler::stop()
> - * \brief Stop capturing from all running streams
> + * \brief Stop capturing from all running streams and cancel pending requests
> * \param[in] camera The camera to stop
> *
> * This function stops capturing and processing requests immediately. All
> @@ -276,6 +275,33 @@ void PipelineHandler::unlock()
> *
> * \context This function is called from the CameraManager thread.
> */
> +void PipelineHandler::stop(Camera *camera)
> +{
> + /* Stop the pipeline handler and let the queued requests complete. */
> + stopDevice(camera);
> +
> + /* Cancel and signal as complete all waiting requests. */
> + while (!waitingRequests_.empty()) {
> + Request *request = waitingRequests_.front();
> +
> + request->_d()->cancel();
> + completeRequest(request);
> + waitingRequests_.pop();
I'd move the pop() call just after front() to match what we usually do,
but it likely makes no real difference.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + }
> +
> + /* Make sure no requests are pending. */
> + Camera::Private *data = camera->_d();
> + ASSERT(data->queuedRequests_.empty());
> +}
> +
> +/**
> + * \fn PipelineHandler::stopDevice()
> + * \brief Stop capturing from all running streams
> + * \param[in] camera The camera to stop
> + *
> + * This function stops capturing and processing requests immediately. All
> + * pending requests are cancelled and complete immediately in an error state.
> + */
>
> /**
> * \brief Determine if the camera has any requests pending
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list