[libcamera-devel,v3,2/8] libcamera: request: Add Request::cancel()
diff mbox series

Message ID 20210521154227.60186-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • Implement flush() camera operation
Related show

Commit Message

Jacopo Mondi May 21, 2021, 3:42 p.m. UTC 
Add a cancel() function to the Request class that allows to forcefully
complete the request and its associated buffers in error state.

Only pending requests can be forcefully cancelled. Enforce that
by asserting the request state to be RequestPending.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
---
 .../libcamera/internal/tracepoints/request.tp |  8 ++++++
 include/libcamera/request.h                   |  1 +
 src/libcamera/request.cpp                     | 28 +++++++++++++++++++
 3 files changed, 37 insertions(+)

Comments

Niklas Söderlund May 22, 2021, 9:26 a.m. UTC | #1 
Hi Jacopo,

Thanks for your patch.

On 2021-05-21 17:42:21 +0200, Jacopo Mondi wrote:
> Add a cancel() function to the Request class that allows to forcefully
> complete the request and its associated buffers in error state.
> 
> Only pending requests can be forcefully cancelled. Enforce that
> by asserting the request state to be RequestPending.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  .../libcamera/internal/tracepoints/request.tp |  8 ++++++
>  include/libcamera/request.h                   |  1 +
>  src/libcamera/request.cpp                     | 28 +++++++++++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp
> index 9e872951374d..9c841b97438a 100644
> --- a/include/libcamera/internal/tracepoints/request.tp
> +++ b/include/libcamera/internal/tracepoints/request.tp
> @@ -66,6 +66,14 @@ TRACEPOINT_EVENT_INSTANCE(
>  	)
>  )
>  
> +TRACEPOINT_EVENT_INSTANCE(
> +	libcamera,
> +	request,
> +	request_cancel,
> +	TP_ARGS(
> +		libcamera::Request *, req
> +	)
> +)
>  
>  TRACEPOINT_EVENT(
>  	libcamera,
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 4cf5ff3f7d3b..5596901ddd8e 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -65,6 +65,7 @@ private:
>  	friend class PipelineHandler;
>  
>  	void complete();
> +	void cancel();
>  
>  	bool completeBuffer(FrameBuffer *buffer);
>  
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index ce2dd7b17f10..69b2d0172e3d 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -292,6 +292,34 @@ void Request::complete()
>  	LIBCAMERA_TRACEPOINT(request_complete, this);
>  }
>  
> +/**
> + * \brief Cancel a queued request
> + *
> + * Mark the request and its associated buffers as cancelled and complete it.
> + *
> + * Set each pending buffer in error state and emit the buffer completion signal
> + * before completing the Request.
> + */
> +void Request::cancel()
> +{
> +	LIBCAMERA_TRACEPOINT(request_cancel, this);
> +
> +	ASSERT(status_ == RequestPending);
> +
> +	/*
> +	 * We can't simply loop and call completeBuffer() as the erase() call
> +	 * invalidates pointers and iterators, so we have to manually cancel the
> +	 * buffer from the pending buffers list.
> +	 */
> +	for (auto buffer = pending_.begin(); buffer != pending_.end();) {
> +		(*buffer)->cancel();
> +		camera_->bufferCompleted.emit(this, *buffer);
> +		buffer = pending_.erase(buffer);
> +	}
> +
> +	cancelled_ = true;
> +}
> +
>  /**
>   * \brief Complete a buffer for the request
>   * \param[in] buffer The buffer that has completed
> -- 
> 2.31.1
>
Laurent Pinchart May 23, 2021, 5:55 p.m. UTC | #2 
Hi Jacopo,

Thank you for the patch.

On Fri, May 21, 2021 at 05:42:21PM +0200, Jacopo Mondi wrote:
> Add a cancel() function to the Request class that allows to forcefully
> complete the request and its associated buffers in error state.
> 
> Only pending requests can be forcefully cancelled. Enforce that
> by asserting the request state to be RequestPending.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  .../libcamera/internal/tracepoints/request.tp |  8 ++++++
>  include/libcamera/request.h                   |  1 +
>  src/libcamera/request.cpp                     | 28 +++++++++++++++++++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp
> index 9e872951374d..9c841b97438a 100644
> --- a/include/libcamera/internal/tracepoints/request.tp
> +++ b/include/libcamera/internal/tracepoints/request.tp
> @@ -66,6 +66,14 @@ TRACEPOINT_EVENT_INSTANCE(
>  	)
>  )
>  
> +TRACEPOINT_EVENT_INSTANCE(
> +	libcamera,
> +	request,
> +	request_cancel,
> +	TP_ARGS(
> +		libcamera::Request *, req
> +	)
> +)
>  
>  TRACEPOINT_EVENT(
>  	libcamera,
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 4cf5ff3f7d3b..5596901ddd8e 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -65,6 +65,7 @@ private:
>  	friend class PipelineHandler;
>  
>  	void complete();
> +	void cancel();
>  
>  	bool completeBuffer(FrameBuffer *buffer);
>  
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index ce2dd7b17f10..69b2d0172e3d 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -292,6 +292,34 @@ void Request::complete()
>  	LIBCAMERA_TRACEPOINT(request_complete, this);
>  }
>  
> +/**
> + * \brief Cancel a queued request
> + *
> + * Mark the request and its associated buffers as cancelled and complete it.
> + *
> + * Set each pending buffer in error state and emit the buffer completion signal
> + * before completing the Request.
> + */
> +void Request::cancel()
> +{
> +	LIBCAMERA_TRACEPOINT(request_cancel, this);
> +
> +	ASSERT(status_ == RequestPending);
> +
> +	/*
> +	 * We can't simply loop and call completeBuffer() as the erase() call
> +	 * invalidates pointers and iterators, so we have to manually cancel the
> +	 * buffer from the pending buffers list.
> +	 */
> +	for (auto buffer = pending_.begin(); buffer != pending_.end();) {
> +		(*buffer)->cancel();
> +		camera_->bufferCompleted.emit(this, *buffer);
> +		buffer = pending_.erase(buffer);
> +	}

How about

	for (FrameBuffer *buffer : pending_) {
		buffer->cancel();
		camera_->bufferCompleted.emit(this, buffer);
	}

	pending_.clear();

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	cancelled_ = true;
> +}
> +
>  /**
>   * \brief Complete a buffer for the request
>   * \param[in] buffer The buffer that has completed

Patch
diff mbox series 

diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp
index 9e872951374d..9c841b97438a 100644
--- a/include/libcamera/internal/tracepoints/request.tp
+++ b/include/libcamera/internal/tracepoints/request.tp
@@ -66,6 +66,14 @@  TRACEPOINT_EVENT_INSTANCE(
 	)
 )
 
+TRACEPOINT_EVENT_INSTANCE(
+	libcamera,
+	request,
+	request_cancel,
+	TP_ARGS(
+		libcamera::Request *, req
+	)
+)
 
 TRACEPOINT_EVENT(
 	libcamera,
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 4cf5ff3f7d3b..5596901ddd8e 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -65,6 +65,7 @@  private:
 	friend class PipelineHandler;
 
 	void complete();
+	void cancel();
 
 	bool completeBuffer(FrameBuffer *buffer);
 
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index ce2dd7b17f10..69b2d0172e3d 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -292,6 +292,34 @@  void Request::complete()
 	LIBCAMERA_TRACEPOINT(request_complete, this);
 }
 
+/**
+ * \brief Cancel a queued request
+ *
+ * Mark the request and its associated buffers as cancelled and complete it.
+ *
+ * Set each pending buffer in error state and emit the buffer completion signal
+ * before completing the Request.
+ */
+void Request::cancel()
+{
+	LIBCAMERA_TRACEPOINT(request_cancel, this);
+
+	ASSERT(status_ == RequestPending);
+
+	/*
+	 * We can't simply loop and call completeBuffer() as the erase() call
+	 * invalidates pointers and iterators, so we have to manually cancel the
+	 * buffer from the pending buffers list.
+	 */
+	for (auto buffer = pending_.begin(); buffer != pending_.end();) {
+		(*buffer)->cancel();
+		camera_->bufferCompleted.emit(this, *buffer);
+		buffer = pending_.erase(buffer);
+	}
+
+	cancelled_ = true;
+}
+
 /**
  * \brief Complete a buffer for the request
  * \param[in] buffer The buffer that has completed