[{"id":11857,"web_url":"https://patchwork.libcamera.org/comment/11857/","msgid":"<20200805004653.GX6075@pendragon.ideasonboard.com>","date":"2020-08-05T00:46:53","subject":"Re: [libcamera-devel] [PATCH v3 10/13] android: camera_device:\n\tReport configuration changes from validate()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Tue, Aug 04, 2020 at 10:47:08PM +0100, Kieran Bingham wrote:\n> When we call validate on a configuration, if there are any adjustments\n> on the configuration, we fail without showing why.\n> \n> Display the stream configuration after the validate stage to aid\n> debugging stream startup failures.\n> \n> Reviewed-by: Jacopo Mondi \n> Signed-off-by: Kieran Bingham \n> ---\n> src/android/camera_device.cpp | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 5899154b3e78..4178db952846 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1047,7 +1047,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> \tcase CameraConfiguration::Valid:\n> \t\tbreak;\n> \tcase CameraConfiguration::Adjusted:\n> -\t\tLOG(HAL, Info) << \"Camera configuration adjusted\";\n> +\t\tLOG(HAL, Info) << \"Camera configuration adjusted:\";\n> +\n> +\t\tfor (const StreamConfiguration &cfg : *config_)\n> +\t\t\tLOG(HAL, Info) << \" : \" << cfg.toString();\n\nThis would output\n\nCamera configuration ajusted:\n : 1920x1080-NV12\n : 800x600-YUYV\n\nThe leading colons seem a bit weird to me, are they intended ? Would\n\nCamera configuration ajusted:\n- 1920x1080-NV12\n- 800x600-YUYV\n\nbe better ?\n\nYou could also use the Debug level to log the stream configurations (but\nas we're not supposed to fail, Info could be fine), in which case I'd\nremove the colon as the end of the first line.\n\nWith or without any of those changes,\n\nReviewed-by: Laurent Pinchart \n\n> +\n> \t\tconfig_.reset();\n> \t\treturn -EINVAL;\n> \tcase CameraConfiguration::Invalid:","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 95D15BD86F\n\tfor ;\n\tWed, 5 Aug 2020 00:47:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C2BE60547;\n\tWed, 5 Aug 2020 02:47:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3395A60536\n\tfor ;\n\tWed, 5 Aug 2020 02:47:06 +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 9920227B;\n\tWed, 5 Aug 2020 02:47:05 +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=\"C4AypoS+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596588425;\n\tbh=WWesXPJN4Dv1/zUprBEWXwE6ZGzWZnrGQdFli1/klPg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=C4AypoS+w4jbrNhlphdtEE2vN8eY+CXwg0HG2mNZraDeE7nTq9M0EsjY8HrnKi0s9\n\tXOkqqLhtzoR6bXoQhs7fTvfwJvlNm6Y3qsfgePzrMMxJ/i++5fe0Owwya3nYdXs5NM\n\tFcSXq8qO2A1kHFAOquYD0D/B2Fha56C41zlcS9jA=","Date":"Wed, 5 Aug 2020 03:46:53 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20200805004653.GX6075@pendragon.ideasonboard.com>","References":"<20200804214711.177645-1-kieran.bingham@ideasonboard.com>\n\t<20200804214711.177645-11-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200804214711.177645-11-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 10/13] android: camera_device:\n\tReport configuration changes from validate()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11877,"web_url":"https://patchwork.libcamera.org/comment/11877/","msgid":"","date":"2020-08-05T12:05:04","subject":"Re: [libcamera-devel] [PATCH v3 10/13] android: camera_device:\n\tReport configuration changes from validate()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 05/08/2020 01:46, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Tue, Aug 04, 2020 at 10:47:08PM +0100, Kieran Bingham wrote:\n>> When we call validate on a configuration, if there are any adjustments\n>> on the configuration, we fail without showing why.\n>>\n>> Display the stream configuration after the validate stage to aid\n>> debugging stream startup failures.\n>>\n>> Reviewed-by: Jacopo Mondi \n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/android/camera_device.cpp | 6 +++++-\n>> 1 file changed, 5 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index 5899154b3e78..4178db952846 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -1047,7 +1047,11 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>> \tcase CameraConfiguration::Valid:\n>> \t\tbreak;\n>> \tcase CameraConfiguration::Adjusted:\n>> -\t\tLOG(HAL, Info) << \"Camera configuration adjusted\";\n>> +\t\tLOG(HAL, Info) << \"Camera configuration adjusted:\";\n>> +\n>> +\t\tfor (const StreamConfiguration &cfg : *config_)\n>> +\t\t\tLOG(HAL, Info) << \" : \" << cfg.toString();\n> \n> This would output\n> \n> Camera configuration ajusted:\n> : 1920x1080-NV12\n> : 800x600-YUYV\n> \n> The leading colons seem a bit weird to me, are they intended ? Would\n\nThe colons are a leftover from when I was able to display the associated\nandroid stream index before the stream configuration.\n\nI removed that because I now iterate the StreamConfigurations directly.\n\nA hyphen makes more sense yes.\n\n\n> \n> Camera configuration ajusted:\n> - 1920x1080-NV12\n> - 800x600-YUYV\n> \n> be better ?\n> \n> You could also use the Debug level to log the stream configurations (but\n> as we're not supposed to fail, Info could be fine), in which case I'd\n> remove the colon as the end of the first line.\n\nI added the colon at the end because it indicates that the lines\nfollowing are a continuation of this statement.\n\nIn an earlier review Jacopo also suggested making these Debug, but I\nwould almost prefer to go the other way, and make all three lines 'Error'...\n\nIf the camera configuration is adjusted, then the android configuration\nis going to fail, and it would be really helpful to know why ;-)\n\n\n> With or without any of those changes,\n> \n> Reviewed-by: Laurent Pinchart \n> \n>> +\n>> \t\tconfig_.reset();\n>> \t\treturn -EINVAL;\n>> \tcase CameraConfiguration::Invalid:\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 24C63BD87A\n\tfor ;\n\tWed, 5 Aug 2020 12:05:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 967D660564;\n\tWed, 5 Aug 2020 14:05:09 +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 B9F3360492\n\tfor ;\n\tWed, 5 Aug 2020 14:05:07 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 942942C0;\n\tWed, 5 Aug 2020 14:05:06 +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=\"J+0prFKN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596629106;\n\tbh=D1kJwmF0UY7YVL+XBoP0/oXcbWBv2xpfZbXkAJTyP5k=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=J+0prFKN0K6w5HWm1tMKB8hcMnf2j6z/FTb7KAVHK3B0JumYvQVLtDY3rr7E3O2+q\n\tLZHRQam1iLxSdga2XTnyQQfFkZ0RWuGzKBFVO0gOCnyHDKqUWVeSxiKZlTKLepXK+5\n\tOkl6awsB/BbWFYs7dKUsB8uX7asH/a0t1bdFIX5s=","To":"Laurent Pinchart ","References":"<20200804214711.177645-1-kieran.bingham@ideasonboard.com>\n\t<20200804214711.177645-11-kieran.bingham@ideasonboard.com>\n\t<20200805004653.GX6075@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Wed, 5 Aug 2020 13:05:04 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200805004653.GX6075@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 10/13] android: camera_device:\n\tReport configuration changes from validate()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]