summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLaurent Pinchart <laurent.pinchart@ideasonboard.com>2020-03-20 00:53:51 +0200
committerLaurent Pinchart <laurent.pinchart@ideasonboard.com>2020-03-20 16:47:45 +0200
commit0028536d70c79ebabf11f77cc2df965181509297 (patch)
treee41a356c2d3cd0b5efe41407949bb7dac1f94044
parent916df9e38d3af191853838ab4f2b0049ee207218 (diff)
libcamera: controls: Don't over-optimize ControlValue layout
The ControlValue class size should be minimized to save space, as it can be instantiated in large numbers. The current implementation does so by specifying several members as bitfields, but does so too aggressively, resulting in fields being packed in an inefficient to access way on some platforms. For instance, on 32-bit x86, the numElements_ field is offset by 7 bits in a 32-bit word. This additionally causes a static assert that checks the size of the class to fail. Relax the constraints on the isArray_ and numElements_ fields to avoid inefficient access, and to ensure that the class size is identical across all platforms. This will need to be revisited anyway when stabilizing the ABI, so add a \todo comment as a reminder. Fixes: 1fa4b43402a0 ("libcamera: controls: Support array controls in ControlValue") Reported-by: Jan Engelhardt <jengelh@inai.de> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
-rw-r--r--include/libcamera/controls.h4
-rw-r--r--src/libcamera/controls.cpp1
-rw-r--r--src/libcamera/device_enumerator_udev.cpp3
3 files changed, 5 insertions, 3 deletions
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index 56ee8280..2a665712 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -176,8 +176,8 @@ public:
private:
ControlType type_ : 8;
- bool isArray_ : 1;
- std::size_t numElements_ : 16;
+ bool isArray_;
+ std::size_t numElements_ : 32;
union {
uint64_t value_;
void *storage_;
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index fefd292b..25684474 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -86,6 +86,7 @@ static constexpr size_t ControlValueSize[] = {
* \brief Abstract type representing the value of a control
*/
+/** \todo Revisit the ControlValue layout when stabilizing the ABI */
static_assert(sizeof(ControlValue) == 16, "Invalid size of ControlValue class");
/**
diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
index 87638c59..f5a482e3 100644
--- a/src/libcamera/device_enumerator_udev.cpp
+++ b/src/libcamera/device_enumerator_udev.cpp
@@ -90,7 +90,8 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
return ret;
}
- addDevice(media);
+ if (!ret)
+ addDevice(media);
return 0;
}