From 3335d5a504374166f749a267ba1e1d803a0ed1f6 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Fri, 27 Aug 2021 04:45:28 +0300 Subject: libcamera: Drop emitter object pointer from signal arguments Many signals used in internal and public APIs carry the emitter pointer as a signal argument. This was done to allow slots connected to multiple signal instances to differentiate between emitters. While starting from a good intention of facilitating the implementation of slots, it turned out to be a bad API design as the signal isn't meant to know what it will be connected to, and thus shouldn't carry parameters that are solely meant to support a use case specific to the connected slot. These pointers turn out to be unused in all slots but one. In the only case where it is needed, it can be obtained by wrapping the slot in a lambda function when connecting the signal. Do so, and drop the emitter pointer from all signals. Signed-off-by: Laurent Pinchart Reviewed-by: Umang Jain --- include/libcamera/base/event_notifier.h | 2 +- include/libcamera/base/thread.h | 2 +- include/libcamera/base/timer.h | 2 +- include/libcamera/camera.h | 2 +- include/libcamera/internal/device_enumerator_udev.h | 2 +- include/libcamera/internal/ipc_pipe_unixsocket.h | 2 +- include/libcamera/internal/ipc_unixsocket.h | 4 ++-- include/libcamera/internal/media_device.h | 2 +- include/libcamera/internal/process.h | 4 ++-- include/libcamera/internal/v4l2_device.h | 2 +- include/libcamera/internal/v4l2_videodevice.h | 2 +- src/libcamera/base/event_dispatcher_poll.cpp | 4 ++-- src/libcamera/base/thread.cpp | 2 +- src/libcamera/camera.cpp | 2 +- src/libcamera/device_enumerator.cpp | 2 +- src/libcamera/device_enumerator_udev.cpp | 2 +- src/libcamera/ipc_pipe_unixsocket.cpp | 2 +- src/libcamera/ipc_unixsocket.cpp | 4 ++-- src/libcamera/pipeline_handler.cpp | 2 +- src/libcamera/process.cpp | 4 ++-- src/libcamera/v4l2_device.cpp | 3 +-- src/libcamera/v4l2_videodevice.cpp | 3 +-- test/event-thread.cpp | 2 +- test/event.cpp | 2 +- test/ipa/ipa_interface_test.cpp | 2 +- test/ipc/unixsocket.cpp | 4 ++-- test/ipc/unixsocket_ipc.cpp | 2 +- test/log/log_process.cpp | 3 +-- test/process/process_test.cpp | 3 +-- test/timer-thread.cpp | 2 +- test/timer.cpp | 2 +- .../generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +- 32 files changed, 38 insertions(+), 42 deletions(-) diff --git a/include/libcamera/base/event_notifier.h b/include/libcamera/base/event_notifier.h index 5055ccbf..f7722a32 100644 --- a/include/libcamera/base/event_notifier.h +++ b/include/libcamera/base/event_notifier.h @@ -34,7 +34,7 @@ public: bool enabled() const { return enabled_; } void setEnabled(bool enable); - Signal activated; + Signal<> activated; protected: void message(Message *msg) override; diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h index 762beab2..e0ca0aea 100644 --- a/include/libcamera/base/thread.h +++ b/include/libcamera/base/thread.h @@ -41,7 +41,7 @@ public: bool isRunning(); - Signal finished; + Signal<> finished; static Thread *current(); static pid_t currentId(); diff --git a/include/libcamera/base/timer.h b/include/libcamera/base/timer.h index 79882161..44876a85 100644 --- a/include/libcamera/base/timer.h +++ b/include/libcamera/base/timer.h @@ -33,7 +33,7 @@ public: std::chrono::steady_clock::time_point deadline() const { return deadline_; } - Signal timeout; + Signal<> timeout; protected: void message(Message *msg) override; diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 05cdab72..601ee46e 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -86,7 +86,7 @@ public: Signal bufferCompleted; Signal requestCompleted; - Signal disconnected; + Signal<> disconnected; int acquire(); int release(); diff --git a/include/libcamera/internal/device_enumerator_udev.h b/include/libcamera/internal/device_enumerator_udev.h index 58e64a29..c0352980 100644 --- a/include/libcamera/internal/device_enumerator_udev.h +++ b/include/libcamera/internal/device_enumerator_udev.h @@ -59,7 +59,7 @@ private: std::string lookupDeviceNode(dev_t devnum); int addV4L2Device(dev_t devnum); - void udevNotify(EventNotifier *notifier); + void udevNotify(); struct udev *udev_; struct udev_monitor *monitor_; diff --git a/include/libcamera/internal/ipc_pipe_unixsocket.h b/include/libcamera/internal/ipc_pipe_unixsocket.h index 4ffdddcc..ad2927fe 100644 --- a/include/libcamera/internal/ipc_pipe_unixsocket.h +++ b/include/libcamera/internal/ipc_pipe_unixsocket.h @@ -35,7 +35,7 @@ private: bool done; }; - void readyRead(IPCUnixSocket *socket); + void readyRead(); int call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response, uint32_t seq); diff --git a/include/libcamera/internal/ipc_unixsocket.h b/include/libcamera/internal/ipc_unixsocket.h index 9f5b0677..2b87196c 100644 --- a/include/libcamera/internal/ipc_unixsocket.h +++ b/include/libcamera/internal/ipc_unixsocket.h @@ -37,7 +37,7 @@ public: int send(const Payload &payload); int receive(Payload *payload); - Signal readyRead; + Signal<> readyRead; private: struct Header { @@ -48,7 +48,7 @@ private: int sendData(const void *buffer, size_t length, const int32_t *fds, unsigned int num); int recvData(void *buffer, size_t length, int32_t *fds, unsigned int num); - void dataNotifier(EventNotifier *notifier); + void dataNotifier(); int fd_; bool headerReceived_; diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h index 3a7722c2..1f2304c1 100644 --- a/include/libcamera/internal/media_device.h +++ b/include/libcamera/internal/media_device.h @@ -53,7 +53,7 @@ public: MediaLink *link(const MediaPad *source, const MediaPad *sink); int disableLinks(); - Signal disconnected; + Signal<> disconnected; protected: std::string logPrefix() const override; diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index c4d5d9c1..300e0521 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -38,7 +38,7 @@ public: void kill(); - Signal finished; + Signal finished; private: void closeAllFdsExcept(const std::vector &fds); @@ -70,7 +70,7 @@ public: private: static ProcessManager *self_; - void sighandler(EventNotifier *notifier); + void sighandler(); std::list processes_; diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index 423c8fb1..f21bc370 100644 --- a/include/libcamera/internal/v4l2_device.h +++ b/include/libcamera/internal/v4l2_device.h @@ -65,7 +65,7 @@ private: void updateControls(ControlList *ctrls, Span v4l2Ctrls); - void eventAvailable(EventNotifier *notifier); + void eventAvailable(); std::map controlInfo_; std::vector> controlIds_; diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 4a5d2cad..7a145f60 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -238,7 +238,7 @@ private: std::unique_ptr createBuffer(unsigned int index); FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane); - void bufferAvailable(EventNotifier *notifier); + void bufferAvailable(); FrameBuffer *dequeueBuffer(); V4L2Capability caps_; diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp index 4f22f579..3c9a126c 100644 --- a/src/libcamera/base/event_dispatcher_poll.cpp +++ b/src/libcamera/base/event_dispatcher_poll.cpp @@ -278,7 +278,7 @@ void EventDispatcherPoll::processNotifiers(const std::vector &pol } if (pfd.revents & event.events) - notifier->activated.emit(notifier); + notifier->activated.emit(); } /* Erase the notifiers_ entry if it is now empty. */ @@ -300,7 +300,7 @@ void EventDispatcherPoll::processTimers() timers_.pop_front(); timer->stop(); - timer->timeout.emit(timer); + timer->timeout.emit(); } } diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp index bd7b7391..d0ca30e3 100644 --- a/src/libcamera/base/thread.cpp +++ b/src/libcamera/base/thread.cpp @@ -384,7 +384,7 @@ void Thread::finishThread() data_->running_ = false; data_->mutex_.unlock(); - finished.emit(this); + finished.emit(); data_->cv_.notify_all(); } diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 98438312..71809bcd 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -688,7 +688,7 @@ void Camera::disconnect() LOG(Camera, Debug) << "Disconnecting camera " << id(); _d()->disconnect(); - disconnected.emit(this); + disconnected.emit(); } int Camera::exportFrameBuffers(Stream *stream, diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index ec59927e..d1258050 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -288,7 +288,7 @@ void DeviceEnumerator::removeDevice(const std::string &deviceNode) LOG(DeviceEnumerator, Debug) << "Media device for node " << deviceNode << " removed."; - media->disconnected.emit(media.get()); + media->disconnected.emit(); } /** diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index 37a2c5aa..5317afbd 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -327,7 +327,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum) return 0; } -void DeviceEnumeratorUdev::udevNotify([[maybe_unused]] EventNotifier *notifier) +void DeviceEnumeratorUdev::udevNotify() { struct udev_device *dev = udev_monitor_receive_device(monitor_); std::string action(udev_device_get_action(dev)); diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp index 38bcc30a..533560cf 100644 --- a/src/libcamera/ipc_pipe_unixsocket.cpp +++ b/src/libcamera/ipc_pipe_unixsocket.cpp @@ -82,7 +82,7 @@ int IPCPipeUnixSocket::sendAsync(const IPCMessage &data) return 0; } -void IPCPipeUnixSocket::readyRead([[maybe_unused]] IPCUnixSocket *socket) +void IPCPipeUnixSocket::readyRead() { IPCUnixSocket::Payload payload; int ret = socket_->receive(&payload); diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp index 7188cf29..bd32fca3 100644 --- a/src/libcamera/ipc_unixsocket.cpp +++ b/src/libcamera/ipc_unixsocket.cpp @@ -311,7 +311,7 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, return 0; } -void IPCUnixSocket::dataNotifier([[maybe_unused]] EventNotifier *notifier) +void IPCUnixSocket::dataNotifier() { int ret; @@ -342,7 +342,7 @@ void IPCUnixSocket::dataNotifier([[maybe_unused]] EventNotifier *notifier) return; notifier_->setEnabled(false); - readyRead.emit(this); + readyRead.emit(); } } /* namespace libcamera */ diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 597d4c6a..f69c4f03 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -448,7 +448,7 @@ void PipelineHandler::registerCamera(std::shared_ptr camera) */ void PipelineHandler::hotplugMediaDevice(MediaDevice *media) { - media->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected); + media->disconnected.connect(this, [=]() { mediaDeviceDisconnected(media); }); } /** diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 998d08c2..eca1b300 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -66,7 +66,7 @@ void sigact(int signal, siginfo_t *info, void *ucontext) } /* namespace */ -void ProcessManager::sighandler([[maybe_unused]] EventNotifier *notifier) +void ProcessManager::sighandler() { char data; ssize_t ret = read(pipe_[0], &data, sizeof(data)); @@ -326,7 +326,7 @@ void Process::died(int wstatus) exitStatus_ = WIFEXITED(wstatus) ? NormalExit : SignalExit; exitCode_ = exitStatus_ == NormalExit ? WEXITSTATUS(wstatus) : -1; - finished.emit(this, exitStatus_, exitCode_); + finished.emit(exitStatus_, exitCode_); } /** diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 951592c6..9c783c9c 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -705,12 +705,11 @@ void V4L2Device::updateControls(ControlList *ctrls, /** * \brief Slot to handle V4L2 events from the V4L2 device - * \param[in] notifier The event notifier * * When this slot is called, a V4L2 event is available to be dequeued from the * device. */ -void V4L2Device::eventAvailable([[maybe_unused]] EventNotifier *notifier) +void V4L2Device::eventAvailable() { struct v4l2_event event{}; int ret = ioctl(VIDIOC_DQEVENT, &event); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 9d35618d..4e1c2b7c 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1524,7 +1524,6 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) /** * \brief Slot to handle completed buffer events from the V4L2 video device - * \param[in] notifier The event notifier * * When this slot is called, a Buffer has become available from the device, and * will be emitted through the bufferReady Signal. @@ -1532,7 +1531,7 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) * For Capture video devices the FrameBuffer will contain valid data. * For Output video devices the FrameBuffer can be considered empty. */ -void V4L2VideoDevice::bufferAvailable([[maybe_unused]] EventNotifier *notifier) +void V4L2VideoDevice::bufferAvailable() { FrameBuffer *buffer = dequeueBuffer(); if (!buffer) diff --git a/test/event-thread.cpp b/test/event-thread.cpp index 12021710..ef8a52c3 100644 --- a/test/event-thread.cpp +++ b/test/event-thread.cpp @@ -66,7 +66,7 @@ public: } private: - void readReady([[maybe_unused]] EventNotifier *notifier) + void readReady() { size_ = read(notifier_->fd(), data_, sizeof(data_)); notified_ = true; diff --git a/test/event.cpp b/test/event.cpp index e338335c..d4765eb1 100644 --- a/test/event.cpp +++ b/test/event.cpp @@ -22,7 +22,7 @@ using namespace libcamera; class EventTest : public Test { protected: - void readReady([[maybe_unused]] EventNotifier *notifier) + void readReady() { size_ = read(notifier_->fd(), data_, sizeof(data_)); notified_ = true; diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 0ee51f71..43562e60 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -153,7 +153,7 @@ protected: } private: - void readTrace([[maybe_unused]] EventNotifier *notifier) + void readTrace() { ssize_t s = read(notifier_->fd(), &trace_, sizeof(trace_)); if (s < 0) { diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp index 6507fb12..7270bf4d 100644 --- a/test/ipc/unixsocket.cpp +++ b/test/ipc/unixsocket.cpp @@ -68,7 +68,7 @@ public: } private: - void readyRead([[maybe_unused]] IPCUnixSocket *ipc) + void readyRead() { IPCUnixSocket::Payload message, response; int ret; @@ -447,7 +447,7 @@ private: return 0; } - void readyRead([[maybe_unused]] IPCUnixSocket *ipc) + void readyRead() { if (!callResponse_) { cerr << "Read ready without expecting data, fail." << endl; diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp index 60317a49..ab5d2557 100644 --- a/test/ipc/unixsocket_ipc.cpp +++ b/test/ipc/unixsocket_ipc.cpp @@ -65,7 +65,7 @@ public: } private: - void readyRead([[maybe_unused]] IPCUnixSocket *ipc) + void readyRead() { IPCUnixSocket::Payload message; int ret; diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp index d138aa7f..a56a3998 100644 --- a/test/log/log_process.cpp +++ b/test/log/log_process.cpp @@ -126,8 +126,7 @@ protected: } private: - void procFinished([[maybe_unused]] Process *proc, - enum Process::ExitStatus exitStatus, int exitCode) + void procFinished(enum Process::ExitStatus exitStatus, int exitCode) { exitStatus_ = exitStatus; exitCode_ = exitCode; diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp index 8f7a1f05..378d680b 100644 --- a/test/process/process_test.cpp +++ b/test/process/process_test.cpp @@ -81,8 +81,7 @@ protected: } private: - void procFinished([[maybe_unused]] Process *proc, - enum Process::ExitStatus exitStatus, int exitCode) + void procFinished(enum Process::ExitStatus exitStatus, int exitCode) { exitStatus_ = exitStatus; exitCode_ = exitCode; diff --git a/test/timer-thread.cpp b/test/timer-thread.cpp index 2c14865b..f7e8743d 100644 --- a/test/timer-thread.cpp +++ b/test/timer-thread.cpp @@ -39,7 +39,7 @@ public: } private: - void timeoutHandler([[maybe_unused]] Timer *timer) + void timeoutHandler() { timeout_ = true; } diff --git a/test/timer.cpp b/test/timer.cpp index 88f226e7..be79d010 100644 --- a/test/timer.cpp +++ b/test/timer.cpp @@ -56,7 +56,7 @@ public: } private: - void timeoutHandler([[maybe_unused]] Timer *timer) + void timeoutHandler() { expiration_ = std::chrono::steady_clock::now(); count_++; diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl index b4cd1aa9..c54ecdb9 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl @@ -57,7 +57,7 @@ public: ~{{proxy_worker_name}}() {} - void readyRead([[maybe_unused]] IPCUnixSocket *socket) + void readyRead() { IPCUnixSocket::Payload _message; int _retRecv = socket_.receive(&_message); -- cgit v1.2.1