[PATCH 1/4] libcamera: Add signal disconnected for IPC
Kieran Bingham
kieran.bingham at ideasonboard.com
Sun Oct 20 12:30:45 CEST 2024
Quoting Kieran Bingham (2024-10-19 23:44:07)
> Quoting Harvey Yang (2024-10-18 08:57:34)
> > This CL also adds an API in Camera::Private to trigger
> > Camera::disconnected signal.
>
> CL?
>
>
> Please read: https://cbea.ms/git-commit/#why-not-how
>
> But adding a signal on this sounds like a good idea to me.
>
> How does the disconnected signal work. Is it reported when the IPA is
> closed? (or crashes?)
Sorry - somehow I dropped a sentence here:
> or is there some reason for the IP
... or is there some other reason for the IPC to be disconnected ?
>
> >
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > Co-developed-by: Han-Lin Chen <hanlinchen at chromium.org>
> > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > ---
> > include/libcamera/internal/camera.h | 2 ++
> > .../libcamera/internal/ipc_pipe_unixsocket.h | 2 ++
> > include/libcamera/internal/ipc_unixsocket.h | 2 ++
> > src/libcamera/camera.cpp | 10 ++++++++
> > src/libcamera/ipc_pipe_unixsocket.cpp | 8 +++++++
> > src/libcamera/ipc_unixsocket.cpp | 24 +++++++++++++++++--
> > .../module_ipa_proxy.cpp.tmpl | 8 +++++++
> > .../module_ipa_proxy.h.tmpl | 2 ++
> > 8 files changed, 56 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
> > index 0add0428b..0bef0980e 100644
> > --- a/include/libcamera/internal/camera.h
> > +++ b/include/libcamera/internal/camera.h
> > @@ -33,6 +33,8 @@ public:
> >
> > PipelineHandler *pipe() { return pipe_.get(); }
> >
> > + void notifyDisconnection();
> > +
> > std::list<Request *> queuedRequests_;
> > ControlInfoMap controlInfo_;
> > ControlList properties_;
> > diff --git a/include/libcamera/internal/ipc_pipe_unixsocket.h b/include/libcamera/internal/ipc_pipe_unixsocket.h
> > index 8c972613f..09f65b102 100644
> > --- a/include/libcamera/internal/ipc_pipe_unixsocket.h
> > +++ b/include/libcamera/internal/ipc_pipe_unixsocket.h
> > @@ -28,6 +28,8 @@ public:
> >
> > int sendAsync(const IPCMessage &data) override;
> >
> > + Signal<> *disconnected();
> > +
> > private:
> > struct CallData {
> > IPCUnixSocket::Payload *response;
> > diff --git a/include/libcamera/internal/ipc_unixsocket.h b/include/libcamera/internal/ipc_unixsocket.h
> > index 48bb7a942..a1e326b6b 100644
> > --- a/include/libcamera/internal/ipc_unixsocket.h
> > +++ b/include/libcamera/internal/ipc_unixsocket.h
> > @@ -39,6 +39,8 @@ public:
> >
> > Signal<> readyRead;
> >
> > + Signal<> disconnected;
> > +
> > private:
> > struct Header {
> > uint32_t data;
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index a86f552a4..ef5a6725f 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -603,6 +603,16 @@ Camera::Private::~Private()
> > * \return The pipeline handler that created this camera
> > */
> >
> > +/**
> > + * \brief Notify the application that camera is disconnected with signal
> > + * Camera::disconnected
> > + */
> > +void Camera::Private::notifyDisconnection()
> > +{
> > + Camera *o = LIBCAMERA_O_PTR();
> > + o->disconnected.emit();
> > +}
> > +
> > /**
> > * \fn Camera::Private::validator()
> > * \brief Retrieve the control validator related to this camera
> > diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
> > index 668ec73b9..51fd3b1fb 100644
> > --- a/src/libcamera/ipc_pipe_unixsocket.cpp
> > +++ b/src/libcamera/ipc_pipe_unixsocket.cpp
> > @@ -84,6 +84,14 @@ int IPCPipeUnixSocket::sendAsync(const IPCMessage &data)
> > return 0;
> > }
> >
> > +Signal<> *IPCPipeUnixSocket::disconnected()
> > +{
> > + if (socket_)
> > + return &socket_->disconnected;
> > +
> > + return nullptr;
> > +}
> > +
> > void IPCPipeUnixSocket::readyRead()
> > {
> > IPCUnixSocket::Payload payload;
> > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp
> > index 002053e35..3d0248857 100644
> > --- a/src/libcamera/ipc_unixsocket.cpp
> > +++ b/src/libcamera/ipc_unixsocket.cpp
> > @@ -186,11 +186,16 @@ int IPCUnixSocket::send(const Payload &payload)
> > if (!hdr.data && !hdr.fds)
> > return -EINVAL;
> >
> > - ret = ::send(fd_.get(), &hdr, sizeof(hdr), 0);
> > + ret = ::send(fd_.get(), &hdr, sizeof(hdr), MSG_NOSIGNAL);
>
> What's this flag change for? Can you explain in the commit message or as
> a comment please?
>
> > if (ret < 0) {
> > ret = -errno;
> > LOG(IPCUnixSocket, Error)
> > << "Failed to send: " << strerror(-ret);
> > + if (errno == ECONNRESET) {
> > + disconnected.emit();
> > + fd_.reset();
>
> So this is really about if the IPA connection gets severed indeed?
>
> > + }
> > +
> > return ret;
> > }
> >
> > @@ -241,6 +246,11 @@ int IPCUnixSocket::receive(Payload *payload)
> > * \brief A Signal emitted when a message is ready to be read
> > */
> >
> > +/**
> > + * \var IPCUnixSocket::disconnected
> > + * \brief A Signal emitted when the Unix socket IPC is disconnected
> > + */
> > +
> > int IPCUnixSocket::sendData(const void *buffer, size_t length,
> > const int32_t *fds, unsigned int num)
> > {
> > @@ -266,10 +276,15 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,
> > if (fds)
> > memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t));
> >
> > - if (sendmsg(fd_.get(), &msg, 0) < 0) {
> > + if (sendmsg(fd_.get(), &msg, MSG_NOSIGNAL) < 0) {
> > int ret = -errno;
> > LOG(IPCUnixSocket, Error)
> > << "Failed to sendmsg: " << strerror(-ret);
> > + if (errno == ECONNRESET) {
> > + disconnected.emit();
> > + fd_.reset();
> > + }
> > +
> > return ret;
> > }
> >
> > @@ -324,6 +339,11 @@ void IPCUnixSocket::dataNotifier()
> > ret = -errno;
> > LOG(IPCUnixSocket, Error)
> > << "Failed to receive header: " << strerror(-ret);
> > + if (errno == ECONNRESET) {
> > + disconnected.emit();
> > + fd_.reset();
> > + }
> > +
> > return;
> > }
> >
> > diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > index ce3cc5ab6..27f03417a 100644
> > --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> > @@ -104,6 +104,14 @@ namespace {{ns}} {
> > }
> > }
> >
> > +Signal<> *{{proxy_name}}::disconnected()
> > +{
> > + if (ipc_)
> > + return ipc_->disconnected();
> > +
> > + return nullptr;
> > +}
> > +
> > {% if interface_event.methods|length > 0 %}
> > void {{proxy_name}}::recvMessage(const IPCMessage &data)
> > {
> > diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> > index e213b18a0..2b7ba872e 100644
> > --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> > +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> > @@ -53,6 +53,8 @@ public:
> > > {{method.mojom_name}};
> > {% endfor %}
> >
> > + Signal<> *disconnected();
> > +
> > private:
> > void recvMessage(const IPCMessage &data);
> >
> > --
> > 2.47.0.rc1.288.g06298d1525-goog
> >
More information about the libcamera-devel
mailing list