[{"id":11061,"web_url":"https://patchwork.libcamera.org/comment/11061/","msgid":"<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>","date":"2020-07-01T21:42:38","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 01/07/2020 22:09, Niklas Söderlund wrote:\n> Use a helper instead of local code to retrieve all keys from a map.\n> \n\nI see this is the reason for changing Laurent's patch to return a\nvector, but couldn't this patch update things to use sets locally?\n\nThe keys are 'unique' right? Is there a distinct benefit for returning a\nvector over a set?\n\nPerhaps there is a performance improvement with a vector if it doesn't\nneed to ensure uniqueness? But I'd be surprised if it was much... but I\ndon't know.\n\nOr is it to prevent updating the functions that the set (vector) is then\npassed to, i.e. sensor_->getFormat() ?\n\n--\nKieran\n\n\n\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n>  1 file changed, 3 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index aa1459fb3599283b..7941c6845cbc9a9a 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n>  \t * utils::set_overlap requires the ranges to be sorted, keep the\n>  \t * cio2Codes vector sorted in ascending order.\n>  \t */\n> -\tstd::vector<unsigned int> cio2Codes;\n> -\tcio2Codes.reserve(mbusCodesToInfo.size());\n> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> -\t\t       std::back_inserter(cio2Codes),\n> -\t\t       [](auto &pair) { return pair.first; });\n> +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n>  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n>  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n>  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n> @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n>  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n>  \t * the CIO2 output device.\n>  \t */\n> -\tstd::vector<unsigned int> mbusCodes;\n> -\tmbusCodes.reserve(mbusCodesToInfo.size());\n> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> -\t\t       std::back_inserter(mbusCodes),\n> -\t\t       [](auto &pair) { return pair.first; });\n> -\n> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n>  \tret = sensor_->setFormat(&sensorFormat);\n>  \tif (ret)\n> @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n>  \t\tsize = sensor_->resolution();\n>  \n>  \t/* Query the sensor static information for closest match. */\n> -\tstd::vector<unsigned int> mbusCodes;\n> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> -\t\t       std::back_inserter(mbusCodes),\n> -\t\t       [](auto &pair) { return pair.first; });\n> -\n> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n>  \tif (!sensorFormat.mbus_code) {\n>  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7CDE7BFFE2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  1 Jul 2020 21:42:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1353A60C57;\n\tWed,  1 Jul 2020 23:42:42 +0200 (CEST)","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 1CCFB609A9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  1 Jul 2020 23:42:41 +0200 (CEST)","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 82372556;\n\tWed,  1 Jul 2020 23:42:40 +0200 (CEST)"],"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=\"FyT50NBO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593639760;\n\tbh=qlV7R7krSAIHaP7zDstyTe8o97D8b9au+ApLC3mIe9Q=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=FyT50NBOd/6kZ2QKTIbMFKA/HCE3FXrfOVr9+gh/n1qp+7QC27C6MhzP+EFcCA/F4\n\ty1fPPcTI88JRnkRBdhFHHSrmEvgF+HcW69/cRdCr/QMVJL+hwqMz1caoUOi+qlZXBX\n\tgfx7ZEFkEBaCWVJcp3nDTuZTxTcTXl8t8J/8o+4c=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>","Date":"Wed, 1 Jul 2020 22:42:38 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200701210948.999363-3-niklas.soderlund@ragnatech.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11062,"web_url":"https://patchwork.libcamera.org/comment/11062/","msgid":"<20200701215625.GB2703142@oden.dyn.berto.se>","date":"2020-07-01T21:56:25","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nThanks for your feedback.\n\nOn 2020-07-01 22:42:38 +0100, Kieran Bingham wrote:\n> Hi Niklas,\n> \n> On 01/07/2020 22:09, Niklas Söderlund wrote:\n> > Use a helper instead of local code to retrieve all keys from a map.\n> > \n> \n> I see this is the reason for changing Laurent's patch to return a\n> vector, but couldn't this patch update things to use sets locally?\n> \n> The keys are 'unique' right? Is there a distinct benefit for returning a\n> vector over a set?\n> \n> Perhaps there is a performance improvement with a vector if it doesn't\n> need to ensure uniqueness? But I'd be surprised if it was much... but I\n> don't know.\n> \n> Or is it to prevent updating the functions that the set (vector) is then\n> passed to, i.e. sensor_->getFormat() ?\n\nI would prefer it to be a std::set, but par the discussion around v1 of \nthis patch and the last two versions of the series that introduce \nCIO2Devcice (which is now merged) it seemed the quickest way to avoid \nbikeshedding making it a std::vector.\n\nWe can always later switch the container to a std::set if it becomes \nless trouble then remember to call std::sort() where it mattes, or for \nthat matter start with calling std::sort() in the helper. As it is now \nthe helper leaves no guarantees of the order of the keys so we are not \ncommitting to anything (yet).\n\n> \n> --\n> Kieran\n> \n> \n> \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n> >  1 file changed, 3 insertions(+), 16 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index aa1459fb3599283b..7941c6845cbc9a9a 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> >  \t * utils::set_overlap requires the ranges to be sorted, keep the\n> >  \t * cio2Codes vector sorted in ascending order.\n> >  \t */\n> > -\tstd::vector<unsigned int> cio2Codes;\n> > -\tcio2Codes.reserve(mbusCodesToInfo.size());\n> > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > -\t\t       std::back_inserter(cio2Codes),\n> > -\t\t       [](auto &pair) { return pair.first; });\n> > +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n> >  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n> >  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n> >  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n> > @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> >  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n> >  \t * the CIO2 output device.\n> >  \t */\n> > -\tstd::vector<unsigned int> mbusCodes;\n> > -\tmbusCodes.reserve(mbusCodesToInfo.size());\n> > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > -\t\t       std::back_inserter(mbusCodes),\n> > -\t\t       [](auto &pair) { return pair.first; });\n> > -\n> > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> >  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n> >  \tret = sensor_->setFormat(&sensorFormat);\n> >  \tif (ret)\n> > @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n> >  \t\tsize = sensor_->resolution();\n> >  \n> >  \t/* Query the sensor static information for closest match. */\n> > -\tstd::vector<unsigned int> mbusCodes;\n> > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > -\t\t       std::back_inserter(mbusCodes),\n> > -\t\t       [](auto &pair) { return pair.first; });\n> > -\n> > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> >  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n> >  \tif (!sensorFormat.mbus_code) {\n> >  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DAA18BE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  1 Jul 2020 21:56:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7323C60C57;\n\tWed,  1 Jul 2020 23:56:29 +0200 (CEST)","from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4088F609A9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  1 Jul 2020 23:56:28 +0200 (CEST)","by mail-lf1-x144.google.com with SMTP id t9so14606564lfl.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 01 Jul 2020 14:56:28 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tk20sm2232672ljc.111.2020.07.01.14.56.26\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 01 Jul 2020 14:56:26 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"EkZdNIfL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=/aCqDf7Y3RawiTGBrZRrOtux7eKOY29yP+27cTOHRgQ=;\n\tb=EkZdNIfLc2ZVgD03VOJGHCRAn0KGtKexkH6YsMgTXdGv2cHfLQqEayrbVPsa4LF2tr\n\tcZZ14pBnbBePje3pojYI6fBiNXFCc0Y7GDT6aunBlkejQBelFEQQKFNpOrsOvI9XWw4P\n\tjFJC2bnBvKT9IkkkZw+kQ2X5pjVC/coTxnt1YCoPuuc7zkKYnxcG1HljV9FvceYRnDIz\n\tP7hcxK4/fG5UiDio7NM8gt6vQ8ZQsXHX4arAG6xBuJ+Rok8mcska4ejz08D8Vco2JkVV\n\tup5SFtdFz8aNwU7TmxKtC1+6cyP6IC0N/U+I36lf4bRURounZi7QfXXYSxqhUMqiXkXs\n\tTgbw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=/aCqDf7Y3RawiTGBrZRrOtux7eKOY29yP+27cTOHRgQ=;\n\tb=U9amrl7eSSAqFIQmtiTrPDdOpMCmw6dtvjLPG3X8EY+1ZaoPsLI1B8UgriOgPSS7C+\n\tYCtJSZ0NHMhnYXrx/kQsVzLXrWj6FMr7E5MC6rQ9BkdkxvOJNHhc0/TWg1IRuAk/3tMp\n\twt6b/W2WnCxTD2cHAP9mReTL6rX6mqOuZP88HVPTakwUtc96leYHrrEmqzQTSIMEj5U6\n\tbctAwHIaBOy+hivyyXo7KwwVfh43XxNEhslBQ94fJYGI8eCRhIC2UBqN1Z0b01F2eqmu\n\tiKx4yPQfs4kyianefMGYAou1s5WYFBaFSRZl7EpmlzMn0jx6WekGhHa6wbTOC5iJiqpM\n\tka7A==","X-Gm-Message-State":"AOAM532YEdubnSLkyfgMGo48J8IT6k6KfswzPSarO6n+KCqbMvSvu2Zs\n\tIQYFZNYyGrMKv3FnUoc0wxKASA==","X-Google-Smtp-Source":"ABdhPJy0vAFhSN88gVCHbmng1qg+33fROwyCYo7cwtE6+I+ofiHrEJUZmPx/Ie4zCik9Jg5vbEhjrw==","X-Received":"by 2002:a19:8497:: with SMTP id\n\tg145mr16231963lfd.73.1593640587317; \n\tWed, 01 Jul 2020 14:56:27 -0700 (PDT)","Date":"Wed, 1 Jul 2020 23:56:25 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200701215625.GB2703142@oden.dyn.berto.se>","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>\n\t<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11070,"web_url":"https://patchwork.libcamera.org/comment/11070/","msgid":"<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>","date":"2020-07-02T08:07:44","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Wed, Jul 01, 2020 at 10:42:38PM +0100, Kieran Bingham wrote:\n> Hi Niklas,\n>\n> On 01/07/2020 22:09, Niklas Söderlund wrote:\n> > Use a helper instead of local code to retrieve all keys from a map.\n> >\n>\n> I see this is the reason for changing Laurent's patch to return a\n> vector, but couldn't this patch update things to use sets locally?\n>\n> The keys are 'unique' right? Is there a distinct benefit for returning a\n> vector over a set?\n>\n> Perhaps there is a performance improvement with a vector if it doesn't\n> need to ensure uniqueness? But I'd be surprised if it was much... but I\n> don't know.\n>\n> Or is it to prevent updating the functions that the set (vector) is then\n> passed to, i.e. sensor_->getFormat() ?\n\nI'll try to summarize my understanding of the discussion.\n\nWhen considered in isolation from the context, yes, this function\nshould return an std::set: keys in a map are sorted and unique and\nstd::set makes that explicit so the user doesn't have to inspect what\nis returned (if it's a vector) to make sure about this.\n\nWhen this is applied to the context of enumerating image formats (like\nin the series Niklas has posted yesterday), I think introducing\nstd::set there will very quickly spread to CameraSensor and V4L2\nvideo (sub)device and I would be less confortable with that. Mostly\nbecause, when we use this for enumerating mbus or fourcc codes, we\nalready have a guarantee that\n1) the codes are unique, otherwise is a driver bug\n2) the vector is generated iterating the map's key, and the resulting\nsorting order will be the same.\n\nUsing std::set you pay a performance price, as the container needs to be\nkept sorted, and possibly a small penalty in space occupation as well,\nas sets are usually implemented using trees instead of being a simpler\ncontigous chunk of allocated space as vectors. That said, we're\nworking with number of item where all these considerations are moot,\nwe have very few elements, so I would consider std::set and\nstd::vector to be equally performant. My main concern is that std::set\nwould soon take over all our APIs and we'll find ourselves to have at\nsome point to convert between set and vectors, which I would really\nnot like to.\n\nIt's a shame we can't overload on return value :(\n\n>\n> --\n> Kieran\n>\n>\n>\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n> >  1 file changed, 3 insertions(+), 16 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index aa1459fb3599283b..7941c6845cbc9a9a 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> >  \t * utils::set_overlap requires the ranges to be sorted, keep the\n> >  \t * cio2Codes vector sorted in ascending order.\n> >  \t */\n> > -\tstd::vector<unsigned int> cio2Codes;\n> > -\tcio2Codes.reserve(mbusCodesToInfo.size());\n> > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > -\t\t       std::back_inserter(cio2Codes),\n> > -\t\t       [](auto &pair) { return pair.first; });\n> > +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n> >  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n> >  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n> >  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n> > @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> >  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n> >  \t * the CIO2 output device.\n> >  \t */\n> > -\tstd::vector<unsigned int> mbusCodes;\n> > -\tmbusCodes.reserve(mbusCodesToInfo.size());\n> > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > -\t\t       std::back_inserter(mbusCodes),\n> > -\t\t       [](auto &pair) { return pair.first; });\n> > -\n> > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> >  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n> >  \tret = sensor_->setFormat(&sensorFormat);\n> >  \tif (ret)\n> > @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n> >  \t\tsize = sensor_->resolution();\n> >\n> >  \t/* Query the sensor static information for closest match. */\n> > -\tstd::vector<unsigned int> mbusCodes;\n> > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > -\t\t       std::back_inserter(mbusCodes),\n> > -\t\t       [](auto &pair) { return pair.first; });\n> > -\n> > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> >  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n> >  \tif (!sensorFormat.mbus_code) {\n> >  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";\n> >\n>\n> --\n> Regards\n> --\n> Kieran\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 08633BFFE2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  2 Jul 2020 08:04:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AE8660C56;\n\tThu,  2 Jul 2020 10:04:13 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E68F4603AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Jul 2020 10:04:12 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id D18F920002;\n\tThu,  2 Jul 2020 08:04:11 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 2 Jul 2020 10:07:44 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>\n\t<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11072,"web_url":"https://patchwork.libcamera.org/comment/11072/","msgid":"<67bfd24e-a731-ee09-5c94-391037b1ce09@ideasonboard.com>","date":"2020-07-02T08:55:46","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 02/07/2020 09:07, Jacopo Mondi wrote:\n> Hi Kieran\n> \n> On Wed, Jul 01, 2020 at 10:42:38PM +0100, Kieran Bingham wrote:\n>> Hi Niklas,\n>>\n>> On 01/07/2020 22:09, Niklas Söderlund wrote:\n>>> Use a helper instead of local code to retrieve all keys from a map.\n>>>\n>>\n>> I see this is the reason for changing Laurent's patch to return a\n>> vector, but couldn't this patch update things to use sets locally?\n>>\n>> The keys are 'unique' right? Is there a distinct benefit for returning a\n>> vector over a set?\n>>\n>> Perhaps there is a performance improvement with a vector if it doesn't\n>> need to ensure uniqueness? But I'd be surprised if it was much... but I\n>> don't know.\n>>\n>> Or is it to prevent updating the functions that the set (vector) is then\n>> passed to, i.e. sensor_->getFormat() ?\n> \n> I'll try to summarize my understanding of the discussion.\n> \n> When considered in isolation from the context, yes, this function\n> should return an std::set: keys in a map are sorted and unique and\n> std::set makes that explicit so the user doesn't have to inspect what\n> is returned (if it's a vector) to make sure about this.\n> \n> When this is applied to the context of enumerating image formats (like\n> in the series Niklas has posted yesterday), I think introducing\n> std::set there will very quickly spread to CameraSensor and V4L2\n> video (sub)device and I would be less confortable with that. Mostly\n> because, when we use this for enumerating mbus or fourcc codes, we\n> already have a guarantee that\n> 1) the codes are unique, otherwise is a driver bug\n> 2) the vector is generated iterating the map's key, and the resulting\n> sorting order will be the same.\n> \n> Using std::set you pay a performance price, as the container needs to be\n> kept sorted, and possibly a small penalty in space occupation as well,\n> as sets are usually implemented using trees instead of being a simpler\n> contigous chunk of allocated space as vectors. That said, we're\n> working with number of item where all these considerations are moot,\n> we have very few elements, so I would consider std::set and\n> std::vector to be equally performant. My main concern is that std::set\n> would soon take over all our APIs and we'll find ourselves to have at\n> some point to convert between set and vectors, which I would really\n> not like to.\n\nThanks, that's a lot clearer.\n\nI wonder if we should capture some of that in the commit message at 1/2 ...\n\n\n> It's a shame we can't overload on return value :(\n\nYes, I guess being able to return a set or a vector could also make\n'every one happy' ... but I'm not overly concerned. A vector should be fine.\n\nThanks for the description.\n\nKieran.\n\n\n>> --\n>> Kieran\n>>\n>>\n>>\n>>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>>> ---\n>>>  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n>>>  1 file changed, 3 insertions(+), 16 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> index aa1459fb3599283b..7941c6845cbc9a9a 100644\n>>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n>>>  \t * utils::set_overlap requires the ranges to be sorted, keep the\n>>>  \t * cio2Codes vector sorted in ascending order.\n>>>  \t */\n>>> -\tstd::vector<unsigned int> cio2Codes;\n>>> -\tcio2Codes.reserve(mbusCodesToInfo.size());\n>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>> -\t\t       std::back_inserter(cio2Codes),\n>>> -\t\t       [](auto &pair) { return pair.first; });\n>>> +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n>>>  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n>>>  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n>>>  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n>>> @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n>>>  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n>>>  \t * the CIO2 output device.\n>>>  \t */\n>>> -\tstd::vector<unsigned int> mbusCodes;\n>>> -\tmbusCodes.reserve(mbusCodesToInfo.size());\n>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>> -\t\t       std::back_inserter(mbusCodes),\n>>> -\t\t       [](auto &pair) { return pair.first; });\n>>> -\n>>> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>>>  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n>>>  \tret = sensor_->setFormat(&sensorFormat);\n>>>  \tif (ret)\n>>> @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n>>>  \t\tsize = sensor_->resolution();\n>>>\n>>>  \t/* Query the sensor static information for closest match. */\n>>> -\tstd::vector<unsigned int> mbusCodes;\n>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>> -\t\t       std::back_inserter(mbusCodes),\n>>> -\t\t       [](auto &pair) { return pair.first; });\n>>> -\n>>> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>>>  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n>>>  \tif (!sensorFormat.mbus_code) {\n>>>  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AA203BFFE2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  2 Jul 2020 08:55:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4796560C56;\n\tThu,  2 Jul 2020 10:55:52 +0200 (CEST)","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 4A5A4603AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Jul 2020 10:55:51 +0200 (CEST)","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 05A93734;\n\tThu,  2 Jul 2020 10:55:49 +0200 (CEST)"],"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=\"hEY8D0dp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593680150;\n\tbh=S5qkrdIqpFxcQ5ycaE9OhEYsdiWqaQicd1bcwqPK7EI=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=hEY8D0dp64GnURjKjeimooc/ghw7DNrHzwdltxcHBbudlrP0yP4VTZVgsyjsD4+r2\n\ttBEdXGH9JhYRYdCQU/Y0r5n/Yb625ZL0Hj1FVPsM/+OKCEj3GJuL004mCaE0el4zMD\n\tIAaP5hCapq/E0tcVzEWCQH6gikCvKBCYCblWeKTo=","To":"Jacopo Mondi <jacopo@jmondi.org>","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>\n\t<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>\n\t<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<67bfd24e-a731-ee09-5c94-391037b1ce09@ideasonboard.com>","Date":"Thu, 2 Jul 2020 09:55:46 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11073,"web_url":"https://patchwork.libcamera.org/comment/11073/","msgid":"<eb9c0b40-9143-fc59-a9e4-678dac71d16a@ideasonboard.com>","date":"2020-07-02T08:56:39","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 01/07/2020 22:56, Niklas Söderlund wrote:\n> Hi Kieran,\n> \n> Thanks for your feedback.\n> \n> On 2020-07-01 22:42:38 +0100, Kieran Bingham wrote:\n>> Hi Niklas,\n>>\n>> On 01/07/2020 22:09, Niklas Söderlund wrote:\n>>> Use a helper instead of local code to retrieve all keys from a map.\n>>>\n>>\n>> I see this is the reason for changing Laurent's patch to return a\n>> vector, but couldn't this patch update things to use sets locally?\n>>\n>> The keys are 'unique' right? Is there a distinct benefit for returning a\n>> vector over a set?\n>>\n>> Perhaps there is a performance improvement with a vector if it doesn't\n>> need to ensure uniqueness? But I'd be surprised if it was much... but I\n>> don't know.\n>>\n>> Or is it to prevent updating the functions that the set (vector) is then\n>> passed to, i.e. sensor_->getFormat() ?\n> \n> I would prefer it to be a std::set, but par the discussion around v1 of \n> this patch and the last two versions of the series that introduce \n> CIO2Devcice (which is now merged) it seemed the quickest way to avoid \n> bikeshedding making it a std::vector.\n> \n> We can always later switch the container to a std::set if it becomes \n> less trouble then remember to call std::sort() where it mattes, or for \n> that matter start with calling std::sort() in the helper. As it is now \n> the helper leaves no guarantees of the order of the keys so we are not \n> committing to anything (yet).\n> \n\n\nIt seems I've read Jacopo's response first, so now I have a better\nunderstanding of what is needed.\n\nNo objections to returning / using a vector as it simplifies things ;-)\n\n--\nKieran\n\n\n\n\n>>\n>> --\n>> Kieran\n>>\n>>\n>>\n>>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>>> ---\n>>>  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n>>>  1 file changed, 3 insertions(+), 16 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> index aa1459fb3599283b..7941c6845cbc9a9a 100644\n>>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>> @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n>>>  \t * utils::set_overlap requires the ranges to be sorted, keep the\n>>>  \t * cio2Codes vector sorted in ascending order.\n>>>  \t */\n>>> -\tstd::vector<unsigned int> cio2Codes;\n>>> -\tcio2Codes.reserve(mbusCodesToInfo.size());\n>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>> -\t\t       std::back_inserter(cio2Codes),\n>>> -\t\t       [](auto &pair) { return pair.first; });\n>>> +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n>>>  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n>>>  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n>>>  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n>>> @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n>>>  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n>>>  \t * the CIO2 output device.\n>>>  \t */\n>>> -\tstd::vector<unsigned int> mbusCodes;\n>>> -\tmbusCodes.reserve(mbusCodesToInfo.size());\n>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>> -\t\t       std::back_inserter(mbusCodes),\n>>> -\t\t       [](auto &pair) { return pair.first; });\n>>> -\n>>> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>>>  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n>>>  \tret = sensor_->setFormat(&sensorFormat);\n>>>  \tif (ret)\n>>> @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n>>>  \t\tsize = sensor_->resolution();\n>>>  \n>>>  \t/* Query the sensor static information for closest match. */\n>>> -\tstd::vector<unsigned int> mbusCodes;\n>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>> -\t\t       std::back_inserter(mbusCodes),\n>>> -\t\t       [](auto &pair) { return pair.first; });\n>>> -\n>>> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>>>  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n>>>  \tif (!sensorFormat.mbus_code) {\n>>>  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";\n>>>\n>>\n>> -- \n>> Regards\n>> --\n>> Kieran\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E07C7BFFE2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  2 Jul 2020 08:56:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC1AF60C56;\n\tThu,  2 Jul 2020 10:56:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 25C6A603AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Jul 2020 10:56:44 +0200 (CEST)","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 A9453734;\n\tThu,  2 Jul 2020 10:56:43 +0200 (CEST)"],"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=\"VUbkEbFi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593680203;\n\tbh=um1+/Fm0OdwCXwGsMgfmty+SiR/b60KK/4BYQLkFvos=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=VUbkEbFiG/jaqaRKewSaHIxP8r/2Cl2qGD0I5HA3hXNn/3redPI2MuyPcqd/TMXQr\n\tkt4Mg2fLuwd091CXK7aS6D13nM0kAJLegTOtfvaRdTiolGIjTdzURxTc+vitpyuh6M\n\tlkrtxzN4ZHyAhKhl9rt9EHMhUX/f+8+s7q95/eCo=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>\n\t<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>\n\t<20200701215625.GB2703142@oden.dyn.berto.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<eb9c0b40-9143-fc59-a9e4-678dac71d16a@ideasonboard.com>","Date":"Thu, 2 Jul 2020 09:56:39 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200701215625.GB2703142@oden.dyn.berto.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11074,"web_url":"https://patchwork.libcamera.org/comment/11074/","msgid":"<20200702085918.GC5853@pendragon.ideasonboard.com>","date":"2020-07-02T08:59:18","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello everybody,\n\nOn Thu, Jul 02, 2020 at 10:07:44AM +0200, Jacopo Mondi wrote:\n> On Wed, Jul 01, 2020 at 10:42:38PM +0100, Kieran Bingham wrote:\n> > On 01/07/2020 22:09, Niklas Söderlund wrote:\n> > > Use a helper instead of local code to retrieve all keys from a map.\n> > >\n> >\n> > I see this is the reason for changing Laurent's patch to return a\n> > vector, but couldn't this patch update things to use sets locally?\n> >\n> > The keys are 'unique' right? Is there a distinct benefit for returning a\n> > vector over a set?\n> >\n> > Perhaps there is a performance improvement with a vector if it doesn't\n> > need to ensure uniqueness? But I'd be surprised if it was much... but I\n> > don't know.\n> >\n> > Or is it to prevent updating the functions that the set (vector) is then\n> > passed to, i.e. sensor_->getFormat() ?\n> \n> I'll try to summarize my understanding of the discussion.\n> \n> When considered in isolation from the context, yes, this function\n> should return an std::set: keys in a map are sorted and unique and\n> std::set makes that explicit so the user doesn't have to inspect what\n> is returned (if it's a vector) to make sure about this.\n> \n> When this is applied to the context of enumerating image formats (like\n> in the series Niklas has posted yesterday), I think introducing\n> std::set there will very quickly spread to CameraSensor and V4L2\n> video (sub)device and I would be less confortable with that. Mostly\n> because, when we use this for enumerating mbus or fourcc codes, we\n> already have a guarantee that\n> 1) the codes are unique, otherwise is a driver bug\n> 2) the vector is generated iterating the map's key, and the resulting\n> sorting order will be the same.\n> \n> Using std::set you pay a performance price, as the container needs to be\n> kept sorted, and possibly a small penalty in space occupation as well,\n> as sets are usually implemented using trees instead of being a simpler\n> contigous chunk of allocated space as vectors. That said, we're\n> working with number of item where all these considerations are moot,\n> we have very few elements, so I would consider std::set and\n> std::vector to be equally performant. My main concern is that std::set\n> would soon take over all our APIs and we'll find ourselves to have at\n> some point to convert between set and vectors, which I would really\n> not like to.\n> \n> It's a shame we can't overload on return value :(\n\nHow about starting with std::vector and seeing where it leads us ? It's\nnot like we'll set this in stone.\n\n> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > ---\n> > >  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n> > >  1 file changed, 3 insertions(+), 16 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > index aa1459fb3599283b..7941c6845cbc9a9a 100644\n> > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> > >  \t * utils::set_overlap requires the ranges to be sorted, keep the\n> > >  \t * cio2Codes vector sorted in ascending order.\n> > >  \t */\n> > > -\tstd::vector<unsigned int> cio2Codes;\n> > > -\tcio2Codes.reserve(mbusCodesToInfo.size());\n> > > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > > -\t\t       std::back_inserter(cio2Codes),\n> > > -\t\t       [](auto &pair) { return pair.first; });\n> > > +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n> > >  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n> > >  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n> > >  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n> > > @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > >  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n> > >  \t * the CIO2 output device.\n> > >  \t */\n> > > -\tstd::vector<unsigned int> mbusCodes;\n> > > -\tmbusCodes.reserve(mbusCodesToInfo.size());\n> > > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > > -\t\t       std::back_inserter(mbusCodes),\n> > > -\t\t       [](auto &pair) { return pair.first; });\n> > > -\n> > > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> > >  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n> > >  \tret = sensor_->setFormat(&sensorFormat);\n> > >  \tif (ret)\n> > > @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n> > >  \t\tsize = sensor_->resolution();\n> > >\n> > >  \t/* Query the sensor static information for closest match. */\n> > > -\tstd::vector<unsigned int> mbusCodes;\n> > > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > > -\t\t       std::back_inserter(mbusCodes),\n> > > -\t\t       [](auto &pair) { return pair.first; });\n> > > -\n> > > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> > >  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n> > >  \tif (!sensorFormat.mbus_code) {\n> > >  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 96254BE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  2 Jul 2020 08:59:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3818460C53;\n\tThu,  2 Jul 2020 10:59:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED2C5603B4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Jul 2020 10:59:21 +0200 (CEST)","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 5E32D9CB;\n\tThu,  2 Jul 2020 10:59:21 +0200 (CEST)"],"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=\"LQBSiR8F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593680361;\n\tbh=+bTFpomeBCetwXNTaCfJVNglGrnxO86+9p39mKTLt0A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LQBSiR8FhEzWfBQZwC58Bxhqs7/bP4AYsBvEMh3wKqRGtq/TO04Oy9Tn5dk21gGDY\n\tiR0lHnpkgRQfqv7YInaSZqS9lAHt9bzex88t6CMgYlA+qRFeDZErLSpnZ9OaG5sxCB\n\t0JssQeacKSEklt1dHLcVtyr+qndkwtVm5zdyyD2c=","Date":"Thu, 2 Jul 2020 11:59:18 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200702085918.GC5853@pendragon.ideasonboard.com>","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>\n\t<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>\n\t<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11086,"web_url":"https://patchwork.libcamera.org/comment/11086/","msgid":"<20200702214704.GB3158543@oden.dyn.berto.se>","date":"2020-07-02T21:47:04","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello,\n\nOn 2020-07-02 11:59:18 +0300, Laurent Pinchart wrote:\n> Hello everybody,\n> \n> On Thu, Jul 02, 2020 at 10:07:44AM +0200, Jacopo Mondi wrote:\n> > On Wed, Jul 01, 2020 at 10:42:38PM +0100, Kieran Bingham wrote:\n> > > On 01/07/2020 22:09, Niklas Söderlund wrote:\n> > > > Use a helper instead of local code to retrieve all keys from a map.\n> > > >\n> > >\n> > > I see this is the reason for changing Laurent's patch to return a\n> > > vector, but couldn't this patch update things to use sets locally?\n> > >\n> > > The keys are 'unique' right? Is there a distinct benefit for returning a\n> > > vector over a set?\n> > >\n> > > Perhaps there is a performance improvement with a vector if it doesn't\n> > > need to ensure uniqueness? But I'd be surprised if it was much... but I\n> > > don't know.\n> > >\n> > > Or is it to prevent updating the functions that the set (vector) is then\n> > > passed to, i.e. sensor_->getFormat() ?\n> > \n> > I'll try to summarize my understanding of the discussion.\n> > \n> > When considered in isolation from the context, yes, this function\n> > should return an std::set: keys in a map are sorted and unique and\n> > std::set makes that explicit so the user doesn't have to inspect what\n> > is returned (if it's a vector) to make sure about this.\n> > \n> > When this is applied to the context of enumerating image formats (like\n> > in the series Niklas has posted yesterday), I think introducing\n> > std::set there will very quickly spread to CameraSensor and V4L2\n> > video (sub)device and I would be less confortable with that. Mostly\n> > because, when we use this for enumerating mbus or fourcc codes, we\n> > already have a guarantee that\n> > 1) the codes are unique, otherwise is a driver bug\n> > 2) the vector is generated iterating the map's key, and the resulting\n> > sorting order will be the same.\n> > \n> > Using std::set you pay a performance price, as the container needs to be\n> > kept sorted, and possibly a small penalty in space occupation as well,\n> > as sets are usually implemented using trees instead of being a simpler\n> > contigous chunk of allocated space as vectors. That said, we're\n> > working with number of item where all these considerations are moot,\n> > we have very few elements, so I would consider std::set and\n> > std::vector to be equally performant. My main concern is that std::set\n> > would soon take over all our APIs and we'll find ourselves to have at\n> > some point to convert between set and vectors, which I would really\n> > not like to.\n> > \n> > It's a shame we can't overload on return value :(\n> \n> How about starting with std::vector and seeing where it leads us ? It's\n> not like we'll set this in stone.\n\nI like this, if we can scrape together some R-b we can take this to the \nnext level :-)\n\n> \n> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > > ---\n> > > >  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n> > > >  1 file changed, 3 insertions(+), 16 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > index aa1459fb3599283b..7941c6845cbc9a9a 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > > > @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n> > > >  \t * utils::set_overlap requires the ranges to be sorted, keep the\n> > > >  \t * cio2Codes vector sorted in ascending order.\n> > > >  \t */\n> > > > -\tstd::vector<unsigned int> cio2Codes;\n> > > > -\tcio2Codes.reserve(mbusCodesToInfo.size());\n> > > > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > > > -\t\t       std::back_inserter(cio2Codes),\n> > > > -\t\t       [](auto &pair) { return pair.first; });\n> > > > +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n> > > >  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n> > > >  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n> > > >  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n> > > > @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n> > > >  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n> > > >  \t * the CIO2 output device.\n> > > >  \t */\n> > > > -\tstd::vector<unsigned int> mbusCodes;\n> > > > -\tmbusCodes.reserve(mbusCodesToInfo.size());\n> > > > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > > > -\t\t       std::back_inserter(mbusCodes),\n> > > > -\t\t       [](auto &pair) { return pair.first; });\n> > > > -\n> > > > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> > > >  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n> > > >  \tret = sensor_->setFormat(&sensorFormat);\n> > > >  \tif (ret)\n> > > > @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n> > > >  \t\tsize = sensor_->resolution();\n> > > >\n> > > >  \t/* Query the sensor static information for closest match. */\n> > > > -\tstd::vector<unsigned int> mbusCodes;\n> > > > -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n> > > > -\t\t       std::back_inserter(mbusCodes),\n> > > > -\t\t       [](auto &pair) { return pair.first; });\n> > > > -\n> > > > +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n> > > >  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n> > > >  \tif (!sensorFormat.mbus_code) {\n> > > >  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 8DA13BE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  2 Jul 2020 21:47:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26B8D609C7;\n\tThu,  2 Jul 2020 23:47:08 +0200 (CEST)","from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 63C45603B4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Jul 2020 23:47:06 +0200 (CEST)","by mail-lf1-x144.google.com with SMTP id c11so17115159lfh.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 02 Jul 2020 14:47:06 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tz7sm3414687ljj.33.2020.07.02.14.47.04\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 02 Jul 2020 14:47:04 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"HuZR0nqN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=rJ3FwDV80sN+2ji2RbCh/0BRnmRN3xMajcuF23x7XSY=;\n\tb=HuZR0nqNflhheN7M659+thwNk/KR+KHv1skh9c1yjZ8VCR0KGQZwU77MsLp/2XP2FK\n\t6nxc3HeUi7WGMZinRb+rQH9iAFLj/8PUXMN9SvFqmAni/K9S5YX2G20uWzdk1ib7decG\n\tHtvTqni5GKjAZKgNo73W2JbRxkwQv4BwDQM8Q0q2WUQZs2e6nKAhdcANYWhZXx0dYfkE\n\t3244y/5fu7Ltj2jL/1pMegsXyLl3Due3X13s0Y3n4qspNV2nOvqF8+o7hIJi7NFJJCVr\n\tTRn+IJbyRdiyUiX0LftGpv87iy+fdy73qtXB5O/bwHG5amGNXc05eGtjLvqykZIGe3xX\n\t8Tcw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=rJ3FwDV80sN+2ji2RbCh/0BRnmRN3xMajcuF23x7XSY=;\n\tb=mVRoiWNzwx20StbK2KKyOKrExWU26aCmk/FkDhfqIcNMxSUbjl+mcGQ9l4bTVv6X6Q\n\t5wG2nznDXMlEAAUvFE1I5Y1RVR+EUwAVNIrfm1edLeHg4j+p7OEEkTAQhygW6YPhK0d5\n\tr817H5gQ1WrXVQQzOayVFa1SO20rgE6pd9NLkGpeiii/v21uM+KyyI1A4E1BWWeIQyg7\n\tDy6S6699q2EEtN25JzU4rzCD3ejsOh9vdtC8aAvxzxOUc2QF0BM4ZPsqR09gaenwUVyi\n\tlzoMi4VuDmZqiowCgys7ODaHM3eg2Y8BDBOolbl5NqbKMbzZjoKTi0ax2DTc2eLlNK0u\n\tZwKQ==","X-Gm-Message-State":"AOAM533EaXlj4r/nsZXXueoCFaloTWmM/P5C7EUoNvkW5ilE0S0qBeav\n\t1Xnx57xXgMPqj6LWUxfq13FsqA==","X-Google-Smtp-Source":"ABdhPJy0k30ttbDptyoxxR2ulrvyGPrnY3RqAkHfStccFZqtUlPX+6b3D5bQcOg0P0oYQI1NH/gVHA==","X-Received":"by 2002:a19:e61a:: with SMTP id\n\td26mr14172583lfh.96.1593726425415; \n\tThu, 02 Jul 2020 14:47:05 -0700 (PDT)","Date":"Thu, 2 Jul 2020 23:47:04 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20200702214704.GB3158543@oden.dyn.berto.se>","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>\n\t<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>\n\t<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>\n\t<20200702085918.GC5853@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200702085918.GC5853@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11087,"web_url":"https://patchwork.libcamera.org/comment/11087/","msgid":"<eb40a85f-777f-4210-fbd1-d59bf25e0435@ideasonboard.com>","date":"2020-07-02T21:50:51","subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas, Laurent,\n\nOn 02/07/2020 22:47, Niklas Söderlund wrote:\n> Hello,\n> \n> On 2020-07-02 11:59:18 +0300, Laurent Pinchart wrote:\n>> Hello everybody,\n>>\n>> On Thu, Jul 02, 2020 at 10:07:44AM +0200, Jacopo Mondi wrote:\n>>> On Wed, Jul 01, 2020 at 10:42:38PM +0100, Kieran Bingham wrote:\n>>>> On 01/07/2020 22:09, Niklas Söderlund wrote:\n>>>>> Use a helper instead of local code to retrieve all keys from a map.\n>>>>>\n>>>>\n>>>> I see this is the reason for changing Laurent's patch to return a\n>>>> vector, but couldn't this patch update things to use sets locally?\n>>>>\n>>>> The keys are 'unique' right? Is there a distinct benefit for returning a\n>>>> vector over a set?\n>>>>\n>>>> Perhaps there is a performance improvement with a vector if it doesn't\n>>>> need to ensure uniqueness? But I'd be surprised if it was much... but I\n>>>> don't know.\n>>>>\n>>>> Or is it to prevent updating the functions that the set (vector) is then\n>>>> passed to, i.e. sensor_->getFormat() ?\n>>>\n>>> I'll try to summarize my understanding of the discussion.\n>>>\n>>> When considered in isolation from the context, yes, this function\n>>> should return an std::set: keys in a map are sorted and unique and\n>>> std::set makes that explicit so the user doesn't have to inspect what\n>>> is returned (if it's a vector) to make sure about this.\n>>>\n>>> When this is applied to the context of enumerating image formats (like\n>>> in the series Niklas has posted yesterday), I think introducing\n>>> std::set there will very quickly spread to CameraSensor and V4L2\n>>> video (sub)device and I would be less confortable with that. Mostly\n>>> because, when we use this for enumerating mbus or fourcc codes, we\n>>> already have a guarantee that\n>>> 1) the codes are unique, otherwise is a driver bug\n>>> 2) the vector is generated iterating the map's key, and the resulting\n>>> sorting order will be the same.\n>>>\n>>> Using std::set you pay a performance price, as the container needs to be\n>>> kept sorted, and possibly a small penalty in space occupation as well,\n>>> as sets are usually implemented using trees instead of being a simpler\n>>> contigous chunk of allocated space as vectors. That said, we're\n>>> working with number of item where all these considerations are moot,\n>>> we have very few elements, so I would consider std::set and\n>>> std::vector to be equally performant. My main concern is that std::set\n>>> would soon take over all our APIs and we'll find ourselves to have at\n>>> some point to convert between set and vectors, which I would really\n>>> not like to.\n>>>\n>>> It's a shame we can't overload on return value :(\n>>\n>> How about starting with std::vector and seeing where it leads us ? It's\n>> not like we'll set this in stone.\n\nYes, I think that was the agreement in the other mails ;-)\n\n> \n> I like this, if we can scrape together some R-b we can take this to the \n> next level :-)\n\nHa, ok well I suppose I could actually provide a tag too ...\n\nFor both patches in this series:\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n>>\n>>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>>>>> ---\n>>>>>  src/libcamera/pipeline/ipu3/cio2.cpp | 19 +++----------------\n>>>>>  1 file changed, 3 insertions(+), 16 deletions(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>>>> index aa1459fb3599283b..7941c6845cbc9a9a 100644\n>>>>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n>>>>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n>>>>> @@ -95,11 +95,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index)\n>>>>>  \t * utils::set_overlap requires the ranges to be sorted, keep the\n>>>>>  \t * cio2Codes vector sorted in ascending order.\n>>>>>  \t */\n>>>>> -\tstd::vector<unsigned int> cio2Codes;\n>>>>> -\tcio2Codes.reserve(mbusCodesToInfo.size());\n>>>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>>>> -\t\t       std::back_inserter(cio2Codes),\n>>>>> -\t\t       [](auto &pair) { return pair.first; });\n>>>>> +\tstd::vector<unsigned int> cio2Codes = utils::map_keys(mbusCodesToInfo);\n>>>>>  \tconst std::vector<unsigned int> &sensorCodes = sensor_->mbusCodes();\n>>>>>  \tif (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(),\n>>>>>  \t\t\t\tcio2Codes.begin(), cio2Codes.end())) {\n>>>>> @@ -145,12 +141,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat)\n>>>>>  \t * Apply the selected format to the sensor, the CSI-2 receiver and\n>>>>>  \t * the CIO2 output device.\n>>>>>  \t */\n>>>>> -\tstd::vector<unsigned int> mbusCodes;\n>>>>> -\tmbusCodes.reserve(mbusCodesToInfo.size());\n>>>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>>>> -\t\t       std::back_inserter(mbusCodes),\n>>>>> -\t\t       [](auto &pair) { return pair.first; });\n>>>>> -\n>>>>> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>>>>>  \tsensorFormat = sensor_->getFormat(mbusCodes, size);\n>>>>>  \tret = sensor_->setFormat(&sensorFormat);\n>>>>>  \tif (ret)\n>>>>> @@ -189,11 +180,7 @@ CIO2Device::generateConfiguration(Size size) const\n>>>>>  \t\tsize = sensor_->resolution();\n>>>>>\n>>>>>  \t/* Query the sensor static information for closest match. */\n>>>>> -\tstd::vector<unsigned int> mbusCodes;\n>>>>> -\tstd::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(),\n>>>>> -\t\t       std::back_inserter(mbusCodes),\n>>>>> -\t\t       [](auto &pair) { return pair.first; });\n>>>>> -\n>>>>> +\tstd::vector<unsigned int> mbusCodes = utils::map_keys(mbusCodesToInfo);\n>>>>>  \tV4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size);\n>>>>>  \tif (!sensorFormat.mbus_code) {\n>>>>>  \t\tLOG(IPU3, Error) << \"Sensor does not support mbus code\";\n>>\n>> -- \n>> Regards,\n>>\n>> Laurent Pinchart\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 451D7BFFE2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  2 Jul 2020 21:50:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BFB0060C57;\n\tThu,  2 Jul 2020 23:50:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 66C7F603B4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Jul 2020 23:50:55 +0200 (CEST)","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 B35C59CB;\n\tThu,  2 Jul 2020 23:50:54 +0200 (CEST)"],"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=\"rs0dD5D+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593726655;\n\tbh=pc+fsiFmIXyXnPk22CGH+TECTkR068kdhcVwI4FDC94=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=rs0dD5D+3G3CdUq5+xyvPwurXaOKFTertg8wv0/u06sGXjJXSR/lP+a35ol7wTs4U\n\tEGx6p7WZrGzFRzmB+kwJxTpQPcadNVkBm6YlLyrwAguJx4IYRufCf++lHCNdzkHfsh\n\tMp49b8KRW2ZWN8PVZNcDxNjj1aFpzjNerIb5dCGI=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20200701210948.999363-1-niklas.soderlund@ragnatech.se>\n\t<20200701210948.999363-3-niklas.soderlund@ragnatech.se>\n\t<791198f4-0949-41b1-8f1a-b55d97e0160b@ideasonboard.com>\n\t<20200702080744.4agq3rllzqbkrzj5@uno.localdomain>\n\t<20200702085918.GC5853@pendragon.ideasonboard.com>\n\t<20200702214704.GB3158543@oden.dyn.berto.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","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":"<eb40a85f-777f-4210-fbd1-d59bf25e0435@ideasonboard.com>","Date":"Thu, 2 Jul 2020 22:50:51 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200702214704.GB3158543@oden.dyn.berto.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: cio2: Make\n\tuse of utils::map_keys() helper","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>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]