| Message ID | 20211201142936.107405-9-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
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@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@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
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(); + } + + /* 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
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@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(-)