[{"id":13657,"web_url":"https://patchwork.libcamera.org/comment/13657/","msgid":"","date":"2020-11-10T09:33:05","subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi all,\n\nGentle ping to get some feedback on this patch set when you get a moment.\n\nRegards,\nNaush\n\n\nOn Mon, 26 Oct 2020 at 09:53, Naushir Patuck wrote:\n\n> Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline\n> handler to use in the cases where the sensor does not fill it in. This\n> avoids the need to mmap and unmap on every frame.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 35 ++++++++++++++-----\n> 1 file changed, 27 insertions(+), 8 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index dd62dfc7..8817915b 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -24,6 +24,7 @@\n> #include \n>\n> #include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/buffer.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/ipa_manager.h\"\n> @@ -165,6 +166,12 @@ public:\n> /* Stores the ids of the buffers mapped in the IPA. */\n> std::unordered_set ipaBuffers_;\n>\n> + /*\n> + * Map of (internal) mmaped embedded data buffers, to avoid having\n> to\n> + * map/unmap on every frame.\n> + */\n> + std::map mappedEmbeddedBuffers_;\n> +\n> /* DMAHEAP allocation helper. */\n> RPi::DmaHeap dmaHeap_;\n> FileDescriptor lsTable_;\n> @@ -1040,6 +1047,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera\n> *camera)\n> return ret;\n> }\n>\n> + if (!data->sensorMetadata_) {\n> + for (auto const &it :\n> data->unicam_[Unicam::Embedded].getBuffers()) {\n> +\n> data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct,\n> +\n> std::forward_as_tuple(it.first),\n> +\n> std::forward_as_tuple(\n> +\n> MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)));\n> + }\n> + }\n> +\n> /*\n> * Pass the stats and embedded data buffers to the IPA. No other\n> * buffers need to be passed.\n> @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> std::vector ipaBuffers(data->ipaBuffers_.begin(),\n> data->ipaBuffers_.end());\n> data->ipa_->unmapBuffers(ipaBuffers);\n> data->ipaBuffers_.clear();\n> + data->mappedEmbeddedBuffers_.clear();\n>\n> for (auto const stream : data->streams_)\n> stream->releaseBuffers();\n> @@ -1310,14 +1327,16 @@ void\n> RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> * metadata buffer.\n> */\n> if (!sensorMetadata_) {\n> - const FrameBuffer &fb = buffer->planes();\n> - uint32_t *mem = static_cast *>(::mmap(nullptr, fb.planes()[0].length,\n> -\n> PROT_READ | PROT_WRITE,\n> -\n> MAP_SHARED,\n> -\n> fb.planes()[0].fd.fd(), 0));\n> - mem[0] = ctrl[V4L2_CID_EXPOSURE];\n> - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> - munmap(mem, fb.planes()[0].length);\n> + unsigned int bufferId =\n> unicam_[Unicam::Embedded].getBufferId(buffer);\n> + auto it = mappedEmbeddedBuffers_.find(bufferId);\n> + if (it != mappedEmbeddedBuffers_.end()) {\n> + uint32_t *mem = reinterpret_cast *>(it->second.maps()[0].data());\n> + mem[0] = ctrl[V4L2_CID_EXPOSURE];\n> + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> + } else {\n> + LOG(RPI, Warning) << \"Failed to find\n> embedded buffer \"\n> + << bufferId;\n> + }\n> }\n> }\n>\n> --\n> 2.25.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 12D22BE082\n\tfor ;\n\tTue, 10 Nov 2020 09:33:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8217C630BB;\n\tTue, 10 Nov 2020 10:33:26 +0100 (CET)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BD6B60340\n\tfor ;\n\tTue, 10 Nov 2020 10:33:24 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id o24so7195797ljj.6\n\tfor ;\n\tTue, 10 Nov 2020 01:33:24 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"BOrIUqEE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to;\n\tbh=+DP/nrS2fFQQ3wTpeixX0lDpKelJJ/0P8JEOdSkygxI=;\n\tb=BOrIUqEEF7TGsmKyEVwmtEiFJA3jyvEQlrlrfQ7jTAc6jo5E+mbx9eH+y8YeM3xWDR\n\tJ6iQjalMDuHgkAbPvlnuxlxNr8az2D4p1o+YZapWnmQKWjYX5D1434PRyS+QL7yuyzcm\n\thfGI/W4yFlAB81M9nDxAmpCFXLl2hUYkDzRapIePofDDRB2H02ZFfHLSnPlxxocLBmUc\n\t8wcXOVQ/ukvvlHX2Nq6vNqWkPM4DnloAIYzOAO21+na4qwQG/NQzip77EPyzMkuwOIup\n\tWH6v2hr5BiIBxMU5P5E+1tWaITQTuutGj/N3rkZKMusNv2LUPq9V8lmnGeuOIoCb1pCF\n\tLVGw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=+DP/nrS2fFQQ3wTpeixX0lDpKelJJ/0P8JEOdSkygxI=;\n\tb=AArt/uKZlG04KjdjR3YUjN4wq41zZs0g9QRnKMYjkNTLbePQvK+M1NChNi0JtR3qM/\n\tZfbxezI1xdnNAyw5W+8M+6u92RaE8G2KC3ziBL9LPEe4h/7qiGSAPtt1KUFPFL8JrqTv\n\tzkkMZLi3tpVLWpd+d1lczAJDjxP9XnBJgCvVyGFJyTf5xTku5r3+RNh9f0+EOt4jW8Rs\n\toWjQlUkixU/oc/+PKBPhwwKajzi7/9HtVjXzOYJaEi+jMuZ4dL+ot9vTKkVP+uaFYOFQ\n\tkdFc7LrbPnqQMXcozu8JHKwopiBhY3yX8DNlrWg6jmlZ+1mSpX6UCL8MRwjVj4cwADBp\n\tP0NQ==","X-Gm-Message-State":"AOAM530VFuduvS3c+zFWcawp7gf6b30jm8oIHJ3jlVPaprllKOu2Y6sX\n\tUX/ojuvwOsU8AVswFsniMeIt01QGfxbhCbaeBBYFrYNrE6U=","X-Google-Smtp-Source":"ABdhPJxe0fYRoDYVqLyQUX+TQsy7x3jGKLAc4UADhN0rvVhgjwNHs4z2yvLeDI6v3ihHRN1rkayQQSS8hlBVxEA88QM=","X-Received":"by 2002:a2e:a410:: with SMTP id\n\tp16mr8329585ljn.252.1605000803177; \n\tTue, 10 Nov 2020 01:33:23 -0800 (PST)","MIME-Version":"1.0","References":"<20201026095356.442605-1-naush@raspberrypi.com>","In-Reply-To":"<20201026095356.442605-1-naush@raspberrypi.com>","From":"Naushir Patuck ","Date":"Tue, 10 Nov 2020 09:33:05 +0000","Message-ID":"","To":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"multipart/mixed;\n\tboundary=\"===============3826103537153629190==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13688,"web_url":"https://patchwork.libcamera.org/comment/13688/","msgid":"<435299dc-fdb1-dc04-da68-1b61c28ee9ef@ideasonboard.com>","date":"2020-11-12T22:16:30","subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 26/10/2020 09:53, Naushir Patuck wrote:\n> Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline\n> handler to use in the cases where the sensor does not fill it in. This\n> avoids the need to mmap and unmap on every frame.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 35 ++++++++++++++-----\n> 1 file changed, 27 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index dd62dfc7..8817915b 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -24,6 +24,7 @@\n> #include \n> \n> #include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/buffer.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/ipa_manager.h\"\n> @@ -165,6 +166,12 @@ public:\n> \t/* Stores the ids of the buffers mapped in the IPA. */\n> \tstd::unordered_set ipaBuffers_;\n> \n> +\t/*\n> +\t * Map of (internal) mmaped embedded data buffers, to avoid having to\n> +\t * map/unmap on every frame.\n> +\t */\n> +\tstd::map mappedEmbeddedBuffers_;\n> +\n> \t/* DMAHEAP allocation helper. */\n> \tRPi::DmaHeap dmaHeap_;\n> \tFileDescriptor lsTable_;\n> @@ -1040,6 +1047,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \t\t\treturn ret;\n> \t}\n> \n> +\tif (!data->sensorMetadata_) {\n> +\t\tfor (auto const &it : data->unicam_[Unicam::Embedded].getBuffers()) {\n> +\t\t\tdata->mappedEmbeddedBuffers_.emplace(std::piecewise_construct,\n> +\t\t\t\t\t\t\t std::forward_as_tuple(it.first),\n> +\t\t\t\t\t\t\t std::forward_as_tuple(\n> +\t\t\t\t\t\t\t MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)));\n\nDoes this need to use piecewise?\n\nCan't you just use:\n\t\n emplace(it.first,\n\t MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE));\ndirectly?\n\nOr is that then trying to use an invalid copy or move constructor?\n\nIf you do need to piecewise, Does it make sense to shorten it to:\n\nemplace(std::piecewise_construct,\n\tstd::forward_as_tuple(it.first),\n\tstd::forward_as_tuple(it.second, PROT_READ | PROT_WRITE)));\n\nAlthough maybe it's more clear showing the MappedFrameBuffer type.\n\nNot really a problem what route is used as long as it works I think ;-)\n\n\nJust tried out a few options and they all seem to compile, so I'd try to\nremove the piecewise if it's not needed ...\n\n> +\t\t}\n> +\t}\n> +\n> \t/*\n> \t * Pass the stats and embedded data buffers to the IPA. No other\n> \t * buffers need to be passed.\n> @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> \tstd::vector ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end());\n> \tdata->ipa_->unmapBuffers(ipaBuffers);\n> \tdata->ipaBuffers_.clear();\n> +\tdata->mappedEmbeddedBuffers_.clear();\n> \n> \tfor (auto const stream : data->streams_)\n> \t\tstream->releaseBuffers();\n> @@ -1310,14 +1327,16 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> \t\t * metadata buffer.\n> \t\t */\n> \t\tif (!sensorMetadata_) {\n> -\t\t\tconst FrameBuffer &fb = buffer->planes();\n> -\t\t\tuint32_t *mem = static_cast(::mmap(nullptr, fb.planes()[0].length,\n> -\t\t\t\t\t\t\t\t PROT_READ | PROT_WRITE,\n> -\t\t\t\t\t\t\t\t MAP_SHARED,\n> -\t\t\t\t\t\t\t\t fb.planes()[0].fd.fd(), 0));\n> -\t\t\tmem[0] = ctrl[V4L2_CID_EXPOSURE];\n> -\t\t\tmem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> -\t\t\tmunmap(mem, fb.planes()[0].length);\n\nOh, yes, keeping these mappings around rather than mapping and unmapping\neach frame will definitely help.\n\n> +\t\t\tunsigned int bufferId = unicam_[Unicam::Embedded].getBufferId(buffer);\n> +\t\t\tauto it = mappedEmbeddedBuffers_.find(bufferId);\n> +\t\t\tif (it != mappedEmbeddedBuffers_.end()) {\n> +\t\t\t\tuint32_t *mem = reinterpret_cast(it->second.maps()[0].data());\n\nI wonder if we should add a helper to MappedFrameBuffer to return a more\nfriendly data type to avoid casts like this, but that's not for this patch.\n\n\nWith the piecewise constructs either clarified that you want to keep\nthem, or simplified out:\n\nReviewed-by: Kieran Bingham \n\n\n\n> +\t\t\t\tmem[0] = ctrl[V4L2_CID_EXPOSURE];\n> +\t\t\t\tmem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> +\t\t\t} else {\n> +\t\t\t\tLOG(RPI, Warning) << \"Failed to find embedded buffer \"\n> +\t\t\t\t\t\t << bufferId;\n> +\t\t\t}\n> \t\t}\n> \t}\n> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3DB1BBE082\n\tfor ;\n\tThu, 12 Nov 2020 22:16:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B5761631AE;\n\tThu, 12 Nov 2020 23:16:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EDF4763171\n\tfor ;\n\tThu, 12 Nov 2020 23:16:33 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7197AA2A;\n\tThu, 12 Nov 2020 23:16:33 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lbL1jat+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1605219393;\n\tbh=dmrLblDrJY0ekTatKEz1iDWsU/BtXRn9uLQluim3ANM=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=lbL1jat+X8aBhK1eyyGG319vZwBHXsgOgO5D2gRpHb3cpYrSkHpTFa34mUT6YKtqj\n\ts7aUxxA7Y4ptj6ynbbFPWvt+LszaxvnmHQVoXcey3MysGBCGBSoOexBXeaEjDEXRw0\n\t3vA/8JHOT5YNCd9I0o8FV8t7ZQ9GMU8sgcNuqauU=","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201026095356.442605-1-naush@raspberrypi.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<435299dc-fdb1-dc04-da68-1b61c28ee9ef@ideasonboard.com>","Date":"Thu, 12 Nov 2020 22:16:30 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201026095356.442605-1-naush@raspberrypi.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13690,"web_url":"https://patchwork.libcamera.org/comment/13690/","msgid":"","date":"2020-11-12T22:47:46","subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\n\nOn Thu, 12 Nov 2020 at 22:16, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On 26/10/2020 09:53, Naushir Patuck wrote:\n> > Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline\n> > handler to use in the cases where the sensor does not fill it in. This\n> > avoids the need to mmap and unmap on every frame.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 35 ++++++++++++++-----\n> > 1 file changed, 27 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index dd62dfc7..8817915b 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -24,6 +24,7 @@\n> > #include \n> >\n> > #include \"libcamera/internal/bayer_format.h\"\n> > +#include \"libcamera/internal/buffer.h\"\n> > #include \"libcamera/internal/camera_sensor.h\"\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > #include \"libcamera/internal/ipa_manager.h\"\n> > @@ -165,6 +166,12 @@ public:\n> > /* Stores the ids of the buffers mapped in the IPA. */\n> > std::unordered_set ipaBuffers_;\n> >\n> > + /*\n> > + * Map of (internal) mmaped embedded data buffers, to avoid having\n> to\n> > + * map/unmap on every frame.\n> > + */\n> > + std::map mappedEmbeddedBuffers_;\n> > +\n> > /* DMAHEAP allocation helper. */\n> > RPi::DmaHeap dmaHeap_;\n> > FileDescriptor lsTable_;\n> > @@ -1040,6 +1047,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera\n> *camera)\n> > return ret;\n> > }\n> >\n> > + if (!data->sensorMetadata_) {\n> > + for (auto const &it :\n> data->unicam_[Unicam::Embedded].getBuffers()) {\n> > +\n> data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct,\n> > +\n> std::forward_as_tuple(it.first),\n> > +\n> std::forward_as_tuple(\n> > +\n> MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)));\n>\n> Does this need to use piecewise?\n>\n> Can't you just use:\n>\n> emplace(it.first,\n> MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE));\n> directly?\n>\n\nThis is hitting the limits of my C++ knowledge :-)\n\nI thought using a piecewise_construct might be more efficient here, as you\ndo not create a temporary object of MappedFrameBuffer on the stack and do a\ncopy. Although, the compiler might possibly optimise that out anyway....?\nNot sure. Either way, this is in startup code, and any possible penalty of\nconstructing and copying temporary objects is not really going to cause me\nto stay up at night. I'll change it to the above and post an update.\n\n\n> Or is that then trying to use an invalid copy or move constructor?\n>\n> If you do need to piecewise, Does it make sense to shorten it to:\n>\n> emplace(std::piecewise_construct,\n> std::forward_as_tuple(it.first),\n> std::forward_as_tuple(it.second, PROT_READ | PROT_WRITE)));\n>\n\nI did not think this would work, but admittedly never tried it!\n\nRegards,\nNaush\n\n\n\n> Although maybe it's more clear showing the MappedFrameBuffer type.\n>\n> Not really a problem what route is used as long as it works I think ;-)\n>\n>\n> Just tried out a few options and they all seem to compile, so I'd try to\n> remove the piecewise if it's not needed ...\n>\n> > + }\n> > + }\n> > +\n> > /*\n> > * Pass the stats and embedded data buffers to the IPA. No other\n> > * buffers need to be passed.\n> > @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera\n> *camera)\n> > std::vector ipaBuffers(data->ipaBuffers_.begin(),\n> data->ipaBuffers_.end());\n> > data->ipa_->unmapBuffers(ipaBuffers);\n> > data->ipaBuffers_.clear();\n> > + data->mappedEmbeddedBuffers_.clear();\n> >\n> > for (auto const stream : data->streams_)\n> > stream->releaseBuffers();\n> > @@ -1310,14 +1327,16 @@ void\n> RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> > * metadata buffer.\n> > */\n> > if (!sensorMetadata_) {\n> > - const FrameBuffer &fb = buffer->planes();\n> > - uint32_t *mem = static_cast *>(::mmap(nullptr, fb.planes()[0].length,\n> > -\n> PROT_READ | PROT_WRITE,\n> > -\n> MAP_SHARED,\n> > -\n> fb.planes()[0].fd.fd(), 0));\n> > - mem[0] = ctrl[V4L2_CID_EXPOSURE];\n> > - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> > - munmap(mem, fb.planes()[0].length);\n>\n> Oh, yes, keeping these mappings around rather than mapping and unmapping\n> each frame will definitely help.\n>\n> > + unsigned int bufferId =\n> unicam_[Unicam::Embedded].getBufferId(buffer);\n> > + auto it = mappedEmbeddedBuffers_.find(bufferId);\n> > + if (it != mappedEmbeddedBuffers_.end()) {\n> > + uint32_t *mem = reinterpret_cast *>(it->second.maps()[0].data());\n>\n> I wonder if we should add a helper to MappedFrameBuffer to return a more\n> friendly data type to avoid casts like this, but that's not for this patch.\n>\n>\n> With the piecewise constructs either clarified that you want to keep\n> them, or simplified out:\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n>\n> > + mem[0] = ctrl[V4L2_CID_EXPOSURE];\n> > + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> > + } else {\n> > + LOG(RPI, Warning) << \"Failed to find\n> embedded buffer \"\n> > + << bufferId;\n> > + }\n> > }\n> > }\n> >\n> >\n>\n> --\n> Regards\n> --\n> Kieran\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 61E4CBE082\n\tfor ;\n\tThu, 12 Nov 2020 22:48:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C19BF631B0;\n\tThu, 12 Nov 2020 23:48:06 +0100 (CET)","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n\t[IPv6:2a00:1450:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CBF9663171\n\tfor ;\n\tThu, 12 Nov 2020 23:48:05 +0100 (CET)","by mail-lf1-x12a.google.com with SMTP id u18so10923548lfd.9\n\tfor ;\n\tThu, 12 Nov 2020 14:48:05 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"hBMDHnAL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=qzjY6doHVS2ZP1VIq/CyPBugdpUEknYbencM9r8nU3w=;\n\tb=hBMDHnALmoLRcXQ5lOH2vRZ7ldIBhsmc6Soh3XLM4fBDC0loPs6kDSxr8DnZbWiv+V\n\tHW7T2iRxQn9XS0q2dBS2qyZZZQgBad3KQZFhvaka71ZtrBGKd/4Y5fHPEE/HhB9XaDT/\n\toZm6P2pOI5O7RDzz0IDBgY+HowLf1Zk+w+o3jt7uKJ2ssT8swvfddf9VPC2YhXjY3CM4\n\tXSH96zgfOK6mQWf5GWGiRWYRA8lAz9U8Kelhm6T4s1ipzeuYzsBB1Nq1I+y26eO8kA6N\n\to0E8dhPLFa6YVrAwpJV/hG9sP+Vsh3XMxJOS2441AB5htdJ2xUEnMr720Ri33x3d0Hlm\n\tD7cQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=qzjY6doHVS2ZP1VIq/CyPBugdpUEknYbencM9r8nU3w=;\n\tb=FgfSzFp7EQpWmOEOyBJj8oFOC+jzv/K0zsif+2QMMyw18thZxYAoXUe0pGTR/wlW0p\n\t0qiaDCXcH4hHCC/iba79kOyBq/KxBWj1jXROr0ec/xk8aM4Zw6TZYyZuRisqrh8VFF8M\n\tNHsBEwpkrEsdGtPUIgRQ6CuBRYxaLzIUWIVKSc+qPpUk9NhC22J/UcoYW7o86rHYrzvY\n\tWuAD9O2eI140a+wXANEKj2vSQlsoSo8AlsouE+qAWhM5NR2M1cqaVjdf63cg1+FV02sl\n\tdm0AKtoaIRJc6a5vDVue9KeQp5oj+TMIC0iYP+xjEyPiqYSwhPDEK1j9llX+95f73yVA\n\twYhw==","X-Gm-Message-State":"AOAM532aQSRnyR3DxnQo9xkDjSUlqXrNUfLhczSmsY5T1p4ZgLATWN5P\n\ttoULjNlVmW4hjTWHvCLsxqKWqw/sJIkVST2mBzLtvQ==","X-Google-Smtp-Source":"ABdhPJyzFFJKB0yaPBe6wi0WyMpYitUnSt2CyUAFYYZDBcixovTn1CzEOnH1YxRKYXC1wYBTRL7f4r6oSRBwO4X+Dj4=","X-Received":"by 2002:a19:6e46:: with SMTP id q6mr647697lfk.413.1605221284967; \n\tThu, 12 Nov 2020 14:48:04 -0800 (PST)","MIME-Version":"1.0","References":"<20201026095356.442605-1-naush@raspberrypi.com>\n\t<435299dc-fdb1-dc04-da68-1b61c28ee9ef@ideasonboard.com>","In-Reply-To":"<435299dc-fdb1-dc04-da68-1b61c28ee9ef@ideasonboard.com>","From":"Naushir Patuck ","Date":"Thu, 12 Nov 2020 22:47:46 +0000","Message-ID":"","To":"Kieran Bingham ","Subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============3548450680397190929==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13691,"web_url":"https://patchwork.libcamera.org/comment/13691/","msgid":"<3c3dddd0-7142-8399-4075-0b7c13ad85c2@ideasonboard.com>","date":"2020-11-12T22:58:27","subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 12/11/2020 22:47, Naushir Patuck wrote:\n> Hi Kieran,\n> \n> \n> On Thu, 12 Nov 2020 at 22:16, Kieran Bingham\n> > wrote:\n> \n> Hi Naush,\n> \n> On 26/10/2020 09:53, Naushir Patuck wrote:\n> > Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline\n> > handler to use in the cases where the sensor does not fill it in. This\n> > avoids the need to mmap and unmap on every frame.\n> >\n> > Signed-off-by: Naushir Patuck >\n> > ---\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 35\n> ++++++++++++++-----\n> >  1 file changed, 27 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index dd62dfc7..8817915b 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -24,6 +24,7 @@\n> >  #include \n> > \n> >  #include \"libcamera/internal/bayer_format.h\"\n> > +#include \"libcamera/internal/buffer.h\"\n> >  #include \"libcamera/internal/camera_sensor.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> >  #include \"libcamera/internal/ipa_manager.h\"\n> > @@ -165,6 +166,12 @@ public:\n> >       /* Stores the ids of the buffers mapped in the IPA. */\n> >       std::unordered_set ipaBuffers_;\n> > \n> > +     /*\n> > +      * Map of (internal) mmaped embedded data buffers, to avoid\n> having to\n> > +      * map/unmap on every frame.\n> > +      */\n> > +     std::map\n> mappedEmbeddedBuffers_;\n> > +\n> >       /* DMAHEAP allocation helper. */\n> >       RPi::DmaHeap dmaHeap_;\n> >       FileDescriptor lsTable_;\n> > @@ -1040,6 +1047,15 @@ int\n> PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> >                       return ret;\n> >       }\n> > \n> > +     if (!data->sensorMetadata_) {\n> > +             for (auto const &it :\n> data->unicam_[Unicam::Embedded].getBuffers()) {\n> > +                   \n>  data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct,\n> > +                                                         \n> std::forward_as_tuple(it.first),\n> > +                                                         \n> std::forward_as_tuple(\n> > +                                                         \n> MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE)));\n> \n> Does this need to use piecewise?\n> \n> Can't you just use:\n> \n>    emplace(it.first,\n>            MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE));\n> directly?\n> \n> \n> This is hitting the limits of my C++ knowledge :-)\n> \n\npiecewise requirements stretched mine a bit too. Back in January Laurent\ndid some code simplification around this, and I didn't really understand\nwhat it was doing then so I (while sat next to him, quite the novelty)\ngot him to write a long commit message explaining it more..\n\nNot sure if it helps or not, but this is the relevant commit message there:\n\nhttps://git.libcamera.org/libcamera/libcamera.git/commit/?id=38dd90307ab2b0d25a0a233eae04455f769153b4\n\n\n\n> I thought using a piecewise_construct might be more efficient here, as\n> you do not create a temporary object of MappedFrameBuffer on the stack\n> and do a copy.  Although, the compiler might possibly optimise that out\n> anyway....?  Not sure.  Either way, this is in startup code, and any\n\nI think by doing the emplace we're already heading to that efficiency.\nThe piecewise is needed when the thing being emplaced is more complex or\nwhen one of the things being emplaced needs more than one argument.\n\n> possible penalty of constructing and copying temporary objects is not\n> really going to cause me to stay up at night.  I'll change it to the\n> above and post an update.\n\nI think\n\n emplace(std::piecewise_construct,\n std::forward_as_tuple(it.first),\n std::forward_as_tuple(it.second, PROT_READ | PROT_WRITE)));\n\nwould ensure the performance gain, but I would fear:\n\n emplace(std::piecewise_construct,\n std::forward_as_tuple(it.first),\n std::forward_as_tuple(MappedFrameBuffer(it.second, PROT_READ\n| PROT_WRITE))));\n\nwould be invoking the MappedFrameBuffer constructor explicitly, forcing\na copy or a move. But I'm not really sure what the compiler does here\nwithout going through and putting some prints in to see what gets invoked.\n\nAs this construct is only at initialise I'm not worried by that, but I\ndon't think the MappedFrameBuffers are copyable anyway, as that would\nlead to two objects that could unmap the same memory.\n\nI assume I handled that all in the MappedFrameBuffer class though ;-)\n\nWell - on the topic of not losing any sleep over this - I'm going to go\nand try to find some! hehe\n\n\n--\nKieran\n\n> \n> \n> Or is that then trying to use an invalid copy or move constructor?\n> \n> If you do need to piecewise, Does it make sense to shorten it to:\n> \n> emplace(std::piecewise_construct,\n>         std::forward_as_tuple(it.first),\n>         std::forward_as_tuple(it.second, PROT_READ | PROT_WRITE)));\n>\n> I did not think this would work, but admittedly never tried it!\n> \n> Regards,\n> Naush\n> \n>  \n> \n> Although maybe it's more clear showing the MappedFrameBuffer type.\n> \n> Not really a problem what route is used as long as it works I think ;-)\n> \n> \n> Just tried out a few options and they all seem to compile, so I'd try to\n> remove the piecewise if it's not needed ...\n> \n> > +             }\n> > +     }\n> > +\n> >       /*\n> >        * Pass the stats and embedded data buffers to the IPA. No other\n> >        * buffers need to be passed.\n> > @@ -1078,6 +1094,7 @@ void PipelineHandlerRPi::freeBuffers(Camera\n> *camera)\n> >       std::vector\n> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end());\n> >       data->ipa_->unmapBuffers(ipaBuffers);\n> >       data->ipaBuffers_.clear();\n> > +     data->mappedEmbeddedBuffers_.clear();\n> > \n> >       for (auto const stream : data->streams_)\n> >               stream->releaseBuffers();\n> > @@ -1310,14 +1327,16 @@ void\n> RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >                * metadata buffer.\n> >                */\n> >               if (!sensorMetadata_) {\n> > -                     const FrameBuffer &fb = buffer->planes();\n> > -                     uint32_t *mem = static_cast *>(::mmap(nullptr, fb.planes()[0].length,\n> > -                                                                 \n>   PROT_READ | PROT_WRITE,\n> > -                                                                 \n>   MAP_SHARED,\n> > -                                                                 \n>   fb.planes()[0].fd.fd(), 0));\n> > -                     mem[0] = ctrl[V4L2_CID_EXPOSURE];\n> > -                     mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> > -                     munmap(mem, fb.planes()[0].length);\n> \n> Oh, yes, keeping these mappings around rather than mapping and unmapping\n> each frame will definitely help.\n> \n> > +                     unsigned int bufferId =\n> unicam_[Unicam::Embedded].getBufferId(buffer);\n> > +                     auto it = mappedEmbeddedBuffers_.find(bufferId);\n> > +                     if (it != mappedEmbeddedBuffers_.end()) {\n> > +                             uint32_t *mem =\n> reinterpret_cast(it->second.maps()[0].data());\n> \n> I wonder if we should add a helper to MappedFrameBuffer to return a more\n> friendly data type to avoid casts like this, but that's not for this\n> patch.\n> \n> \n> With the piecewise constructs either clarified that you want to keep\n> them, or simplified out:\n> \n> Reviewed-by: Kieran Bingham >\n> \n> \n> \n> > +                             mem[0] = ctrl[V4L2_CID_EXPOSURE];\n> > +                             mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];\n> > +                     } else {\n> > +                             LOG(RPI, Warning) << \"Failed to find\n> embedded buffer \"\n> > +                                               << bufferId;\n> > +                     }\n> >               }\n> >       }\n> > \n> >\n> \n> -- \n> Regards\n> --\n> Kieran\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id ACC86BE086\n\tfor ;\n\tThu, 12 Nov 2020 22:58:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C9E763171;\n\tThu, 12 Nov 2020 23:58:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C5EA63171\n\tfor ;\n\tThu, 12 Nov 2020 23:58:30 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B0D77A2A;\n\tThu, 12 Nov 2020 23:58:29 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PO8mbrve\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1605221909;\n\tbh=8Z8t1z1KGQ24q9AE4WuIurcqPIGXdcf9iXY45QSq7wQ=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=PO8mbrve7DX4EtUFyu6qaqPGdCM5Q1xf9E36tUTzr4Q6PpUlR9dis+M/zKBS1gcYC\n\t0BPHJ6ozpyNGT0Ot7RtBa4I+GII6X+fU5LO71s1TA/pM0VuYHjxWcxVBe0mxK/Xp7s\n\tb0+QPCM+cCpc1mWigw4siIFM+LMRMl2mADtCZJPw=","To":"Naushir Patuck ","References":"<20201026095356.442605-1-naush@raspberrypi.com>\n\t<435299dc-fdb1-dc04-da68-1b61c28ee9ef@ideasonboard.com>\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<3c3dddd0-7142-8399-4075-0b7c13ad85c2@ideasonboard.com>","Date":"Thu, 12 Nov 2020 22:58:27 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Use\n\tMappedFrameBuffer for embedded data buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]