[{"id":3945,"web_url":"https://patchwork.libcamera.org/comment/3945/","msgid":"<2b2cb5ca-7e5f-1f81-fca5-cf7fd4b4fed0@ideasonboard.com>","date":"2020-03-05T17:19:51","subject":"Re: [libcamera-devel] [PATCH 26/31] libcamera: control_serializer:\n\tUse zero-copy ByteStreamBuffer::read()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 29/02/2020 16:42, Laurent Pinchart wrote:\n> Use the zero-copy variant of ByteStreamBuffer::read() to read packet\n> haders and control entries. This enhance performance of ControlList and\n\n/haders/headers/\n\n/enhance/enhances the/ or /enhance/will enhance the/\n\n\n> ControlInfoMap deserialization.\n> \n> Deserialization of the actual ControlValue is untouched for now and will\n> be optimized later.\n> \n> Signed-off-by: Laurent Pinchart \n\nVery happy to see removal of copies from our serialization :)\n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/libcamera/control_serializer.cpp | 74 +++++++++++++++++-----------\n> 1 file changed, 44 insertions(+), 30 deletions(-)\n> \n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index dc87b96f384b..6cac70739468 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -364,39 +364,46 @@ ControlRange ControlSerializer::load(ControlType type,\n> template<>\n> ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &buffer)\n> {\n> -\tstruct ipa_controls_header hdr;\n> -\tbuffer.read(&hdr);\n> +\tconst struct ipa_controls_header *hdr = buffer.read();\n\nAha, now I see why it doesn't return a span ...\n\nThis looks like a lot of template additions ... couldn't they be just as\neasily added for this functionality with taking a size parameter and\nreturning a void pointer?\n\n(or does C++ not like that)\n\n\n> +\tif (!hdr) {\n> +\t\tLOG(Serializer, Error) << \"Out of data\";\n> +\t\treturn {};\n> +\t}\n> \n> -\tif (hdr.version != IPA_CONTROLS_FORMAT_VERSION) {\n> +\tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n> \t\tLOG(Serializer, Error)\n> \t\t\t<< \"Unsupported controls format version \"\n> -\t\t\t<< hdr.version;\n> +\t\t\t<< hdr->version;\n> \t\treturn {};\n> \t}\n> \n> -\tByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr));\n> -\tByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset);\n> +\tByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr));\n> +\tByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset);\n> \n> \tif (buffer.overflow()) {\n> -\t\tLOG(Serializer, Error) << \"Serialized packet too small\";\n> +\t\tLOG(Serializer, Error) << \"Out of data\";\n> \t\treturn {};\n> \t}\n> \n> \tControlInfoMap::Map ctrls;\n> \n> -\tfor (unsigned int i = 0; i < hdr.entries; ++i) {\n> -\t\tstruct ipa_control_range_entry entry;\n> -\t\tentries.read(&entry);\n> +\tfor (unsigned int i = 0; i < hdr->entries; ++i) {\n> +\t\tconst struct ipa_control_range_entry *entry =\n> +\t\t\tentries.read();\n> +\t\tif (!entry) {\n> +\t\t\tLOG(Serializer, Error) << \"Out of data\";\n> +\t\t\treturn {};\n> +\t\t}\n> \n> \t\t/* Create and cache the individual ControlId. */\n> -\t\tControlType type = static_cast(entry.type);\n> +\t\tControlType type = static_cast(entry->type);\n> \t\t/**\n> \t\t * \\todo Find a way to preserve the control name for debugging\n> \t\t * purpose.\n> \t\t */\n> -\t\tcontrolIds_.emplace_back(std::make_unique(entry.id, \"\", type));\n> +\t\tcontrolIds_.emplace_back(std::make_unique(entry->id, \"\", type));\n> \n> -\t\tif (entry.offset != values.offset()) {\n> +\t\tif (entry->offset != values.offset()) {\n> \t\t\tLOG(Serializer, Error)\n> \t\t\t\t<< \"Bad data, entry offset mismatch (entry \"\n> \t\t\t\t<< i << \")\";\n> @@ -412,8 +419,8 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> \t * Create the ControlInfoMap in the cache, and store the map to handle\n> \t * association.\n> \t */\n> -\tControlInfoMap &map = infoMaps_[hdr.handle] = std::move(ctrls);\n> -\tinfoMapHandles_[&map] = hdr.handle;\n> +\tControlInfoMap &map = infoMaps_[hdr->handle] = std::move(ctrls);\n> +\tinfoMapHandles_[&map] = hdr->handle;\n> \n> \treturn map;\n> }\n> @@ -430,21 +437,24 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> template<>\n> ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer)\n> {\n> -\tstruct ipa_controls_header hdr;\n> -\tbuffer.read(&hdr);\n> +\tconst struct ipa_controls_header *hdr = buffer.read();\n> +\tif (!hdr) {\n> +\t\tLOG(Serializer, Error) << \"Out of data\";\n> +\t\treturn {};\n> +\t}\n> \n> -\tif (hdr.version != IPA_CONTROLS_FORMAT_VERSION) {\n> +\tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n> \t\tLOG(Serializer, Error)\n> \t\t\t<< \"Unsupported controls format version \"\n> -\t\t\t<< hdr.version;\n> +\t\t\t<< hdr->version;\n> \t\treturn {};\n> \t}\n> \n> -\tByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr));\n> -\tByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset);\n> +\tByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr));\n> +\tByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset);\n> \n> \tif (buffer.overflow()) {\n> -\t\tLOG(Serializer, Error) << \"Serialized packet too small\";\n> +\t\tLOG(Serializer, Error) << \"Out of data\";\n> \t\treturn {};\n> \t}\n> \n> @@ -456,10 +466,10 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> \t * use the global control::control idmap.\n> \t */\n> \tconst ControlInfoMap *infoMap;\n> -\tif (hdr.handle) {\n> +\tif (hdr->handle) {\n> \t\tauto iter = std::find_if(infoMapHandles_.begin(), infoMapHandles_.end(),\n> \t\t\t\t\t [&](decltype(infoMapHandles_)::value_type &entry) {\n> -\t\t\t\t\t\t return entry.second == hdr.handle;\n> +\t\t\t\t\t\t return entry.second == hdr->handle;\n> \t\t\t\t\t });\n> \t\tif (iter == infoMapHandles_.end()) {\n> \t\t\tLOG(Serializer, Error)\n> @@ -474,19 +484,23 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> \n> \tControlList ctrls(infoMap ? infoMap->idmap() : controls::controls);\n> \n> -\tfor (unsigned int i = 0; i < hdr.entries; ++i) {\n> -\t\tstruct ipa_control_value_entry entry;\n> -\t\tentries.read(&entry);\n> +\tfor (unsigned int i = 0; i < hdr->entries; ++i) {\n> +\t\tconst struct ipa_control_value_entry *entry =\n> +\t\t\tentries.read();\n> +\t\tif (!entry) {\n> +\t\t\tLOG(Serializer, Error) << \"Out of data\";\n> +\t\t\treturn {};\n> +\t\t}\n> \n> -\t\tif (entry.offset != values.offset()) {\n> +\t\tif (entry->offset != values.offset()) {\n> \t\t\tLOG(Serializer, Error)\n> \t\t\t\t<< \"Bad data, entry offset mismatch (entry \"\n> \t\t\t\t<< i << \")\";\n> \t\t\treturn {};\n> \t\t}\n> \n> -\t\tControlType type = static_cast(entry.type);\n> -\t\tctrls.set(entry.id, load(type, values));\n> +\t\tControlType type = static_cast(entry->type);\n> +\t\tctrls.set(entry->id, load(type, values));\n> \t}\n> \n> \treturn ctrls;\n>","headers":{"Return-Path":"","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 597E560427\n\tfor ;\n\tThu, 5 Mar 2020 18:19:58 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D0D2F312;\n\tThu, 5 Mar 2020 18:19:57 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583428798;\n\tbh=l8LMu8s7QHdm4GDs7kv6jEvEJlk91dHPxVUSUW0Ki94=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=k3n9bQW8npvuE/WyLmTF2LCCSIxbVy2TENedXD8N3RobnHZyG01Z9E3VVxdU8pJbs\n\t1Gi+S6kkSC3Yu2kd3jgMIhJvYsuwFtTxjDhkESBJBG1SMFuXlL4bsQX9NN1VgJDXoo\n\t0URxs9DRz1qzw+/3mAxsMu8XOJjMgcJsgdWZUmSs=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200229164254.23604-1-laurent.pinchart@ideasonboard.com>\n\t<20200229164254.23604-27-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","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":"<2b2cb5ca-7e5f-1f81-fca5-cf7fd4b4fed0@ideasonboard.com>","Date":"Thu, 5 Mar 2020 17:19:51 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200229164254.23604-27-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 26/31] libcamera: control_serializer:\n\tUse zero-copy ByteStreamBuffer::read()","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","X-List-Received-Date":"Thu, 05 Mar 2020 17:19:58 -0000"}},{"id":3955,"web_url":"https://patchwork.libcamera.org/comment/3955/","msgid":"<20200306145831.GL4878@pendragon.ideasonboard.com>","date":"2020-03-06T14:58:31","subject":"Re: [libcamera-devel] [PATCH 26/31] libcamera: control_serializer:\n\tUse zero-copy ByteStreamBuffer::read()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Thu, Mar 05, 2020 at 05:19:51PM +0000, Kieran Bingham wrote:\n> On 29/02/2020 16:42, Laurent Pinchart wrote:\n> > Use the zero-copy variant of ByteStreamBuffer::read() to read packet\n> > haders and control entries. This enhance performance of ControlList and\n> \n> /haders/headers/\n> \n> /enhance/enhances the/ or /enhance/will enhance the/\n> \n> > ControlInfoMap deserialization.\n> > \n> > Deserialization of the actual ControlValue is untouched for now and will\n> > be optimized later.\n> > \n> > Signed-off-by: Laurent Pinchart \n> \n> Very happy to see removal of copies from our serialization :)\n> \n> Reviewed-by: Kieran Bingham \n> \n> > ---\n> > src/libcamera/control_serializer.cpp | 74 +++++++++++++++++-----------\n> > 1 file changed, 44 insertions(+), 30 deletions(-)\n> > \n> > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> > index dc87b96f384b..6cac70739468 100644\n> > --- a/src/libcamera/control_serializer.cpp\n> > +++ b/src/libcamera/control_serializer.cpp\n> > @@ -364,39 +364,46 @@ ControlRange ControlSerializer::load(ControlType type,\n> > template<>\n> > ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &buffer)\n> > {\n> > -\tstruct ipa_controls_header hdr;\n> > -\tbuffer.read(&hdr);\n> > +\tconst struct ipa_controls_header *hdr = buffer.read();\n> \n> Aha, now I see why it doesn't return a span ...\n> \n> This looks like a lot of template additions ... couldn't they be just as\n> easily added for this functionality with taking a size parameter and\n> returning a void pointer?\n> \n> (or does C++ not like that)\n\nThat would require casting in the caller, which is potentially unsafe.\nWith the above code the compiler will tell you if the variable to which\nyou assigned the returned pointer is of a different type than the type\npassed to the read<> template function. It would be nice if C++ could\nperform template type deduction based no the return value.\n\n> > +\tif (!hdr) {\n> > +\t\tLOG(Serializer, Error) << \"Out of data\";\n> > +\t\treturn {};\n> > +\t}\n> > \n> > -\tif (hdr.version != IPA_CONTROLS_FORMAT_VERSION) {\n> > +\tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n> > \t\tLOG(Serializer, Error)\n> > \t\t\t<< \"Unsupported controls format version \"\n> > -\t\t\t<< hdr.version;\n> > +\t\t\t<< hdr->version;\n> > \t\treturn {};\n> > \t}\n> > \n> > -\tByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr));\n> > -\tByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset);\n> > +\tByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr));\n> > +\tByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset);\n> > \n> > \tif (buffer.overflow()) {\n> > -\t\tLOG(Serializer, Error) << \"Serialized packet too small\";\n> > +\t\tLOG(Serializer, Error) << \"Out of data\";\n> > \t\treturn {};\n> > \t}\n> > \n> > \tControlInfoMap::Map ctrls;\n> > \n> > -\tfor (unsigned int i = 0; i < hdr.entries; ++i) {\n> > -\t\tstruct ipa_control_range_entry entry;\n> > -\t\tentries.read(&entry);\n> > +\tfor (unsigned int i = 0; i < hdr->entries; ++i) {\n> > +\t\tconst struct ipa_control_range_entry *entry =\n> > +\t\t\tentries.read();\n> > +\t\tif (!entry) {\n> > +\t\t\tLOG(Serializer, Error) << \"Out of data\";\n> > +\t\t\treturn {};\n> > +\t\t}\n> > \n> > \t\t/* Create and cache the individual ControlId. */\n> > -\t\tControlType type = static_cast(entry.type);\n> > +\t\tControlType type = static_cast(entry->type);\n> > \t\t/**\n> > \t\t * \\todo Find a way to preserve the control name for debugging\n> > \t\t * purpose.\n> > \t\t */\n> > -\t\tcontrolIds_.emplace_back(std::make_unique(entry.id, \"\", type));\n> > +\t\tcontrolIds_.emplace_back(std::make_unique(entry->id, \"\", type));\n> > \n> > -\t\tif (entry.offset != values.offset()) {\n> > +\t\tif (entry->offset != values.offset()) {\n> > \t\t\tLOG(Serializer, Error)\n> > \t\t\t\t<< \"Bad data, entry offset mismatch (entry \"\n> > \t\t\t\t<< i << \")\";\n> > @@ -412,8 +419,8 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> > \t * Create the ControlInfoMap in the cache, and store the map to handle\n> > \t * association.\n> > \t */\n> > -\tControlInfoMap &map = infoMaps_[hdr.handle] = std::move(ctrls);\n> > -\tinfoMapHandles_[&map] = hdr.handle;\n> > +\tControlInfoMap &map = infoMaps_[hdr->handle] = std::move(ctrls);\n> > +\tinfoMapHandles_[&map] = hdr->handle;\n> > \n> > \treturn map;\n> > }\n> > @@ -430,21 +437,24 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> > template<>\n> > ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer)\n> > {\n> > -\tstruct ipa_controls_header hdr;\n> > -\tbuffer.read(&hdr);\n> > +\tconst struct ipa_controls_header *hdr = buffer.read();\n> > +\tif (!hdr) {\n> > +\t\tLOG(Serializer, Error) << \"Out of data\";\n> > +\t\treturn {};\n> > +\t}\n> > \n> > -\tif (hdr.version != IPA_CONTROLS_FORMAT_VERSION) {\n> > +\tif (hdr->version != IPA_CONTROLS_FORMAT_VERSION) {\n> > \t\tLOG(Serializer, Error)\n> > \t\t\t<< \"Unsupported controls format version \"\n> > -\t\t\t<< hdr.version;\n> > +\t\t\t<< hdr->version;\n> > \t\treturn {};\n> > \t}\n> > \n> > -\tByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr));\n> > -\tByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset);\n> > +\tByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr));\n> > +\tByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset);\n> > \n> > \tif (buffer.overflow()) {\n> > -\t\tLOG(Serializer, Error) << \"Serialized packet too small\";\n> > +\t\tLOG(Serializer, Error) << \"Out of data\";\n> > \t\treturn {};\n> > \t}\n> > \n> > @@ -456,10 +466,10 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> > \t * use the global control::control idmap.\n> > \t */\n> > \tconst ControlInfoMap *infoMap;\n> > -\tif (hdr.handle) {\n> > +\tif (hdr->handle) {\n> > \t\tauto iter = std::find_if(infoMapHandles_.begin(), infoMapHandles_.end(),\n> > \t\t\t\t\t [&](decltype(infoMapHandles_)::value_type &entry) {\n> > -\t\t\t\t\t\t return entry.second == hdr.handle;\n> > +\t\t\t\t\t\t return entry.second == hdr->handle;\n> > \t\t\t\t\t });\n> > \t\tif (iter == infoMapHandles_.end()) {\n> > \t\t\tLOG(Serializer, Error)\n> > @@ -474,19 +484,23 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n> > \n> > \tControlList ctrls(infoMap ? infoMap->idmap() : controls::controls);\n> > \n> > -\tfor (unsigned int i = 0; i < hdr.entries; ++i) {\n> > -\t\tstruct ipa_control_value_entry entry;\n> > -\t\tentries.read(&entry);\n> > +\tfor (unsigned int i = 0; i < hdr->entries; ++i) {\n> > +\t\tconst struct ipa_control_value_entry *entry =\n> > +\t\t\tentries.read();\n> > +\t\tif (!entry) {\n> > +\t\t\tLOG(Serializer, Error) << \"Out of data\";\n> > +\t\t\treturn {};\n> > +\t\t}\n> > \n> > -\t\tif (entry.offset != values.offset()) {\n> > +\t\tif (entry->offset != values.offset()) {\n> > \t\t\tLOG(Serializer, Error)\n> > \t\t\t\t<< \"Bad data, entry offset mismatch (entry \"\n> > \t\t\t\t<< i << \")\";\n> > \t\t\treturn {};\n> > \t\t}\n> > \n> > -\t\tControlType type = static_cast(entry.type);\n> > -\t\tctrls.set(entry.id, load(type, values));\n> > +\t\tControlType type = static_cast(entry->type);\n> > +\t\tctrls.set(entry->id, load(type, values));\n> > \t}\n> > \n> > \treturn ctrls;","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C4E060424\n\tfor ;\n\tFri, 6 Mar 2020 15:58:34 +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 BDD3624B;\n\tFri, 6 Mar 2020 15:58:33 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583506713;\n\tbh=PML3CA8NAOl7qG9srBPDFpE3NDXhTLWXZI5cN79F31g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ogW9fJG4opTeT+fM8jLHggtlVsRuaGBMfxqM3UrXLwFdS07n4n7pPZIXOQ0Hlm2Kv\n\tf23zRUEAcALmT6frSxG2QEkR62aj7YJxBeY+FZS1vZQlACrnbiLECbAEbVlKDfnLDY\n\t1lP3nlaP5QSdfN1PRKCbk5iUIx8NDdmZ+i+JpYK4=","Date":"Fri, 6 Mar 2020 16:58:31 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200306145831.GL4878@pendragon.ideasonboard.com>","References":"<20200229164254.23604-1-laurent.pinchart@ideasonboard.com>\n\t<20200229164254.23604-27-laurent.pinchart@ideasonboard.com>\n\t<2b2cb5ca-7e5f-1f81-fca5-cf7fd4b4fed0@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<2b2cb5ca-7e5f-1f81-fca5-cf7fd4b4fed0@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 26/31] libcamera: control_serializer:\n\tUse zero-copy ByteStreamBuffer::read()","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","X-List-Received-Date":"Fri, 06 Mar 2020 14:58:34 -0000"}}]