[libcamera-devel,v2,1/3] libcamera: PipelineHandler: Make queueRequest() void function
diff mbox series

Message ID 20210329091552.157208-2-hiroh@chromium.org
State Accepted
Headers show
Series
  • Add RequestError to Request::Status
Related show

Commit Message

Hirokazu Honda March 29, 2021, 9:15 a.m. UTC 
PipelineHandler::queueRequest() is asynchronously invoked in
Camera::queueRequest(). Therefore the return value of
PipelineHandler::queueRequest() is useless. This changes the
function to a void function.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/pipeline_handler.h | 2 +-
 src/libcamera/camera.cpp                      | 6 ++++--
 src/libcamera/pipeline_handler.cpp            | 6 +-----
 3 files changed, 6 insertions(+), 8 deletions(-)

Comments

Jacopo Mondi March 29, 2021, 9:22 a.m. UTC | #1 
Hi Hiro,

On Mon, Mar 29, 2021 at 06:15:50PM +0900, Hirokazu Honda wrote:
> PipelineHandler::queueRequest() is asynchronously invoked in
> Camera::queueRequest(). Therefore the return value of
> PipelineHandler::queueRequest() is useless. This changes the
> function to a void function.
>
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  include/libcamera/internal/pipeline_handler.h | 2 +-
>  src/libcamera/camera.cpp                      | 6 ++++--
>  src/libcamera/pipeline_handler.cpp            | 6 +-----
>  3 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 6aca0b46..093b94c6 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -79,7 +79,7 @@ public:
>  	virtual int start(Camera *camera, const ControlList *controls) = 0;
>  	virtual void stop(Camera *camera) = 0;
>
> -	int queueRequest(Request *request);
> +	void queueRequest(Request *request);
>
>  	bool completeBuffer(Request *request, FrameBuffer *buffer);
>  	void completeRequest(Request *request);
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 84edbb8f..2611c701 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -996,8 +996,10 @@ int Camera::queueRequest(Request *request)
>  		}
>  	}
>
> -	return d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
> -				      ConnectionTypeQueued, request);
> +	d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
> +			       ConnectionTypeQueued, request);
> +
> +	return 0;
>  }
>
>  /**
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index d22991d3..eba00ed3 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -371,10 +371,8 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
>   * signalled by the pipeline handler using the completeRequest() method.
>   *
>   * \context This function is called from the CameraManager thread.
> - *
> - * \return 0 on success or a negative error code otherwise
>   */
> -int PipelineHandler::queueRequest(Request *request)
> +void PipelineHandler::queueRequest(Request *request)
>  {
>  	LIBCAMERA_TRACEPOINT(request_queue, request);
>
> @@ -385,8 +383,6 @@ int PipelineHandler::queueRequest(Request *request)
>  	int ret = queueRequestDevice(camera, request);
>  	if (ret)
>  		data->queuedRequests_.remove(request);
> -
> -	return ret;
>  }
>
>  /**
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart April 3, 2021, 1:09 a.m. UTC | #2 
Hi Hiro,

Thank you for the patch.

On Mon, Mar 29, 2021 at 06:15:50PM +0900, Hirokazu Honda wrote:
> PipelineHandler::queueRequest() is asynchronously invoked in
> Camera::queueRequest(). Therefore the return value of
> PipelineHandler::queueRequest() is useless. This changes the
> function to a void function.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I'll apply this already as it doesn't require the rest of the series.

> ---
>  include/libcamera/internal/pipeline_handler.h | 2 +-
>  src/libcamera/camera.cpp                      | 6 ++++--
>  src/libcamera/pipeline_handler.cpp            | 6 +-----
>  3 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index 6aca0b46..093b94c6 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -79,7 +79,7 @@ public:
>  	virtual int start(Camera *camera, const ControlList *controls) = 0;
>  	virtual void stop(Camera *camera) = 0;
>  
> -	int queueRequest(Request *request);
> +	void queueRequest(Request *request);
>  
>  	bool completeBuffer(Request *request, FrameBuffer *buffer);
>  	void completeRequest(Request *request);
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 84edbb8f..2611c701 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -996,8 +996,10 @@ int Camera::queueRequest(Request *request)
>  		}
>  	}
>  
> -	return d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
> -				      ConnectionTypeQueued, request);
> +	d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
> +			       ConnectionTypeQueued, request);
> +
> +	return 0;
>  }
>  
>  /**
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index d22991d3..eba00ed3 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -371,10 +371,8 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
>   * signalled by the pipeline handler using the completeRequest() method.
>   *
>   * \context This function is called from the CameraManager thread.
> - *
> - * \return 0 on success or a negative error code otherwise
>   */
> -int PipelineHandler::queueRequest(Request *request)
> +void PipelineHandler::queueRequest(Request *request)
>  {
>  	LIBCAMERA_TRACEPOINT(request_queue, request);
>  
> @@ -385,8 +383,6 @@ int PipelineHandler::queueRequest(Request *request)
>  	int ret = queueRequestDevice(camera, request);
>  	if (ret)
>  		data->queuedRequests_.remove(request);
> -
> -	return ret;
>  }
>  
>  /**

Patch
diff mbox series 

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index 6aca0b46..093b94c6 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -79,7 +79,7 @@  public:
 	virtual int start(Camera *camera, const ControlList *controls) = 0;
 	virtual void stop(Camera *camera) = 0;
 
-	int queueRequest(Request *request);
+	void queueRequest(Request *request);
 
 	bool completeBuffer(Request *request, FrameBuffer *buffer);
 	void completeRequest(Request *request);
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 84edbb8f..2611c701 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -996,8 +996,10 @@  int Camera::queueRequest(Request *request)
 		}
 	}
 
-	return d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
-				      ConnectionTypeQueued, request);
+	d->pipe_->invokeMethod(&PipelineHandler::queueRequest,
+			       ConnectionTypeQueued, request);
+
+	return 0;
 }
 
 /**
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index d22991d3..eba00ed3 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -371,10 +371,8 @@  const ControlList &PipelineHandler::properties(const Camera *camera) const
  * signalled by the pipeline handler using the completeRequest() method.
  *
  * \context This function is called from the CameraManager thread.
- *
- * \return 0 on success or a negative error code otherwise
  */
-int PipelineHandler::queueRequest(Request *request)
+void PipelineHandler::queueRequest(Request *request)
 {
 	LIBCAMERA_TRACEPOINT(request_queue, request);
 
@@ -385,8 +383,6 @@  int PipelineHandler::queueRequest(Request *request)
 	int ret = queueRequestDevice(camera, request);
 	if (ret)
 		data->queuedRequests_.remove(request);
-
-	return ret;
 }
 
 /**