From patchwork Sat Oct 12 18:43:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2161 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B5A0A61563 for ; Sat, 12 Oct 2019 20:44:14 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 168EB33A for ; Sat, 12 Oct 2019 20:44:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1570905854; bh=1amVpazAjhJYjxR/S0JnT7Y9Z4altdL2wXXvNzdesP0=; h=From:To:Subject:Date:From; b=KUwvp1ChBNvIjQUflj9lPYgQUz5OhtVlkbDNroFiwOmUMZxJIl1lPwNN9b3Hjk+CX SzLkjIzM+qnxkY6EfyJei344xM//FymS02JyF3vqTLifVPgt6YC5o5Shkr6Ruytcyq Kw8cMtGdkriBIxpVTY1du5rLEpZbiXQO8haC1vv4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 12 Oct 2019 21:43:53 +0300 Message-Id: <20191012184407.31684-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 00/14] Use ControlList for both libcamera and V4L2 controls 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: , X-List-Received-Date: Sat, 12 Oct 2019 18:44:14 -0000 Hello, This patch series generalises usage of ControlList for all controls, both libcamera and V4L2. The rationale is that using a single container to store controls allows better sharing of code, especially when we will implement serialisation and deserialisation. Additionally, a single container type simplifies the IPA API. Compared to v1, the series has been rebased on top of the IPA API, which unveiled issues I hadn't foreseen. Large parts of the code have thus been rewritten. Patches 01/14 and 02/14 are small cleanups and fixes, and patch 03/14 restores the global list of libcamera controls that used to be auto-generated. While this wasn't strictly required in v1, this version now depends on it. The next three patches are also small enhancements to simplify usage of ControlValue, ControlList and ControlId (04/14 to 06/14). Patch 07/14 is the first large change, and extends the ControlLis API to support accessing controls by numerical ID. This was previously implemented in the V4L2ControlList class and got moved to ControlList as this API will likely be used for deserialisation. As a result, the ControlList now needs to be constructed with a reference to a ControlIdMap to support mapping numerical IDs to ControlId instances. Patch 08/14 adds a new test for V4L2 controls, based on the vivid driver. The test uses the existing V4L2ControlList API, and gets migrated to ControlList further down in the series, to ensure that the API switch doesn't create regressions. Patch 09/14 is another small enhancement, after which patch 10/14 starts the migration of V4L2 controls by using ControlId in V4L2ControlInfo. To do so, a new V4L2ControlId class is introduced that simply wraps ControlId with a convenience constructor. Patch 11/14 then removes the V4L2ControlInfo::type field that duplicates ControlId::type, and patch 12/14 turns V4L2ControlInfoMap into a real class in preparation for extending it. Patch 13/14 is the bulk of the work, and by far the largest in the series (but with a nice net removal of 168 lines of code). It moves V4L2ControlList to become an optional thin convenience wrapper around ControlList, and ControlList can be used directly for V4L2 controls if desired. Finally patch 14/14 merges the control and v4l2controls fields in IPAOperationData, simplifying the IPA API. More code sharing between V4L2 and libcamera controls may still be possible, especially around V4L2ControlInfo and V4L2ControlInfoMap. I believe it can be built on top of this series. The series is based on top of the master branch, patches have been compiled and tested individually with gcc 5.4.0, and the whole series has been compiled with gcc 5 to 9 and clang 6 to 8. Niklas, would you be able to run your manual rkisp1 IPA tests ? Laurent Pinchart (14): libcamera: pipeline: rkisp1: Avoid copy assignment of V4L2 control map libcamera: control_ids: Fix documentation for controls namespace libcamera: control_ids: Generate map of all supported controls libcamera: controls: Add comparison operators for ControlValue libcamera: controls: Default ControlList validator argument to nullptr libcamera: controls: Store control name in ControlId libcamera: controls: Support accessing controls by numerical ID test: v4l2_videodevice: Add V4L2 control test libcamera: v4l2_device: Avoid copy of V4L2ControlInfo libcamera: v4l2_controls: Add V4L2ControlId libcamera: v4l2_controls: Remove V4L2ControlInfo type field libcamera: v4l2_controls: Turn V4L2ControlInfoMap into a class libcamera: v4l2_device: Replace V4L2ControlList with ControlList libcamera: ipa: Merge controls and v4l2controls in IPAOperationData include/ipa/ipa_interface.h | 1 - include/libcamera/control_ids.h.in | 2 + include/libcamera/controls.h | 30 ++- src/ipa/rkisp1/rkisp1.cpp | 22 +- src/libcamera/camera_sensor.cpp | 4 +- src/libcamera/control_ids.cpp.in | 20 +- src/libcamera/controls.cpp | 181 ++++++++++++--- src/libcamera/gen-controls.py | 21 +- src/libcamera/include/camera_sensor.h | 7 +- src/libcamera/include/v4l2_controls.h | 74 +++--- src/libcamera/include/v4l2_device.h | 6 +- src/libcamera/ipa_interface.cpp | 8 - src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +- src/libcamera/pipeline/uvcvideo.cpp | 24 +- src/libcamera/pipeline/vimc.cpp | 25 +-- src/libcamera/request.cpp | 5 +- src/libcamera/v4l2_controls.cpp | 273 ++++++++--------------- src/libcamera/v4l2_device.cpp | 71 +++--- test/controls/control_list.cpp | 2 +- test/v4l2_videodevice/controls.cpp | 98 ++++++++ test/v4l2_videodevice/meson.build | 1 + 22 files changed, 526 insertions(+), 366 deletions(-) create mode 100644 test/v4l2_videodevice/controls.cpp Reviewed-by: Jacopo Mondi Tested-by: Niklas Söderlund