{"id":18977,"url":"https://patchwork.libcamera.org/api/patches/18977/?format=json","web_url":"https://patchwork.libcamera.org/patch/18977/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20230904215954.3417076-1-kieran.bingham@ideasonboard.com>","date":"2023-09-04T21:59:54","name":"[libcamera-devel,RFC] libcamera: Privatise control enums","commit_ref":null,"pull_url":null,"state":"rejected","archived":false,"hash":"b154f5a0711bf6a0671ea39e42fed64437bcb804","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/?format=json","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/18977/mbox/","series":[{"id":4012,"url":"https://patchwork.libcamera.org/api/series/4012/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4012","date":"2023-09-04T21:59:54","name":"[libcamera-devel,RFC] libcamera: Privatise control enums","version":1,"mbox":"https://patchwork.libcamera.org/series/4012/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/18977/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/18977/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DFFD9C0F1B\n\tfor ;\n\tMon, 4 Sep 2023 21:59:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 411DB628E9;\n\tMon, 4 Sep 2023 23:59:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C5C10628D7\n\tfor ;\n\tMon, 4 Sep 2023 23:59:57 +0200 (CEST)","from Monstersaurus.local\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6B2D5182D;\n\tMon, 4 Sep 2023 23:58:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1693864799;\n\tbh=6u0D5y8KQEvOVW3xYodhKaLi/zl89kaAHvrZVY01o2U=;\n\th=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post:\n\tList-Help:List-Subscribe:From:Reply-To:From;\n\tb=BpEp7K1Hk+Za7SJoW1GUCO6Po/gbscZspQMC/NyaDTIibmvYhLSEHyX3VTe0Y8Yfn\n\t4ii5YhhnnSBGfDs7u8cyqSqdQY6rg3SJZZI8gAwq7p+mol78VMpzmGzKBbvnttDmso\n\thw/C1+GZsAEZ2uY+nGXmITLpoxBG2DkRscrnpaTCMgB8GK05Um+VlEJUb5qXaSuqlJ\n\tI4nQHS9LIPAwhVPL9wPlGsKNQmiXR3vJFA7i40qkwuDEGooOqz/MyyoxofORGUCxNA\n\tI+mu1AzbCfKTP9FwesN+JzUKdEUN2sMEkFl9m5EW5swFyfduVv6JDt51YWWDPGZ3XR\n\t0Ua2QYWtJHd0g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1693864711;\n\tbh=6u0D5y8KQEvOVW3xYodhKaLi/zl89kaAHvrZVY01o2U=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=N9UW5IWVXNmZ4kXbRe+3PAMns84802Tq+TMiUPYx7N3oOZHl/BSj0FGSOIMg8Z+BA\n\tP0DMX/LnTzDtLX/4xmYv61EAcIoJ4Moyw1azyu2/Nhc8XMgOdqb9PhgmUc2G31zALF\n\tOWwKRNBJQh871iX3hy8+rBaNd8xsY9WK2r8FWhxg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"N9UW5IWV\"; dkim-atps=neutral","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","Content-Transfer-Encoding":"8bit","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":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The Control framework generates an enumeration for each control and\nproperty entry which is used to support the ControlIdMap for each.\n\nThis enum listing provides hardcoded integer references for each control\nand is a source of potential ABI breakage when introducing new controls.\n\nThe enum values are expected to only be used internally, so reduce\npublic ABI exposure when modifying controls by moving the enum to new\ninternal header definitions.\n\nSigned-off-by: Kieran Bingham \n---\nWhile trying to work towards a new 'patch' release, I hit the following\nabi-compatbility issues:\n\n Binary compatibility: 99.4%\n Source compatibility: 99.7%\n Total binary compatibility problems: 2, warnings: 0\n Total source compatibility problems: 2, warnings: 0\n Report: compat_reports/libcamera/v0.1.0_to_v0.1.0-30-g59529ca6cb46/compat_report.html\n\nThis stems from the removal of the following symbols:\n\nRemoved Symbols 2 \n control_ids.h, libcamera.so.0.1.0\n namespace libcamera::controls::draft\n SceneFlicker [data]\n SceneFlickerValues [data]\n\nAnd also the introduction of four new controls:\n AeFlickerDetected [data]\n AeFlickerMode [data]\n AeFlickerModeValues [data]\n AeFlickerPeriod [data]\n\nAll of these changes affect the enum values generated to produce the\nControlIdMap.\n\nThis proposal aims to remove this from the Public ABI/API to prevent\npotential issues on every modification to the control list.\n\nOf course - removing a draft control or renaming the controls is still\ngoing to cause linkage problems with any application which may use those\nsymbols.\n\nIn this specific case, I suspect we would be lucky as SceneFlicker and\nSceneFlickerValues are likely to be unused as they were otherwise\nunimplemented.\n\n include/libcamera/control_ids.h.in | 4 ----\n include/libcamera/internal/control_ids.h.in | 24 ++++++++++++++++++++\n include/libcamera/internal/meson.build | 20 ++++++++++++++++\n include/libcamera/internal/property_ids.h.in | 24 ++++++++++++++++++++\n include/libcamera/property_ids.h.in | 4 ----\n src/ipa/rpi/common/ipa_base.cpp | 3 +++\n src/libcamera/control_ids.cpp.in | 4 +++-\n src/libcamera/meson.build | 1 +\n src/libcamera/property_ids.cpp.in | 3 ++-\n 9 files changed, 77 insertions(+), 10 deletions(-)\n create mode 100644 include/libcamera/internal/control_ids.h.in\n create mode 100644 include/libcamera/internal/property_ids.h.in","diff":"diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in\nindex 0718a8886f6c..a0c66192fdc9 100644\n--- a/include/libcamera/control_ids.h.in\n+++ b/include/libcamera/control_ids.h.in\n@@ -18,10 +18,6 @@ namespace libcamera {\n \n namespace controls {\n \n-enum {\n-${ids}\n-};\n-\n ${controls}\n \n extern const ControlIdMap controls;\ndiff --git a/include/libcamera/internal/control_ids.h.in b/include/libcamera/internal/control_ids.h.in\nnew file mode 100644\nindex 000000000000..c9bfaf119578\n--- /dev/null\n+++ b/include/libcamera/internal/control_ids.h.in\n@@ -0,0 +1,24 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2019, Google Inc.\n+ *\n+ * control_ids.h - Internal Control ID list\n+ *\n+ * This file is auto-generated. Do not edit.\n+ */\n+\n+#pragma once\n+\n+#include \n+\n+namespace libcamera {\n+\n+namespace controls {\n+\n+enum {\n+${ids}\n+};\n+\n+} /* namespace controls */\n+\n+} /* namespace libcamera */\ndiff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\nindex 7f1f344014c4..dc7fa83ed978 100644\n--- a/include/libcamera/internal/meson.build\n+++ b/include/libcamera/internal/meson.build\n@@ -47,4 +47,24 @@ libcamera_internal_headers = files([\n 'yaml_parser.h',\n ])\n \n+# Internal control and property ID mappings\n+internal_control_source_files = [\n+ 'control_ids',\n+ 'property_ids',\n+]\n+\n+internal_control_headers = []\n+\n+foreach header : internal_control_source_files\n+ input_files = files('../../../src/libcamera/' + header + '.yaml',\n+ header + '.h.in')\n+ internal_control_headers += custom_target(header + '_h',\n+ input : input_files,\n+ output : header + '.h',\n+ command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'],\n+ install : false)\n+endforeach\n+\n+libcamera_internal_headers += internal_control_headers\n+\n subdir('converter')\ndiff --git a/include/libcamera/internal/property_ids.h.in b/include/libcamera/internal/property_ids.h.in\nnew file mode 100644\nindex 000000000000..15f4950953b4\n--- /dev/null\n+++ b/include/libcamera/internal/property_ids.h.in\n@@ -0,0 +1,24 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2019, Google Inc.\n+ *\n+ * property_ids.h - Internal Property ID list\n+ *\n+ * This file is auto-generated. Do not edit.\n+ */\n+\n+#pragma once\n+\n+#include \n+\n+namespace libcamera {\n+\n+namespace properties {\n+\n+enum {\n+${ids}\n+};\n+\n+} /* namespace properties */\n+\n+} /* namespace libcamera */\ndiff --git a/include/libcamera/property_ids.h.in b/include/libcamera/property_ids.h.in\nindex ff0194083af0..0fbdcc0c8504 100644\n--- a/include/libcamera/property_ids.h.in\n+++ b/include/libcamera/property_ids.h.in\n@@ -17,10 +17,6 @@ namespace libcamera {\n \n namespace properties {\n \n-enum {\n-${ids}\n-};\n-\n ${controls}\n \n namespace draft {\ndiff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\nindex a47ae3a9e5cb..8e083396cb01 100644\n--- a/src/ipa/rpi/common/ipa_base.cpp\n+++ b/src/ipa/rpi/common/ipa_base.cpp\n@@ -11,9 +11,12 @@\n \n #include \n #include \n+\n #include \n #include \n \n+#include \"libcamera/internal/control_ids.h\"\n+\n #include \"controller/af_algorithm.h\"\n #include \"controller/af_status.h\"\n #include \"controller/agc_algorithm.h\"\ndiff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\nindex 5fb1c2c30558..20ef147ab826 100644\n--- a/src/libcamera/control_ids.cpp.in\n+++ b/src/libcamera/control_ids.cpp.in\n@@ -10,8 +10,10 @@\n #include \n #include \n \n+#include \"libcamera/internal/control_ids.h\"\n+\n /**\n- * \\file control_ids.h\n+ * \\file libcamera/control_ids.h\n * \\brief Camera control identifiers\n */\n \ndiff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\nindex b24f82965764..e7f5edb2f39b 100644\n--- a/src/libcamera/meson.build\n+++ b/src/libcamera/meson.build\n@@ -131,6 +131,7 @@ foreach source : control_source_files\n control_sources += custom_target(source + '_cpp',\n input : input_files,\n output : source + '.cpp',\n+ depends : internal_control_headers,\n command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'])\n endforeach\n \ndiff --git a/src/libcamera/property_ids.cpp.in b/src/libcamera/property_ids.cpp.in\nindex f917e3349766..c7dbf9838411 100644\n--- a/src/libcamera/property_ids.cpp.in\n+++ b/src/libcamera/property_ids.cpp.in\n@@ -7,10 +7,11 @@\n * This file is auto-generated. Do not edit.\n */\n \n+#include \"libcamera/internal/property_ids.h\"\n #include \n \n /**\n- * \\file property_ids.h\n+ * \\file libcamera/property_ids.h\n * \\brief Camera property identifiers\n */\n \n","prefixes":["libcamera-devel","RFC"]}