From patchwork Wed Sep 18 10:31:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1978 Return-Path: Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6681760BB0 for ; Wed, 18 Sep 2019 12:30:00 +0200 (CEST) X-Originating-IP: 2.224.242.101 Received: from uno.lan (2-224-242-101.ip172.fastwebnet.it [2.224.242.101]) (Authenticated sender: jacopo@jmondi.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 142681BF205 for ; Wed, 18 Sep 2019 10:29:59 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Wed, 18 Sep 2019 12:31:28 +0200 Message-Id: <20190918103133.14296-1-jacopo@jmondi.org> X-Mailer: git-send-email 2.23.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 0/5] libcamera: Control framework backend rework X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Sep 2019 10:30:00 -0000 Hello, this series is the first half of the currently in progress serialization support work. The basic idea is that to be able to serialize Controls and V4L2Controls using the same API, their back-end storage should be unified to use the same polymorphic data storage class. This was the initial plan to be honest, but it didn't match the urgent need to get controls support in at the time, but it's now required for serialization, so it's now a good time to fix this long standing issue. The series addresses the issue taking the following steps. 1) Break out the DataValue class from the ControlValue class and associate it with a DataInfo class, that is meant to support constant validation information associated with a value. The mapping will be V4L2Control = Control = DataValue V4L2ControlInfo = ControlInfo = DataInfo With V4L2ControlList being a list of V4L2Controls and ControlList being a map of Controls associated to a map of ControlInfo both indexed by ControlId [ later considerations: when writing below about the API of ControlList and V4L2ControlList I realized they're effectively maps of ids to values, and naming them "Map" would make the syntax "ControlMap[id] = value" more natural. What do you think? ] All of this in patches 1, 2, 3 2) Serialization implies de-serialization, and to construct a ControlList we need a camera. Break this dependency in patch 4/5 to be able to reconstruct a ControlList from serialized data later 3) Bikeshedders heaven: rewording of comments and rename of data types in Control framework. As explained in the commit message of 5/5, the rewording tries to enforce the following concepts: control metadata, control info, and control values. On top of this I have patches and tests for Serialization, but not yet for de-serialization. I hope not, but it might be that implementing it require changes in this series, so be prepared. All of this for the back-end storage. The user APIs: there is work there too, as both V4L2Controls and Controls might need re-thinking of their user API. The fundamental difference between the two (and it took me a while to realize the implication of this in the API definition) it's where validation information are kept. ControlList is created with a reference to a ControlInfoMap used for validation and produced by pipeline handlers at camera registration time. This allows to validate a control at the moment it is added to the list, as doing so when queuing requests is indeed too late for a safe recovery. V4L2Control list only contains values. The validation informations are kept in the device the list is applied on. I think this still makes sense, as V4L2 controls are meant to be validate at the time they are immediately applied to the device by the driver exposing them, and they will not be exposed to applications. Anyway, creating a V4L2ControlList from a device might make serialization easier because both values and information would then be kept in the same class, as it happens now for Controls. The other difference in the API is a matter of tastes mostly. Controls use the overloaded operator[] to set/get controls, V4L2Controls have explicit get/set. Having implemented them, it's clear that my preference goes toward explicit get/set, as operator overloading makes me always a bit unconfortable and I prefer to type in 3 letters more for set/get but have an explicit indication of what's happening. Anyway, the use of [] made in ControlList is pretty natural and does what you would expect it to do (maybe if it was a ControlMap it would feel more natural to do controls[x] = y ? Now that I wrote it here, I'll suggest it in the above comment too) and seems to be also in line with https://google.github.io/styleguide/cppguide.html "Operator Overloading" section. So I'll go with popular preference, but in any case I think the two should be unified. As said, serialization will come on top of this work, so please comment to avoid me too many re-bases :) Thanks j Jacopo Mondi (5): libcamera: Create DataValue and DataInfo libcamera: controls: Use DataType and DataValue libcamera: v4l2_controls: Use DataValue and DataInfo libcamera: controls: De-couple Controls from Camera libcamera: controls: Control framework refresh include/libcamera/controls.h | 94 +---- include/libcamera/data_value.h | 84 ++++ include/libcamera/meson.build | 1 + src/libcamera/controls.cpp | 398 +++++------------- src/libcamera/data_value.cpp | 317 ++++++++++++++ src/libcamera/gen-controls.awk | 4 +- src/libcamera/include/v4l2_controls.h | 22 +- src/libcamera/include/v4l2_device.h | 1 - src/libcamera/meson.build | 7 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/request.cpp | 4 +- src/libcamera/v4l2_controls.cpp | 49 +-- src/libcamera/v4l2_device.cpp | 25 +- test/controls/control_info.cpp | 4 +- test/controls/control_list.cpp | 6 +- test/controls/meson.build | 1 - .../data_value.cpp} | 24 +- test/data_value/meson.build | 12 + test/meson.build | 1 + 20 files changed, 593 insertions(+), 465 deletions(-) create mode 100644 include/libcamera/data_value.h create mode 100644 src/libcamera/data_value.cpp rename test/{controls/control_value.cpp => data_value/data_value.cpp} (68%) create mode 100644 test/data_value/meson.build --- 2.23.0