[libcamera-devel,v2] libcamera: ipa_proxy: Allow stop() on a stopped IPA

Message ID 20200706134919.19224-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit 49771c6dccb28228839b5e88ada36bd6646bf933
Headers show
Series
  • [libcamera-devel,v2] libcamera: ipa_proxy: Allow stop() on a stopped IPA
Related show

Commit Message

Laurent Pinchart July 6, 2020, 1:49 p.m. UTC
To make error handling easier in callers, allow the stop() function to
be called when the proxy is already stopped, or not started yet.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
Changes since v1:

- Rewrite documentation
---
 include/libcamera/internal/ipa_proxy.h   |  2 ++
 src/libcamera/ipa_proxy.cpp              | 10 ++++++++++
 src/libcamera/proxy/ipa_proxy_thread.cpp |  3 +++
 3 files changed, 15 insertions(+)

Comments

Niklas Söderlund July 7, 2020, 6:51 p.m. UTC | #1
Hi Laurent,

Thanks for your patch.

On 2020-07-06 16:49:19 +0300, Laurent Pinchart wrote:
> To make error handling easier in callers, allow the stop() function to
> be called when the proxy is already stopped, or not started yet.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
> Changes since v1:
> 
> - Rewrite documentation
> ---
>  include/libcamera/internal/ipa_proxy.h   |  2 ++
>  src/libcamera/ipa_proxy.cpp              | 10 ++++++++++
>  src/libcamera/proxy/ipa_proxy_thread.cpp |  3 +++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h
> index aec8f04ffc15..b429ce5a68a3 100644
> --- a/include/libcamera/internal/ipa_proxy.h
> +++ b/include/libcamera/internal/ipa_proxy.h
> @@ -27,6 +27,8 @@ public:
>  
>  	std::string configurationFile(const std::string &file) const;
>  
> +	void stop() override = 0;
> +
>  protected:
>  	std::string resolvePath(const std::string &file) const;
>  
> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
> index 23be24ad9bf1..ff4d7fd18cda 100644
> --- a/src/libcamera/ipa_proxy.cpp
> +++ b/src/libcamera/ipa_proxy.cpp
> @@ -145,6 +145,16 @@ std::string IPAProxy::configurationFile(const std::string &name) const
>  	return std::string();
>  }
>  
> +/**
> + * \fn IPAProxy::stop()
> + * \brief Stop the IPA proxy
> + *
> + * This function stops the IPA and releases all the resources acquired by the
> + * proxy in start(). Calling stop() when the IPA proxy hasn't been started or
> + * has already been stopped is valid, the proxy shall treat this as a no-op and
> + * shall not forward the call to the IPA.
> + */
> +
>  /**
>   * \brief Find a valid full path for a proxy worker for a given executable name
>   * \param[in] file File name of proxy worker executable
> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
> index aa403e22f297..eead2883708d 100644
> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
> @@ -121,6 +121,9 @@ int IPAProxyThread::start()
>  
>  void IPAProxyThread::stop()
>  {
> +	if (!running_)
> +		return;
> +
>  	running_ = false;
>  
>  	proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h
index aec8f04ffc15..b429ce5a68a3 100644
--- a/include/libcamera/internal/ipa_proxy.h
+++ b/include/libcamera/internal/ipa_proxy.h
@@ -27,6 +27,8 @@  public:
 
 	std::string configurationFile(const std::string &file) const;
 
+	void stop() override = 0;
+
 protected:
 	std::string resolvePath(const std::string &file) const;
 
diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
index 23be24ad9bf1..ff4d7fd18cda 100644
--- a/src/libcamera/ipa_proxy.cpp
+++ b/src/libcamera/ipa_proxy.cpp
@@ -145,6 +145,16 @@  std::string IPAProxy::configurationFile(const std::string &name) const
 	return std::string();
 }
 
+/**
+ * \fn IPAProxy::stop()
+ * \brief Stop the IPA proxy
+ *
+ * This function stops the IPA and releases all the resources acquired by the
+ * proxy in start(). Calling stop() when the IPA proxy hasn't been started or
+ * has already been stopped is valid, the proxy shall treat this as a no-op and
+ * shall not forward the call to the IPA.
+ */
+
 /**
  * \brief Find a valid full path for a proxy worker for a given executable name
  * \param[in] file File name of proxy worker executable
diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
index aa403e22f297..eead2883708d 100644
--- a/src/libcamera/proxy/ipa_proxy_thread.cpp
+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
@@ -121,6 +121,9 @@  int IPAProxyThread::start()
 
 void IPAProxyThread::stop()
 {
+	if (!running_)
+		return;
+
 	running_ = false;
 
 	proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking);