{"id":3206,"url":"https://patchwork.libcamera.org/api/1.1/patches/3206/?format=json","web_url":"https://patchwork.libcamera.org/patch/3206/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20200320005551.10226-2-laurent.pinchart@ideasonboard.com>","date":"2020-03-20T00:55:51","name":"[libcamera-devel,2/2] libcamera: controls: Don't over-optimize ControlValue layout","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"6f6de36e1b78cd1b9bcf7078acf106f7ded6a8c8","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/3206/mbox/","series":[{"id":747,"url":"https://patchwork.libcamera.org/api/1.1/series/747/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=747","date":"2020-03-20T00:55:50","name":"[libcamera-devel,1/2] libcamera: controls: Move ControlValue size check to controls.cpp","version":1,"mbox":"https://patchwork.libcamera.org/series/747/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/3206/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/3206/checks/","tags":{},"headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 E15C660418\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Mar 2020 01:56:02 +0100 (CET)","from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65BAEE41;\n\tFri, 20 Mar 2020 01:56:02 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584665762;\n\tbh=zARkIswhVGcb2TDk5xkBmDrFab00ZSTtIfOtEEAHstQ=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=VHv3hkgF6SwaXDtBfN0b4FnfKN/aU4Cy9ryJ27VAo14kt1bEk1F8lHAT9cvvKScZ9\n\t/ntW8ZZ9AAUWYW4xu7zTXzx6ofbXN638LnQiYF/7/6h+OtAeTbbJ2SfUJxQJgXQnTB\n\t2BCNXG5o18vIShafcc8+5UnNrWQPzVivSpv8ZXWo=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Cc":"Jan Engelhardt <jengelh@inai.de>","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","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 2/2] libcamera: controls: Don't\n\tover-optimize ControlValue layout","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 20 Mar 2020 00:56:03 -0000"},"content":"The ControlValue class size should be minimized to save space, as it can\nbe instantiated in large numbers. The current implementation does so by\nspecifying several members as bitfields, but does so too aggressively,\nresulting in fields being packed in an inefficient to access way on some\nplatforms. For instance, on 32-bit x86, the numElements_ field is offset\nby 7 bits in a 32-bit word. This additionally causes a static assert\nthat checks the size of the class to fail.\n\nRelax the constraints on the isArray_ and numElements_ fields to avoid\ninefficient access, and to ensure that the class size is identical\nacross all platforms. This will anyway need to be revisited when\nstabilizing the ABI, so add a \\todo comment as a reminder.\n\nFixes: 1fa4b43402a0 (\"libcamera: controls: Support array controls in ControlValue\")\nReported-by: Jan Engelhardt <jengelh@inai.de>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/controls.h | 4 ++--\n src/libcamera/controls.cpp   | 1 +\n 2 files changed, 3 insertions(+), 2 deletions(-)","diff":"diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 2fca975f7512..40a29189ba01 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -158,8 +158,8 @@ public:\n \n private:\n \tControlType type_ : 8;\n-\tbool isArray_ : 1;\n-\tstd::size_t numElements_ : 16;\n+\tbool isArray_;\n+\tstd::size_t numElements_ : 32;\n \tunion {\n \t\tuint64_t value_;\n \t\tvoid *storage_;\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 4615e71d7e3c..3c910d38f05d 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -83,6 +83,7 @@ static constexpr size_t ControlValueSize[] = {\n  * \\brief Abstract type representing the value of a control\n  */\n \n+/** \\todo Revisit the ControlValue layout when stabilizing the ABI */\n static_assert(sizeof(ControlValue) == 16, \"Invalid size of ControlValue class\");\n \n /**\n","prefixes":["libcamera-devel","2/2"]}