[PATCH 1/4] libcamera: Add signal disconnected for IPC
Kieran Bingham
kieran.bingham at ideasonboard.com
Sun Oct 20 00:44:07 CEST 2024
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?) or is there some reason for the IP
>
> 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