diff options
author | Laurent Pinchart <laurent.pinchart@ideasonboard.com> | 2020-03-20 00:53:51 +0200 |
---|---|---|
committer | Laurent Pinchart <laurent.pinchart@ideasonboard.com> | 2020-03-20 16:47:45 +0200 |
commit | 0028536d70c79ebabf11f77cc2df965181509297 (patch) | |
tree | e41a356c2d3cd0b5efe41407949bb7dac1f94044 | |
parent | 916df9e38d3af191853838ab4f2b0049ee207218 (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.h | 4 | ||||
-rw-r--r-- | src/libcamera/controls.cpp | 1 | ||||
-rw-r--r-- | src/libcamera/device_enumerator_udev.cpp | 3 |
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; } |