From patchwork Fri Mar 20 00:55:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3205 Return-Path: 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 9BA4860418 for ; Fri, 20 Mar 2020 01:56:02 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 01E64E1D; Fri, 20 Mar 2020 01:56:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584665762; bh=wivTZG+dnHFrO0BmwO5Ww8Mz+ud4I3wgLGw7071qp04=; h=From:To:Cc:Subject:Date:From; b=g3zt44Eqdo8QJGXLYUvKkX7yqND41pyjlZEWGh+ffVMe2KHY7Ol/gO8FQunwJaKQz Ph91koh0Vlb0vS7b77e+Iv71G4TlcWEPLBt20UDtH7n9P2wHth1FVC76rpSTytrolq /tAUdd0VDHNmJ57PLc0mjdxICzU72i2bV7TnfkE8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: Jan Engelhardt Date: Fri, 20 Mar 2020 02:55:50 +0200 Message-Id: <20200320005551.10226-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] libcamera: controls: Move ControlValue size check to controls.cpp 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-List-Received-Date: Fri, 20 Mar 2020 00:56:02 -0000 The size of the ControlValue class is checked by a static_assert() to avoid accidental ABI breakages. There's no need to perform the check every time controls.h is included, move it to controls.cpp. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- include/libcamera/controls.h | 2 -- src/libcamera/controls.cpp | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 0e111ab72bce..2fca975f7512 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -170,8 +170,6 @@ private: std::size_t numElements, std::size_t elementSize); }; -static_assert(sizeof(ControlValue) == 16, "Invalid size of ControlValue class"); - class ControlId { public: diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 4326174adf86..4615e71d7e3c 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -83,6 +83,8 @@ static constexpr size_t ControlValueSize[] = { * \brief Abstract type representing the value of a control */ +static_assert(sizeof(ControlValue) == 16, "Invalid size of ControlValue class"); + /** * \brief Construct an empty ControlValue. */ From patchwork Fri Mar 20 00:55:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3206 Return-Path: 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 E15C660418 for ; Fri, 20 Mar 2020 01:56:02 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 65BAEE41; Fri, 20 Mar 2020 01:56:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1584665762; bh=zARkIswhVGcb2TDk5xkBmDrFab00ZSTtIfOtEEAHstQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VHv3hkgF6SwaXDtBfN0b4FnfKN/aU4Cy9ryJ27VAo14kt1bEk1F8lHAT9cvvKScZ9 /ntW8ZZ9AAUWYW4xu7zTXzx6ofbXN638LnQiYF/7/6h+OtAeTbbJ2SfUJxQJgXQnTB 2BCNXG5o18vIShafcc8+5UnNrWQPzVivSpv8ZXWo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: Jan Engelhardt Date: Fri, 20 Mar 2020 02:55:51 +0200 Message-Id: <20200320005551.10226-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200320005551.10226-1-laurent.pinchart@ideasonboard.com> References: <20200320005551.10226-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/2] libcamera: controls: Don't over-optimize ControlValue layout 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-List-Received-Date: Fri, 20 Mar 2020 00:56:03 -0000 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 anyway need to be revisited 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 Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- include/libcamera/controls.h | 4 ++-- src/libcamera/controls.cpp | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 2fca975f7512..40a29189ba01 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -158,8 +158,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 4615e71d7e3c..3c910d38f05d 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -83,6 +83,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"); /**