From patchwork Thu Aug 5 17:58:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 13218 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 73ECFC3237 for ; Thu, 5 Aug 2021 17:59:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6B22A6884E; Thu, 5 Aug 2021 19:59:06 +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="qa/4gi71"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 26EE86026D for ; Thu, 5 Aug 2021 19:59:05 +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 9AA9CE04 for ; Thu, 5 Aug 2021 19:59:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1628186344; bh=kDfJ6iBlGaJnYsHsFY34Km6FLU6d9GLOfBcEvI3cMic=; h=From:To:Subject:Date:From; b=qa/4gi71w9ohz0xw6S0Tf0YJjNpLfKYQCDlfG7npnwi4eU2Z9umvRbNqQlGa9Q97X ilrDBXfAPBZDA+CQgpnW+AD2ujhOgrjzYSB2jmCF47GcSLp9iNXIrGSa9wTsdpXKvS 8H4CN3pRM+gV1JXICCr6tYMlR2Y6OizhJrbckP7s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 5 Aug 2021 20:58:37 +0300 Message-Id: <20210805175848.24188-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 00/11] 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 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(-)