| Message ID | 20210827023829.5871-7-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 3335d5a504374166f749a267ba1e1d803a0ed1f6 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 8/27/21 8:08 AM, Laurent Pinchart wrote: > 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. hmm, When you said, the emitter object pointer is not required during signal emission, i initially thought, it's not required to be explicitly to be mentioned the argument list - during the signal emission for e.g. class XYZ: .... Signal<XYZ *, Arg A, Arg B> sig; becomes class XYZ: .... Signal<Arg A, Arg B> sig; But the slot essentially remained unchanged for both cases i.e. for every signal, the passing of emitter pointer can be automatic/implicit to: signalSlot(XYZ *, Arg A, Arg B) {} Well, you are right about how the pointers mostly turned out to be unused, so I am not opposed with series as such. I suspect some cases will still pop up, where we do need to emitter pointer in the slot, but as I see below, we can address those with a lamda func. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > 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 +- > .../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 5055ccbf21ca..f7722a32ef55 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<EventNotifier *> activated; > + Signal<> activated; > > protected: > void message(Message *msg) override; > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h > index 762beab2a360..e0ca0aeaa761 100644 > --- a/include/libcamera/base/thread.h > +++ b/include/libcamera/base/thread.h > @@ -41,7 +41,7 @@ public: > > bool isRunning(); > > - Signal<Thread *> 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 798821611c6c..44876a85dc0a 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<Timer *> timeout; > + Signal<> timeout; > > protected: > void message(Message *msg) override; > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 05cdab724b10..601ee46e415b 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -86,7 +86,7 @@ public: > > Signal<Request *, FrameBuffer *> bufferCompleted; > Signal<Request *> requestCompleted; > - Signal<Camera *> 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 58e64a297b1d..c035298081b4 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 4ffdddcc7f92..ad2927fed7f0 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 9f5b06773ced..2b87196c4851 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<IPCUnixSocket *> 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 3a7722c2a215..1f2304c19281 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<MediaDevice *> disconnected; > + Signal<> disconnected; > > protected: > std::string logPrefix() const override; > diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h > index c4d5d9c1c009..300e0521eb03 100644 > --- a/include/libcamera/internal/process.h > +++ b/include/libcamera/internal/process.h > @@ -38,7 +38,7 @@ public: > > void kill(); > > - Signal<Process *, enum ExitStatus, int> finished; > + Signal<enum ExitStatus, int> finished; > > private: > void closeAllFdsExcept(const std::vector<int> &fds); > @@ -70,7 +70,7 @@ public: > private: > static ProcessManager *self_; > > - void sighandler(EventNotifier *notifier); > + void sighandler(); > > std::list<Process *> processes_; > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > index 423c8fb11845..f21bc3701ca0 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<const v4l2_ext_control> v4l2Ctrls); > > - void eventAvailable(EventNotifier *notifier); > + void eventAvailable(); > > std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_; > std::vector<std::unique_ptr<ControlId>> controlIds_; > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > index e767ec84c4da..400d4490d016 100644 > --- a/include/libcamera/internal/v4l2_videodevice.h > +++ b/include/libcamera/internal/v4l2_videodevice.h > @@ -238,7 +238,7 @@ private: > std::unique_ptr<FrameBuffer> 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 4f22f5793bb9..3c9a126c0bd6 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<struct pollfd> &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 bd7b73911d12..d0ca30e3d522 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 c20e05014fb9..b5849d4d401a 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 ec59927eaf34..d12580505303 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 37a2c5aa55db..5317afbd77ce 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 38bcc30a21ed..533560cf95d3 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 7188cf29e56a..bd32fca3a678 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 597d4c6a578a..f69c4f03b80f 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -448,7 +448,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> 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 998d08c2d88a..eca1b30039b8 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 951592c698f7..9c783c9cbed1 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 ce60dff6cdfd..1684d0e9c8dd 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1467,7 +1467,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. > @@ -1475,7 +1474,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 12021710ef41..ef8a52c3de55 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 e338335c11e8..d4765eb14d12 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 0ee51f71fd87..43562e608506 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 6507fb12d74b..7270bf4d2fe7 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 60317a4956b8..ab5d25572d83 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 d138aa7ff562..a56a399848a7 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 8f7a1f05f681..378d680bf4ef 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 2c14865b74d5..f7e8743da9e6 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 88f226e79f5f..be79d0100a58 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 b4cd1aa9e823..c54ecdb90a1a 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);
Hi Umang, On Mon, Aug 30, 2021 at 05:35:37PM +0530, Umang Jain wrote: > On 8/27/21 8:08 AM, Laurent Pinchart wrote: > > 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. > > hmm, When you said, the emitter object pointer is not required during > signal emission, i initially thought, it's not required to be explicitly > to be mentioned the argument list - during the signal emission for e.g. > > class XYZ: > > .... > Signal<XYZ *, Arg A, Arg B> sig; > > becomes > > class XYZ: > > .... > Signal<Arg A, Arg B> sig; > > But the slot essentially remained unchanged for both cases i.e. for > every signal, the passing of emitter pointer can be automatic/implicit to: > > signalSlot(XYZ *, Arg A, Arg B) > > {} > > > Well, you are right about how the pointers mostly turned out to be > unused, so I am not opposed with series as such. I suspect some cases > will still pop up, where we do need to emitter pointer in the slot, but > as I see below, we can address those with a lamda func. Yes, that's the idea. From the point of view of the emitter, having an emitter pointer as a signal argument is really about passing context information, it's not per-emission data. As you've seen, the vast majority of slots (all but one) already have the context. For the only slot that doesn't, using a lambda function is fairly trivial, and the little overhead we have there is I think worse the gain in all other case, as well as the neater design of not having to add a signal context argument that is of no use to the emitter. It creates a better separation between the design of the emitter and the design of the receiver, leading to more reusable components. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > > --- > > 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 +- > > .../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 5055ccbf21ca..f7722a32ef55 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<EventNotifier *> activated; > > + Signal<> activated; > > > > protected: > > void message(Message *msg) override; > > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h > > index 762beab2a360..e0ca0aeaa761 100644 > > --- a/include/libcamera/base/thread.h > > +++ b/include/libcamera/base/thread.h > > @@ -41,7 +41,7 @@ public: > > > > bool isRunning(); > > > > - Signal<Thread *> 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 798821611c6c..44876a85dc0a 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<Timer *> timeout; > > + Signal<> timeout; > > > > protected: > > void message(Message *msg) override; > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > > index 05cdab724b10..601ee46e415b 100644 > > --- a/include/libcamera/camera.h > > +++ b/include/libcamera/camera.h > > @@ -86,7 +86,7 @@ public: > > > > Signal<Request *, FrameBuffer *> bufferCompleted; > > Signal<Request *> requestCompleted; > > - Signal<Camera *> 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 58e64a297b1d..c035298081b4 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 4ffdddcc7f92..ad2927fed7f0 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 9f5b06773ced..2b87196c4851 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<IPCUnixSocket *> 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 3a7722c2a215..1f2304c19281 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<MediaDevice *> disconnected; > > + Signal<> disconnected; > > > > protected: > > std::string logPrefix() const override; > > diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h > > index c4d5d9c1c009..300e0521eb03 100644 > > --- a/include/libcamera/internal/process.h > > +++ b/include/libcamera/internal/process.h > > @@ -38,7 +38,7 @@ public: > > > > void kill(); > > > > - Signal<Process *, enum ExitStatus, int> finished; > > + Signal<enum ExitStatus, int> finished; > > > > private: > > void closeAllFdsExcept(const std::vector<int> &fds); > > @@ -70,7 +70,7 @@ public: > > private: > > static ProcessManager *self_; > > > > - void sighandler(EventNotifier *notifier); > > + void sighandler(); > > > > std::list<Process *> processes_; > > > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > index 423c8fb11845..f21bc3701ca0 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<const v4l2_ext_control> v4l2Ctrls); > > > > - void eventAvailable(EventNotifier *notifier); > > + void eventAvailable(); > > > > std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_; > > std::vector<std::unique_ptr<ControlId>> controlIds_; > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h > > index e767ec84c4da..400d4490d016 100644 > > --- a/include/libcamera/internal/v4l2_videodevice.h > > +++ b/include/libcamera/internal/v4l2_videodevice.h > > @@ -238,7 +238,7 @@ private: > > std::unique_ptr<FrameBuffer> 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 4f22f5793bb9..3c9a126c0bd6 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<struct pollfd> &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 bd7b73911d12..d0ca30e3d522 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 c20e05014fb9..b5849d4d401a 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 ec59927eaf34..d12580505303 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 37a2c5aa55db..5317afbd77ce 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 38bcc30a21ed..533560cf95d3 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 7188cf29e56a..bd32fca3a678 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 597d4c6a578a..f69c4f03b80f 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -448,7 +448,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> 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 998d08c2d88a..eca1b30039b8 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 951592c698f7..9c783c9cbed1 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 ce60dff6cdfd..1684d0e9c8dd 100644 > > --- a/src/libcamera/v4l2_videodevice.cpp > > +++ b/src/libcamera/v4l2_videodevice.cpp > > @@ -1467,7 +1467,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. > > @@ -1475,7 +1474,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 12021710ef41..ef8a52c3de55 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 e338335c11e8..d4765eb14d12 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 0ee51f71fd87..43562e608506 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 6507fb12d74b..7270bf4d2fe7 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 60317a4956b8..ab5d25572d83 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 d138aa7ff562..a56a399848a7 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 8f7a1f05f681..378d680bf4ef 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 2c14865b74d5..f7e8743da9e6 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 88f226e79f5f..be79d0100a58 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 b4cd1aa9e823..c54ecdb90a1a 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);
diff --git a/include/libcamera/base/event_notifier.h b/include/libcamera/base/event_notifier.h index 5055ccbf21ca..f7722a32ef55 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<EventNotifier *> activated; + Signal<> activated; protected: void message(Message *msg) override; diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h index 762beab2a360..e0ca0aeaa761 100644 --- a/include/libcamera/base/thread.h +++ b/include/libcamera/base/thread.h @@ -41,7 +41,7 @@ public: bool isRunning(); - Signal<Thread *> 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 798821611c6c..44876a85dc0a 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<Timer *> timeout; + Signal<> timeout; protected: void message(Message *msg) override; diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 05cdab724b10..601ee46e415b 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -86,7 +86,7 @@ public: Signal<Request *, FrameBuffer *> bufferCompleted; Signal<Request *> requestCompleted; - Signal<Camera *> 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 58e64a297b1d..c035298081b4 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 4ffdddcc7f92..ad2927fed7f0 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 9f5b06773ced..2b87196c4851 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<IPCUnixSocket *> 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 3a7722c2a215..1f2304c19281 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<MediaDevice *> disconnected; + Signal<> disconnected; protected: std::string logPrefix() const override; diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index c4d5d9c1c009..300e0521eb03 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -38,7 +38,7 @@ public: void kill(); - Signal<Process *, enum ExitStatus, int> finished; + Signal<enum ExitStatus, int> finished; private: void closeAllFdsExcept(const std::vector<int> &fds); @@ -70,7 +70,7 @@ public: private: static ProcessManager *self_; - void sighandler(EventNotifier *notifier); + void sighandler(); std::list<Process *> processes_; diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index 423c8fb11845..f21bc3701ca0 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<const v4l2_ext_control> v4l2Ctrls); - void eventAvailable(EventNotifier *notifier); + void eventAvailable(); std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_; std::vector<std::unique_ptr<ControlId>> controlIds_; diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index e767ec84c4da..400d4490d016 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -238,7 +238,7 @@ private: std::unique_ptr<FrameBuffer> 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 4f22f5793bb9..3c9a126c0bd6 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<struct pollfd> &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 bd7b73911d12..d0ca30e3d522 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 c20e05014fb9..b5849d4d401a 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 ec59927eaf34..d12580505303 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 37a2c5aa55db..5317afbd77ce 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 38bcc30a21ed..533560cf95d3 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 7188cf29e56a..bd32fca3a678 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 597d4c6a578a..f69c4f03b80f 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -448,7 +448,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> 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 998d08c2d88a..eca1b30039b8 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 951592c698f7..9c783c9cbed1 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 ce60dff6cdfd..1684d0e9c8dd 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1467,7 +1467,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. @@ -1475,7 +1474,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 12021710ef41..ef8a52c3de55 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 e338335c11e8..d4765eb14d12 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 0ee51f71fd87..43562e608506 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 6507fb12d74b..7270bf4d2fe7 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 60317a4956b8..ab5d25572d83 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 d138aa7ff562..a56a399848a7 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 8f7a1f05f681..378d680bf4ef 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 2c14865b74d5..f7e8743da9e6 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 88f226e79f5f..be79d0100a58 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 b4cd1aa9e823..c54ecdb90a1a 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);
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 <laurent.pinchart@ideasonboard.com> --- 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 +- .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl | 2 +- 32 files changed, 38 insertions(+), 42 deletions(-)