[libcamera-devel] [PATCH v2 00/11] libcamera: Replace CameraData with Camera::Private

[Laurent Pinchart]

Hello,

This patch series reworks the libcamera core and pipeline handlers to
drop the CameraData class, replacing it with Camera::Private.

The Camera::Private class implements the d-pointer design pattern for
the Camera class, and stores internal camera data that shouldn't be
visible through the public API. The CameraData class is a container used
by pipeline handlers to store per-camera private data. The two classes
thus overlap in their purpose.

This series cleans up this situation by merging CameraData into
Camera::Private. Pipeline handlers then subclass Camera::Private instead
of CameraData, and pass the private data directly to the Camera
constructor.

The first 7 patches from the previous RFC version, preparing for this
change, have been merged already. This version starts, in 01/11, by
passing the pointer to the private data to the Camera constructor, to
allow subclassing Camera::Private in pipeline handlers. An alternative
would be to force pipeline handlers to subclass the Camera class, which
I believe should be doable (I haven't tried it though) but would be
a more intrusive change in the pipeline handlers. Decoupling the
instantiation of the public and private classes doesn't seem to result
in awful code, but it requires the public class constructor to manually
initialize the private class instance (at least in some cases). I'm not
entirely sure if this is a slipery slope or not.

After that, changes are fairly straightforward. Patch 09/11 moves the
members of the existing CameraData class to Camera::Private. Patches
02/11 to 08/11 update pipeline handlers individually to migrate to
Camera::Private. Patch 09/11 then cleans up by dropping the CameraData
class, and patch 10/11 updates the pipeline handler guide accordingly.

Finally, patch 11/11 is a cleanup (suggested by Jacopo) that drops the
indirection in the implementation of Camera::controls() and
Camera::properties(). It looks quite neat in my opinion, and worth the
effort.

All pipeline handlers have been compile-tested, and I've runtime-tested
uvcvideo and vimc. Once the approach gets approved (or rejected, and the
series rewritten to subclass Camera instead), I'll test the other
pipeline handlers.

Laurent Pinchart (11):
  libcamera: camera: Pass Private pointer to Camera constructor
  libcamera: pipeline_handler: Move CameraData members to
    Camera::Private
  libcamera: pipeline: simple: Migrate to Camera::Private
  libcamera: pipeline: uvcvideo: Migrate to Camera::Private
  libcamera: pipeline: vimc: Migrate to Camera::Private
  libcamera: pipeline: rkisp1: Migrate to Camera::Private
  libcamera: pipeline: raspberrypi: Migrate to Camera::Private
  libcamera: pipeline: ipu3: Migrate to Camera::Private
  libcamera: pipeline_handler: Drop CameraData class
  Documentation: guides: pipeline-handler: Migrate to Camera::Private
  libcamera: pipeline_handler: Drop controls() and properties()
    functions

 Documentation/Doxyfile.in                     |   1 -
 Documentation/guides/pipeline-handler.rst     |  67 ++++-----
 include/libcamera/camera.h                    |   5 +-
 include/libcamera/internal/camera.h           |  11 +-
 include/libcamera/internal/pipeline_handler.h |  32 +---
 src/libcamera/camera.cpp                      |  89 +++++++++--
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  38 ++---
 .../pipeline/raspberrypi/raspberrypi.cpp      |  24 +--
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  26 ++--
 src/libcamera/pipeline/simple/simple.cpp      |  25 ++--
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  19 +--
 src/libcamera/pipeline/vimc/vimc.cpp          |  19 +--
 src/libcamera/pipeline_handler.cpp            | 138 +-----------------
 13 files changed, 210 insertions(+), 284 deletions(-)

-- 
Regards,

Laurent Pinchart