[libcamera-devel,v1,0/6] libcamera: Drop emitter object pointer from signal arguments

13521 mbox series

Hello,

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.

This series fixes the issue. It starts with four patches that rework the
BoundMethod and Signal implementation. Patch 1/6 is a drive-by fix that
uses a simpler and less costly signal disconnection function when
applicable. Patches 2/6 and 3/6 then simplify the BoundMethod
implementation by removing two template specializations that turned out
to be replaceable by simpler options. Patch 4/6 adds support for
connecting signals to functors (and in particular to lambda functions),
which is needed to address the problem at hand.

Patch 5/6 reworks the implementation of some slots to avoid using the
emitter pointer argument when the same object is available through a
class member. Patch 6/6 finally drops the emitter pointer from signals,
replacing the only case where the pointer is actually needed with a
lambda wrapper.

Laurent Pinchart (6):
  libcamera: Use simpler Signal::disconnect() function
  libcamera: base: bound_method: Remove BoundMethodMember specialization
  libcamera: base: bound_method: Remove BoundMethodArgs specialization
  libcamera: base: signal: Support connecting signals to functors
  libcamera: Don't use emitter object pointer argument to slot
  libcamera: Drop emitter object pointer from signal arguments

 Documentation/Doxyfile.in                     |   1 +
 include/libcamera/base/bound_method.h         | 111 ++++++++----------
 include/libcamera/base/event_notifier.h       |   2 +-
 include/libcamera/base/signal.h               |  19 +++
 include/libcamera/base/thread.h               |   2 +-
 include/libcamera/base/timer.h                |   2 +-
 include/libcamera/camera.h                    |   2 +-
 .../internal/device_enumerator_udev.h         |   2 +-
 .../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/lc-compliance/simple_capture.cpp          |   2 +-
 src/libcamera/base/event_dispatcher_poll.cpp  |   4 +-
 src/libcamera/base/signal.cpp                 |  24 ++++
 src/libcamera/base/thread.cpp                 |   2 +-
 src/libcamera/camera.cpp                      |   2 +-
 src/libcamera/camera_manager.cpp              |   2 +-
 src/libcamera/device_enumerator.cpp           |   2 +-
 src/libcamera/device_enumerator_udev.cpp      |   2 +-
 src/libcamera/ipc_pipe_unixsocket.cpp         |   4 +-
 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 +-
 src/qcam/main_window.cpp                      |   4 +-
 test/camera/camera_reconfigure.cpp            |   2 +-
 test/event-thread.cpp                         |   4 +-
 test/event.cpp                                |  17 ++-
 test/ipa/ipa_interface_test.cpp               |   4 +-
 test/ipc/unixsocket.cpp                       |   8 +-
 test/ipc/unixsocket_ipc.cpp                   |   4 +-
 test/log/log_process.cpp                      |   3 +-
 test/process/process_test.cpp                 |   3 +-
 test/signal.cpp                               |  39 ++++++
 test/timer-thread.cpp                         |   2 +-
 test/timer.cpp                                |   2 +-
 .../module_ipa_proxy_worker.cpp.tmpl          |   4 +-
 41 files changed, 192 insertions(+), 121 deletions(-)