From patchwork Mon Sep 4 21:59:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 18977 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 DFFD9C0F1B for ; Mon, 4 Sep 2023 21:59:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 411DB628E9; Mon, 4 Sep 2023 23:59:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1693864799; bh=6u0D5y8KQEvOVW3xYodhKaLi/zl89kaAHvrZVY01o2U=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=BpEp7K1Hk+Za7SJoW1GUCO6Po/gbscZspQMC/NyaDTIibmvYhLSEHyX3VTe0Y8Yfn 4ii5YhhnnSBGfDs7u8cyqSqdQY6rg3SJZZI8gAwq7p+mol78VMpzmGzKBbvnttDmso hw/C1+GZsAEZ2uY+nGXmITLpoxBG2DkRscrnpaTCMgB8GK05Um+VlEJUb5qXaSuqlJ I4nQHS9LIPAwhVPL9wPlGsKNQmiXR3vJFA7i40qkwuDEGooOqz/MyyoxofORGUCxNA I+mu1AzbCfKTP9FwesN+JzUKdEUN2sMEkFl9m5EW5swFyfduVv6JDt51YWWDPGZ3XR 0Ua2QYWtJHd0g== 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 C5C10628D7 for ; Mon, 4 Sep 2023 23:59:57 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="N9UW5IWV"; dkim-atps=neutral Received: from Monstersaurus.local (aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net [82.37.23.78]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6B2D5182D; Mon, 4 Sep 2023 23:58:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1693864711; bh=6u0D5y8KQEvOVW3xYodhKaLi/zl89kaAHvrZVY01o2U=; h=From:To:Cc:Subject:Date:From; b=N9UW5IWVXNmZ4kXbRe+3PAMns84802Tq+TMiUPYx7N3oOZHl/BSj0FGSOIMg8Z+BA P0DMX/LnTzDtLX/4xmYv61EAcIoJ4Moyw1azyu2/Nhc8XMgOdqb9PhgmUc2G31zALF OWwKRNBJQh871iX3hy8+rBaNd8xsY9WK2r8FWhxg= To: libcamera devel Date: Mon, 4 Sep 2023 22:59:54 +0100 Message-Id: <20230904215954.3417076-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH] libcamera: Privatise control enums 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-Patchwork-Original-From: Kieran Bingham via libcamera-devel From: Kieran Bingham Reply-To: Kieran Bingham Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The Control framework generates an enumeration for each control and property entry which is used to support the ControlIdMap for each. This enum listing provides hardcoded integer references for each control and is a source of potential ABI breakage when introducing new controls. The enum values are expected to only be used internally, so reduce public ABI exposure when modifying controls by moving the enum to new internal header definitions. Signed-off-by: Kieran Bingham --- While trying to work towards a new 'patch' release, I hit the following abi-compatbility issues: Binary compatibility: 99.4% Source compatibility: 99.7% Total binary compatibility problems: 2, warnings: 0 Total source compatibility problems: 2, warnings: 0 Report: compat_reports/libcamera/v0.1.0_to_v0.1.0-30-g59529ca6cb46/compat_report.html This stems from the removal of the following symbols: Removed Symbols 2 control_ids.h, libcamera.so.0.1.0 namespace libcamera::controls::draft SceneFlicker [data] SceneFlickerValues [data] And also the introduction of four new controls: AeFlickerDetected [data] AeFlickerMode [data] AeFlickerModeValues [data] AeFlickerPeriod [data] All of these changes affect the enum values generated to produce the ControlIdMap. This proposal aims to remove this from the Public ABI/API to prevent potential issues on every modification to the control list. Of course - removing a draft control or renaming the controls is still going to cause linkage problems with any application which may use those symbols. In this specific case, I suspect we would be lucky as SceneFlicker and SceneFlickerValues are likely to be unused as they were otherwise unimplemented. include/libcamera/control_ids.h.in | 4 ---- include/libcamera/internal/control_ids.h.in | 24 ++++++++++++++++++++ include/libcamera/internal/meson.build | 20 ++++++++++++++++ include/libcamera/internal/property_ids.h.in | 24 ++++++++++++++++++++ include/libcamera/property_ids.h.in | 4 ---- src/ipa/rpi/common/ipa_base.cpp | 3 +++ src/libcamera/control_ids.cpp.in | 4 +++- src/libcamera/meson.build | 1 + src/libcamera/property_ids.cpp.in | 3 ++- 9 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 include/libcamera/internal/control_ids.h.in create mode 100644 include/libcamera/internal/property_ids.h.in diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in index 0718a8886f6c..a0c66192fdc9 100644 --- a/include/libcamera/control_ids.h.in +++ b/include/libcamera/control_ids.h.in @@ -18,10 +18,6 @@ namespace libcamera { namespace controls { -enum { -${ids} -}; - ${controls} extern const ControlIdMap controls; diff --git a/include/libcamera/internal/control_ids.h.in b/include/libcamera/internal/control_ids.h.in new file mode 100644 index 000000000000..c9bfaf119578 --- /dev/null +++ b/include/libcamera/internal/control_ids.h.in @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_ids.h - Internal Control ID list + * + * This file is auto-generated. Do not edit. + */ + +#pragma once + +#include + +namespace libcamera { + +namespace controls { + +enum { +${ids} +}; + +} /* namespace controls */ + +} /* namespace libcamera */ diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 7f1f344014c4..dc7fa83ed978 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -47,4 +47,24 @@ libcamera_internal_headers = files([ 'yaml_parser.h', ]) +# Internal control and property ID mappings +internal_control_source_files = [ + 'control_ids', + 'property_ids', +] + +internal_control_headers = [] + +foreach header : internal_control_source_files + input_files = files('../../../src/libcamera/' + header + '.yaml', + header + '.h.in') + internal_control_headers += custom_target(header + '_h', + input : input_files, + output : header + '.h', + command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], + install : false) +endforeach + +libcamera_internal_headers += internal_control_headers + subdir('converter') diff --git a/include/libcamera/internal/property_ids.h.in b/include/libcamera/internal/property_ids.h.in new file mode 100644 index 000000000000..15f4950953b4 --- /dev/null +++ b/include/libcamera/internal/property_ids.h.in @@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * property_ids.h - Internal Property ID list + * + * This file is auto-generated. Do not edit. + */ + +#pragma once + +#include + +namespace libcamera { + +namespace properties { + +enum { +${ids} +}; + +} /* namespace properties */ + +} /* namespace libcamera */ diff --git a/include/libcamera/property_ids.h.in b/include/libcamera/property_ids.h.in index ff0194083af0..0fbdcc0c8504 100644 --- a/include/libcamera/property_ids.h.in +++ b/include/libcamera/property_ids.h.in @@ -17,10 +17,6 @@ namespace libcamera { namespace properties { -enum { -${ids} -}; - ${controls} namespace draft { diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index a47ae3a9e5cb..8e083396cb01 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -11,9 +11,12 @@ #include #include + #include #include +#include "libcamera/internal/control_ids.h" + #include "controller/af_algorithm.h" #include "controller/af_status.h" #include "controller/agc_algorithm.h" diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in index 5fb1c2c30558..20ef147ab826 100644 --- a/src/libcamera/control_ids.cpp.in +++ b/src/libcamera/control_ids.cpp.in @@ -10,8 +10,10 @@ #include #include +#include "libcamera/internal/control_ids.h" + /** - * \file control_ids.h + * \file libcamera/control_ids.h * \brief Camera control identifiers */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index b24f82965764..e7f5edb2f39b 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -131,6 +131,7 @@ foreach source : control_source_files control_sources += custom_target(source + '_cpp', input : input_files, output : source + '.cpp', + depends : internal_control_headers, command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@']) endforeach diff --git a/src/libcamera/property_ids.cpp.in b/src/libcamera/property_ids.cpp.in index f917e3349766..c7dbf9838411 100644 --- a/src/libcamera/property_ids.cpp.in +++ b/src/libcamera/property_ids.cpp.in @@ -7,10 +7,11 @@ * This file is auto-generated. Do not edit. */ +#include "libcamera/internal/property_ids.h" #include /** - * \file property_ids.h + * \file libcamera/property_ids.h * \brief Camera property identifiers */