[{"id":24379,"web_url":"https://patchwork.libcamera.org/comment/24379/","msgid":"","date":"2022-08-04T17:44:42","subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Use std::max()\n\tinstead of expandTo() to get the max resolution","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Han-Lin,\n\nThank you for the patch.\n\nOn Thu, Aug 04, 2022 at 08:06:20PM +0800, Han-Lin Chen via libcamera-devel wrote:\n> Using Size::expandTo() to find the max resolution might generate a non-existent\n> resolution. For example, when application request streams for 1920x1080 and\n> 1600x1200, the max resolution will be wrongly 1920x1200 and fails the\n> configuration.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=139\n> Signed-off-by: Han-Lin Chen \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 75231156..335b6c94 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -254,7 +254,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \t\t\trawSize.expandTo(cfg.size);\n\nDoesn't this need to be addressed too ?\n\n> \t\t} else {\n> \t\t\tyuvCount++;\n> -\t\t\tmaxYuvSize.expandTo(cfg.size);\n> +\t\t\tmaxYuvSize = std::max(maxYuvSize, cfg.size);\n\nIs this enough though ? In the example above, if the user requests two\nstreams, in 1920x1080 and 1600x1200 resolutions, shouldn't we pick a\nsize from the sensor that is larger than both, and then crop at the\noutput ? Looking at the rest of the validate() function, the size\nselection seems quite fragile.\n\n> \t\t}\n> \t}\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 432FCC3272\n\tfor ;\n\tThu, 4 Aug 2022 17:44:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A16186332B;\n\tThu, 4 Aug 2022 19:44:51 +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 B135A6330D\n\tfor ;\n\tThu, 4 Aug 2022 19:44:49 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 23F50481;\n\tThu, 4 Aug 2022 19:44:49 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659635091;\n\tbh=ws6uRAdUu5F5Bop909i2PN4mr1AcpYydXrnDuBhCfN4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=OMn4l7I2ez72idBD5XZDj3Zr71RqlIxCjSA5NuUPgwjwHK4uQjsP4+k2m3Ns2vo6b\n\tEue2bKjRF0lhXZ8xW4n8Ui/9rUta7tDQiQD5WPyLtt8P2HJ42UjygL8k0UVJpo/abE\n\t768CDdUfJf4ADbb9K5939r3dqtvsHZ1Luxp5396+RKAQtqEPmy2zoc6HzA9/pYUEPO\n\tqGbiEmP+lk1SfYsuVDzlc/aMKZttm3NpBIcCYP9CkWfmI6SvpW/PE5bydO4EXu5DUE\n\tRmmwKehnA679/9Gh/0tnHAPmPvPwNdv23DewkU7tWMLHjuTnHXg9V+lqCuMd/UjEdS\n\tgQF0m1PPe4gLw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659635089;\n\tbh=ws6uRAdUu5F5Bop909i2PN4mr1AcpYydXrnDuBhCfN4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H6NszC7L53UZ4IruNBqgjra5y7imY8dtJbaPBOEpl1Gj8xzCqit3MMPsmg1wL0fum\n\tvA4BJXwCbR1J3LNGiYHhEjRUzKXoTdlhpaRuSuMHRTnlsKEWi4v4cYTaMIEmUgxWeo\n\tlwiTZe5glclZQ/sWBMbINJpfh4ndjcrLv7lQFBVk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"H6NszC7L\"; dkim-atps=neutral","Date":"Thu, 4 Aug 2022 20:44:42 +0300","To":"Han-Lin Chen ","Message-ID":"","References":"<20220804120620.1696350-1-hanlinchen@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220804120620.1696350-1-hanlinchen@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: ipu3: Use std::max()\n\tinstead of expandTo() to get the max resolution","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]