[libcamera-devel] [PATCH] libcamera: ipa_proxy: Allow stop() on a stopped IPA
Jacopo Mondi
jacopo at jmondi.org
Mon Jul 6 13:56:48 CEST 2020
Hi Laurent,
On Mon, Jul 06, 2020 at 02:51:29PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Jul 06, 2020 at 01:49:13PM +0200, Jacopo Mondi wrote:
> > On Mon, Jul 06, 2020 at 02:08:47PM +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 at ideasonboard.com>
> > > ---
> > > 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..01740a6e39ec 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 method informs the IPA module that the camera is stopped. The IPA module
> > > + * shall release resources prepared in start(). Calling stop() when the IPA
> >
> > s/prepared/acquired ?
> >
> > > + * hasn't been started or has already been stopped is valid, and the IPA shall
> > > + * treat this as a no-op.
> > > + */
> > > +
> > > /**
> > > * \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;
> > > +
> >
> > In case the IPA is then stopped already, IPA won't receive the call,
> > making the above "hasn't been started or has already been stopped is
> > valid" possibly confusing.
>
> I agree with you. How about the following ?
>
> * 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.
>
Looks good, thanks!
> > With out without these addressed
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > > running_ = false;
> > >
> > > proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking);
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list