[{"id":4155,"web_url":"https://patchwork.libcamera.org/comment/4155/","msgid":"<20200320212747.GQ5193@pendragon.ideasonboard.com>","date":"2020-03-20T21:27:47","subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: v4l2_device: Support\n\twriting array U8 controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Mon, Mar 09, 2020 at 05:24:10PM +0100, Jacopo Mondi wrote:\n> Add support to write array controls of type V4L2_CTRL_TYPE_U8.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/v4l2_device.cpp | 32 ++++++++++++++++++++++++++++----\n>  1 file changed, 28 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 65e97f92b01f..950e6286b84d 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -267,11 +267,25 @@ int V4L2Device::setControls(ControlList *ctrls)\n>  \t\tcase ControlTypeInteger64:\n>  \t\t\tv4l2Ctrls[i].value64 = value.get<int64_t>();\n>  \t\t\tbreak;\n> +\t\tcase ControlTypeByte: {\n> +\t\t\tif (!value.isArray())\n> +\t\t\t\t/*\n> +\t\t\t\t * \\todo This happens if a V4L2_CTRL_TYPE_U8\n> +\t\t\t\t * control is set with a non-array ControlValue.\n> +\t\t\t\t *\n> +\t\t\t\t * Should we fail loudly here ?\n> +\t\t\t\t */\n> +\t\t\t\tbreak;\n\nYes we should fail loudly, it's an error.\n\n> +\n> +\t\t\tauto values = value.get<Span<const uint8_t>>();\n\n\t\t\tSpan<const uint8_t> data = value.get<Span<const uint8_t>>();\n\n> +\t\t\tv4l2Ctrls[i].p_u8 = const_cast<uint8_t *>(values.data());\n\nThe VIDIOC_S_EXT_CTRLS call below will potentially update the value of\nthe control in the memory pointed by p_u8, so the const_cast here is\nreally not nice.\n\n> +\t\t\tv4l2Ctrls[i].size = values.size_bytes();\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\n>  \t\tdefault:\n> -\t\t\t/*\n> -\t\t\t * \\todo To be changed when support for string and\n> -\t\t\t * compound controls will be added.\n> -\t\t\t */\n> +\t\t\t/* \\todo To be changed to support strings. */\n>  \t\t\tv4l2Ctrls[i].value = value.get<int32_t>();\n>  \t\t\tbreak;\n>  \t\t}\n> @@ -413,6 +427,16 @@ void V4L2Device::updateControls(ControlList *ctrls,\n>  \t\tcase ControlTypeInteger64:\n>  \t\t\tvalue.set<int64_t>(v4l2Ctrl->value64);\n>  \t\t\tbreak;\n> +\n> +\t\tcase ControlTypeByte: {\n> +\t\t\tstd::vector<uint8_t> data = {\n> +\t\t\t\tv4l2Ctrl->p_u8, v4l2Ctrl->p_u8 + v4l2Ctrl->size\n> +\t\t\t};\n\nYou're creating a copy of the data, you should instead create the span\ndirectly.\n\n\t\t\tSpan<const uint8_t> data(v4l2Ctrl->p_u8, v4l2Ctrl->size);\n\t\t\tvalue.set(data);\n\n> +\t\t\tSpan<const uint8_t> values(data);\n> +\t\t\tvalue.set<Span<const uint8_t>>(values);\n> +\t\t\tbreak;\n\nBut in any case, all this isn't needed as the ioctl hs already updated\nthe value, as explained above.\n\n> +\t\t}\n> +\n>  \t\tdefault:\n>  \t\t\t/*\n>  \t\t\t * \\todo To be changed when support for string and","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 B40E560416\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Mar 2020 22:27:54 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2E2E2504;\n\tFri, 20 Mar 2020 22:27:54 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584739674;\n\tbh=KDi6uA8TUCGnGeG/ETbQqXIdI7qeFwBHhJlmNSkC+qk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dtcHtOMpdF2kVo33OL+i/TgeG7z7mjJvP0p9HoQf6to5uNSvtbMwMc2ZalFQA0/+r\n\twTPSwzcPhp4QZ1O3faSKdUxBAudm3zAihGqV9q/r5qY8nLpdwcX3huYnDYESLqhxya\n\t00pbkJR471+/KUYtVi8gajemYQvQmaVybksgbw58=","Date":"Fri, 20 Mar 2020 23:27:47 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200320212747.GQ5193@pendragon.ideasonboard.com>","References":"<20200309162414.720306-1-jacopo@jmondi.org>\n\t<20200309162414.720306-8-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200309162414.720306-8-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 07/11] libcamera: v4l2_device: Support\n\twriting array U8 controls","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 21:27:54 -0000"}}]