From patchwork Fri Jul 23 04:00:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 13086 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 3E262C322B for ; Fri, 23 Jul 2021 04:00:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 60525687B0; Fri, 23 Jul 2021 06:00:45 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ktcd8NIK"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2247760274 for ; Fri, 23 Jul 2021 06:00:43 +0200 (CEST) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 965A6255 for ; Fri, 23 Jul 2021 06:00:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1627012842; bh=p+qnx+b+MhPlwCmqs1imM+b/SKD4/R2+Eh1A92rZkJQ=; h=From:To:Subject:Date:From; b=ktcd8NIK4K684+tzpxfoiMRYMBJevlqki16oiJcGpKinuQ67jGDE+etPQlTx03lfD Bejys0HelfLosVjnxfQpfO8AT3ZTBQdlnv37qPqcnYg7d0BVZJSkgqPI4HaGlfMihG cMi9MihyT6okSOp/g6Bv0pdymqpCLRl3mNDhdxrs= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 23 Jul 2021 07:00:19 +0300 Message-Id: <20210723040036.32346-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 00/17] libcamera: Replace CameraData with Camera::Private X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 series starts with two documentation enhancements for the Extensible class (01/17 and 02/17). Patch 03/17 then allows decoupling the construction of the public and private classes when using the Extensible infrastructure. This is required to allow pipeline handlers to instantiate their custom class derived from Camera::Extensible and pass it to the Camera constructor manually. Patches 04/17 and 05/17 are additional documentation improvements. Patches 06/17 to 08/17 make the Camera::Private class visible and usable to pipeline handlers, and patch 09/17 then moves the members of the existing CameraData class to Camera::Private. Patches 10/17 to 15/17 update pipeline handlers individual to migrate to Camera::Private. Patch 16/17 then cleans up by dropping the CameraData class, and patch 17/17 updates the pipeline handler guide accordingly. Patch 03/17 is probably the biggest open question in this RFC. The 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. The rest of the infrastructure, however, holds up pretty well I believe, especially access control to the private classes that can use the usual C++ public/protected/private access specifiers to restrict some members to the corresponding public class only. 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 :-)), I'll test the other pipeline handlers. Laurent Pinchart (17): libcamera: base: class: Document Extensible::_d() functions libcamera: base: class: Link LIBCAMERA_O_PTR to Extensible documentation libcamera: base: class: Don't pass Extensible pointer to Private constructor libcamera: frame_buffer: Document the FrameBuffer::Private class Documentation: Doxygen: Don't exclude Private classes libcamera: camera: Move Camera::Private to header file libcamera: camera: Make Camera::Private members private 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 Documentation/Doxyfile.in | 2 +- Documentation/guides/pipeline-handler.rst | 67 +++++----- include/libcamera/base/class.h | 5 +- include/libcamera/camera.h | 5 +- include/libcamera/internal/camera.h | 70 ++++++++++ include/libcamera/internal/framebuffer.h | 2 +- include/libcamera/internal/meson.build | 1 + include/libcamera/internal/pipeline_handler.h | 29 +---- src/android/camera_hal_config.cpp | 7 +- src/android/mm/cros_camera_buffer.cpp | 4 +- src/android/mm/generic_camera_buffer.cpp | 3 +- src/libcamera/base/class.cpp | 30 ++++- src/libcamera/camera.cpp | 120 ++++++++++------- src/libcamera/camera_manager.cpp | 8 +- src/libcamera/framebuffer.cpp | 16 ++- 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 | 121 ++---------------- 22 files changed, 321 insertions(+), 320 deletions(-) create mode 100644 include/libcamera/internal/camera.h