[libcamera-devel] [RFC PATCH] libcamera: Privatise control enums
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Sep 4 23:59:54 CEST 2023
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 <kieran.bingham at ideasonboard.com>
---
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 <libcamera/controls.h>
+
+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 <libcamera/controls.h>
+
+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 <libcamera/base/log.h>
#include <libcamera/base/span.h>
+
#include <libcamera/control_ids.h>
#include <libcamera/property_ids.h>
+#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 <libcamera/control_ids.h>
#include <libcamera/controls.h>
+#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 <libcamera/property_ids.h>
/**
- * \file property_ids.h
+ * \file libcamera/property_ids.h
* \brief Camera property identifiers
*/
--
2.34.1
More information about the libcamera-devel
mailing list