[{"id":29173,"web_url":"https://patchwork.libcamera.org/comment/29173/","msgid":"<20240407010248.GF23422@pendragon.ideasonboard.com>","date":"2024-04-07T01:02:48","subject":"Re: [PATCH v7 01/18] libcamera: pipeline: simple: fix size\n\tadjustment in validate()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Apr 04, 2024 at 10:46:38AM +0200, Milan Zamazal wrote:\n> From: Andrey Konovalov \n> \n> SimpleCameraConfiguration::validate() adjusts the configuration of its\n> streams (if the size is not in the outputSizes) to the captureSize. But\n> the captureSize itself can be not in the outputSizes, and then the\n> adjusted configuration won't be valid resulting in camera configuration\n> failure.\n> \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> Reviewed-by: Milan Zamazal \n> Reviewed-by: Pavel Machek \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Andrey Konovalov \n> Signed-off-by: Hans de Goede \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++--\n> 1 file changed, 38 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 01f2a977..d2b88795 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -882,6 +882,33 @@ SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,\n> {\n> }\n> \n> +namespace {\n> +\n> +static Size adjustSize(const Size &requestedSize, const SizeRange &supportedSizes)\n> +{\n> +\tASSERT(supportedSizes.min <= supportedSizes.max);\n> +\n> +\tif (supportedSizes.min == supportedSizes.max)\n> +\t\treturn supportedSizes.max;\n> +\n> +\tunsigned int hStep = supportedSizes.hStep;\n> +\tunsigned int vStep = supportedSizes.vStep;\n> +\n> +\tif (hStep == 0)\n> +\t\thStep = supportedSizes.max.width - supportedSizes.min.width;\n> +\tif (vStep == 0)\n> +\t\tvStep = supportedSizes.max.height - supportedSizes.min.height;\n> +\n> +\tSize adjusted = requestedSize.boundedTo(supportedSizes.max)\n> +\t\t\t\t.expandedTo(supportedSizes.min);\n> +\n> +\treturn adjusted.shrunkBy(supportedSizes.min)\n> +\t\t.alignedDownTo(hStep, vStep)\n> +\t\t.grownBy(supportedSizes.min);\n\nNitpicking, I think this would be more readable aligning the dots:\n\n\tSize adjusted = requestedSize.boundedTo(supportedSizes.max)\n\t\t\t\t .expandedTo(supportedSizes.min);\n\n\treturn adjusted.shrunkBy(supportedSizes.min)\n\t\t .alignedDownTo(hStep, vStep)\n\t\t .grownBy(supportedSizes.min);\n\nNo need to resubmit just for that.\n\n> +}\n> +\n> +} /* namespace */\n> +\n> CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> {\n> \tconst CameraSensor *sensor = data_->sensor_.get();\n> @@ -998,10 +1025,19 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\t}\n> \n> \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> +\t\t\tSize adjustedSize = pipeConfig_->captureSize;\n> +\t\t\t/*\n> +\t\t\t * The converter (when present) may not be able to output\n> +\t\t\t * a size identical to its input size. The capture size is thus\n> +\t\t\t * not guaranteed to be a valid output size. In such cases, use\n> +\t\t\t * the smaller valid output size closest to the requested.\n> +\t\t\t */\n> +\t\t\tif (!pipeConfig_->outputSizes.contains(adjustedSize))\n> +\t\t\t\tadjustedSize = adjustSize(cfg.size, pipeConfig_->outputSizes);\n> \t\t\tLOG(SimplePipeline, Debug)\n> \t\t\t\t<< \"Adjusting size from \" << cfg.size\n> -\t\t\t\t<< \" to \" << pipeConfig_->captureSize;\n> -\t\t\tcfg.size = pipeConfig_->captureSize;\n> +\t\t\t\t<< \" to \" << adjustedSize;\n> +\t\t\tcfg.size = adjustedSize;\n> \t\t\tstatus = Adjusted;\n> \t\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 767E6C0DA4\n\tfor ;\n\tSun, 7 Apr 2024 01:03:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D5D563352;\n\tSun, 7 Apr 2024 03:03:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5AAC861BB3\n\tfor ;\n\tSun, 7 Apr 2024 03:03:00 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4D9E4231;\n\tSun, 7 Apr 2024 03:02:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KmmrwMnQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712451740;\n\tbh=t41LoyCdEJNQEGSzJfqrtXdVrDao7ro0ssS41o0DCK8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KmmrwMnQaQ4vbHTbujpscNsKgolP1F0UId0rPQ6z9xOsmcyeXlk7XJx4cWwWU3uor\n\tN7RsCSGbUC3GZl3Euuga1Lk+zbhFxviA1RJeAtcN0F2ake8ByVwmr4iQUES3ln68sa\n\tIbkZh2/9/OYAmxvmGtWz357KIIUB6If3162iH7X8=","Date":"Sun, 7 Apr 2024 04:02:48 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tAndrey Konovalov ,\n\tAndrei Konovalov ,\n\tBryan O'Donoghue ,\n\tMaxime Ripard , Pavel Machek ,\n\tHans de Goede ,\n\tKieran Bingham ","Subject":"Re: [PATCH v7 01/18] libcamera: pipeline: simple: fix size\n\tadjustment in validate()","Message-ID":"<20240407010248.GF23422@pendragon.ideasonboard.com>","References":"<20240404084657.353464-1-mzamazal@redhat.com>\n\t<20240404084657.353464-2-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240404084657.353464-2-mzamazal@redhat.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29177,"web_url":"https://patchwork.libcamera.org/comment/29177/","msgid":"<87zfu4fdza.fsf@redhat.com>","date":"2024-04-08T08:36:09","subject":"Re: [PATCH v7 01/18] libcamera: pipeline: simple: fix size\n\tadjustment in validate()","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n> On Thu, Apr 04, 2024 at 10:46:38AM +0200, Milan Zamazal wrote:\n>> From: Andrey Konovalov \n>> \n>\n>> SimpleCameraConfiguration::validate() adjusts the configuration of its\n>> streams (if the size is not in the outputSizes) to the captureSize. But\n>> the captureSize itself can be not in the outputSizes, and then the\n>> adjusted configuration won't be valid resulting in camera configuration\n>> failure.\n>> \n>> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n>> Tested-by: Pavel Machek \n>> Reviewed-by: Milan Zamazal \n>> Reviewed-by: Pavel Machek \n>> Reviewed-by: Laurent Pinchart \n>> Signed-off-by: Andrey Konovalov \n>> Signed-off-by: Hans de Goede \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++--\n>> 1 file changed, 38 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 01f2a977..d2b88795 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -882,6 +882,33 @@ SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,\n>> {\n>> }\n>> \n>> +namespace {\n>> +\n>> +static Size adjustSize(const Size &requestedSize, const SizeRange &supportedSizes)\n>> +{\n>> +\tASSERT(supportedSizes.min <= supportedSizes.max);\n>> +\n>> +\tif (supportedSizes.min == supportedSizes.max)\n>> +\t\treturn supportedSizes.max;\n>> +\n>> +\tunsigned int hStep = supportedSizes.hStep;\n>> +\tunsigned int vStep = supportedSizes.vStep;\n>> +\n>> +\tif (hStep == 0)\n>> +\t\thStep = supportedSizes.max.width - supportedSizes.min.width;\n>> +\tif (vStep == 0)\n>> +\t\tvStep = supportedSizes.max.height - supportedSizes.min.height;\n>> +\n>> +\tSize adjusted = requestedSize.boundedTo(supportedSizes.max)\n>> +\t\t\t\t.expandedTo(supportedSizes.min);\n>> +\n>> +\treturn adjusted.shrunkBy(supportedSizes.min)\n>> +\t\t.alignedDownTo(hStep, vStep)\n>> +\t\t.grownBy(supportedSizes.min);\n>\n> Nitpicking, I think this would be more readable aligning the dots:\n\nI agree, but LSP has different opinion (seems to prefer tab alignment).\nI outsource formatting to LSP, which usually matches libcamera formatting very\nwell, and live with its recommendations, unless it suggests something clearly\nwrong.\n\nIt could look better with line breaks after requestedSize and `adjusted' but if\nanybody uses a more sane tab width than 8 then it doesn't matter much anyway.\n\nRegards,\nMilan\n\n> \tSize adjusted = requestedSize.boundedTo(supportedSizes.max)\n> \t\t\t\t .expandedTo(supportedSizes.min);\n>\n> \treturn adjusted.shrunkBy(supportedSizes.min)\n> \t\t .alignedDownTo(hStep, vStep)\n> \t\t .grownBy(supportedSizes.min);\n>\n> No need to resubmit just for that.\n>\n>> +}\n>> +\n>> +} /* namespace */\n>> +\n>> CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> {\n>> \tconst CameraSensor *sensor = data_->sensor_.get();\n>> @@ -998,10 +1025,19 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> \t\t}\n>> \n>> \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>> +\t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>> +\t\t\t/*\n>> +\t\t\t * The converter (when present) may not be able to output\n>> +\t\t\t * a size identical to its input size. The capture size is thus\n>> +\t\t\t * not guaranteed to be a valid output size. In such cases, use\n>> +\t\t\t * the smaller valid output size closest to the requested.\n>> +\t\t\t */\n>> +\t\t\tif (!pipeConfig_->outputSizes.contains(adjustedSize))\n>> +\t\t\t\tadjustedSize = adjustSize(cfg.size, pipeConfig_->outputSizes);\n>> \t\t\tLOG(SimplePipeline, Debug)\n>> \t\t\t\t<< \"Adjusting size from \" << cfg.size\n>> -\t\t\t\t<< \" to \" << pipeConfig_->captureSize;\n>> -\t\t\tcfg.size = pipeConfig_->captureSize;\n>> +\t\t\t\t<< \" to \" << adjustedSize;\n>> +\t\t\tcfg.size = adjustedSize;\n>> \t\t\tstatus = Adjusted;\n>> \t\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 334D0C0DA4\n\tfor ;\n\tMon, 8 Apr 2024 08:36:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DBD56333B;\n\tMon, 8 Apr 2024 10:36:16 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18D2063339\n\tfor ;\n\tMon, 8 Apr 2024 10:36:14 +0200 (CEST)","from mail-lj1-f200.google.com (mail-lj1-f200.google.com\n\t[209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-355-Co-1PXYLPwSN2hiBsXXX2A-1; Mon, 08 Apr 2024 04:36:12 -0400","by mail-lj1-f200.google.com with SMTP id\n\t38308e7fff4ca-2d87257c610so20161241fa.2\n\tfor ;\n\tMon, 08 Apr 2024 01:36:11 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta23-20020a1709062b1700b00a51dd26f6dcsm517540ejg.51.2024.04.08.01.36.09\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 08 Apr 2024 01:36:09 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Z+40d2AD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1712565373;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=59XRIiEQ030K0fj+aEDCCJujPhroD1h/ClAvSBsAwEk=;\n\tb=Z+40d2ADBjYmxTEm+hsEoamkbnAGfdTxUSMB+IeVE4efbMmGyNn+el6K+2oS9NhPjFo6M3\n\tqOoc1t6NCnyZue4w5v3yQhmS4bqOhSduolu81qU4irwE4nnEuvTIxK7056K8cte5TD/T2N\n\tHlgkmWo3IyQxWZGn2I5Rvzs7kfVI8M0=","X-MC-Unique":"Co-1PXYLPwSN2hiBsXXX2A-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1712565371; x=1713170171;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=59XRIiEQ030K0fj+aEDCCJujPhroD1h/ClAvSBsAwEk=;\n\tb=ivQGneSIUlcs+sbBOE8P7JZOVxfdttqyA9JijbAKPNSKWQA903KmrMu9oL6Uvw1a3L\n\tmY3DnDxaZpDt9BStZPCevZA2SDf10ffHGtkOpYeBC9e7vIRGeiIlEjWTA8qiYhphXPa5\n\tC6ITDXQkWK4aL0/0p4qsJVNGuIZqtTjbtWSKYIOSgMLKaNnJ6AW8ZXDSkt0/yKBLrGeR\n\t1VhODqMAZhZqd/f2O3ECDvYloVpp5Dqws/3fKsGxQdYR9d1nt56RW8kexEoDUfpAIulh\n\tv9TkIuJ+iwc/5AA/dkKGyBxN2CTmRJSwABoZvjusPLcf1uuozUrpXoFHi+LjlhNNtnGe\n\trZcA==","X-Gm-Message-State":"AOJu0YzlXk9XN7Cs8WpL3FR4m4bk9sjhkkcRXnE7fRVkvtJ/uuj4gCoY\n\tdCC10uJNq2Diik/dP921pTw7zJznZGOlpcJCTY8xqDsfXyfnnzcbpmKFxHDiKwmRXkQFPWE0kfv\n\tkzzlf5I9NoLNdX3z1ssYZcU+w50dhSAE8ImX4a6Ng9DVzdX5xJcaQlY7Bh4Cael2mU9JgS58=","X-Received":["by 2002:a2e:3210:0:b0:2d7:121f:6b7 with SMTP id\n\ty16-20020a2e3210000000b002d7121f06b7mr7023169ljy.38.1712565370845; \n\tMon, 08 Apr 2024 01:36:10 -0700 (PDT)","by 2002:a2e:3210:0:b0:2d7:121f:6b7 with SMTP id\n\ty16-20020a2e3210000000b002d7121f06b7mr7023147ljy.38.1712565370448; \n\tMon, 08 Apr 2024 01:36:10 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEJTclioRrq4/sD0L+NwHqlUhdVT0UD//RkKdcNw255aYUi+2MF9UeEoH7VE12yk2vxeqZ65Q==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Andrey Konovalov\n\t, Andrei Konovalov\n\t, Bryan O'Donoghue\n\t, Maxime Ripard , Pavel\n\tMachek , Hans de Goede , Kieran\n\tBingham ","Subject":"Re: [PATCH v7 01/18] libcamera: pipeline: simple: fix size\n\tadjustment in validate()","In-Reply-To":"<20240407010248.GF23422@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Sun, 7 Apr 2024 04:02:48 +0300\")","References":"<20240404084657.353464-1-mzamazal@redhat.com>\n\t<20240404084657.353464-2-mzamazal@redhat.com>\n\t<20240407010248.GF23422@pendragon.ideasonboard.com>","Date":"Mon, 08 Apr 2024 10:36:09 +0200","Message-ID":"<87zfu4fdza.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29218,"web_url":"https://patchwork.libcamera.org/comment/29218/","msgid":"<20240414205315.GG12561@pendragon.ideasonboard.com>","date":"2024-04-14T20:53:15","subject":"Re: [PATCH v7 01/18] libcamera: pipeline: simple: fix size\n\tadjustment in validate()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nOn Mon, Apr 08, 2024 at 10:36:09AM +0200, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> > On Thu, Apr 04, 2024 at 10:46:38AM +0200, Milan Zamazal wrote:\n> >> From: Andrey Konovalov \n> >> \n> >> SimpleCameraConfiguration::validate() adjusts the configuration of its\n> >> streams (if the size is not in the outputSizes) to the captureSize. But\n> >> the captureSize itself can be not in the outputSizes, and then the\n> >> adjusted configuration won't be valid resulting in camera configuration\n> >> failure.\n> >> \n> >> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> >> Tested-by: Pavel Machek \n> >> Reviewed-by: Milan Zamazal \n> >> Reviewed-by: Pavel Machek \n> >> Reviewed-by: Laurent Pinchart \n> >> Signed-off-by: Andrey Konovalov \n> >> Signed-off-by: Hans de Goede \n> >> ---\n> >> src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++--\n> >> 1 file changed, 38 insertions(+), 2 deletions(-)\n> >> \n> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> index 01f2a977..d2b88795 100644\n> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> >> @@ -882,6 +882,33 @@ SimpleCameraConfiguration::SimpleCameraConfiguration(Camera *camera,\n> >> {\n> >> }\n> >> \n> >> +namespace {\n> >> +\n> >> +static Size adjustSize(const Size &requestedSize, const SizeRange &supportedSizes)\n> >> +{\n> >> +\tASSERT(supportedSizes.min <= supportedSizes.max);\n> >> +\n> >> +\tif (supportedSizes.min == supportedSizes.max)\n> >> +\t\treturn supportedSizes.max;\n> >> +\n> >> +\tunsigned int hStep = supportedSizes.hStep;\n> >> +\tunsigned int vStep = supportedSizes.vStep;\n> >> +\n> >> +\tif (hStep == 0)\n> >> +\t\thStep = supportedSizes.max.width - supportedSizes.min.width;\n> >> +\tif (vStep == 0)\n> >> +\t\tvStep = supportedSizes.max.height - supportedSizes.min.height;\n> >> +\n> >> +\tSize adjusted = requestedSize.boundedTo(supportedSizes.max)\n> >> +\t\t\t\t.expandedTo(supportedSizes.min);\n> >> +\n> >> +\treturn adjusted.shrunkBy(supportedSizes.min)\n> >> +\t\t.alignedDownTo(hStep, vStep)\n> >> +\t\t.grownBy(supportedSizes.min);\n> >\n> > Nitpicking, I think this would be more readable aligning the dots:\n> \n> I agree, but LSP has different opinion (seems to prefer tab alignment).\n> I outsource formatting to LSP, which usually matches libcamera formatting very\n> well, and live with its recommendations, unless it suggests something clearly\n> wrong.\n\nOur checkstyle.py uses clang-format, maybe this is something we should\nfile as an enhancement request. We consider the clang-format output as\ninformative and depart from it when needed.\n\n> It could look better with line breaks after requestedSize and `adjusted' but if\n> anybody uses a more sane tab width than 8 then it doesn't matter much anyway.\n\nA tab is 8 spaces, anything else is insane :-)\n\n> > \tSize adjusted = requestedSize.boundedTo(supportedSizes.max)\n> > \t\t\t\t .expandedTo(supportedSizes.min);\n> >\n> > \treturn adjusted.shrunkBy(supportedSizes.min)\n> > \t\t .alignedDownTo(hStep, vStep)\n> > \t\t .grownBy(supportedSizes.min);\n> >\n> > No need to resubmit just for that.\n> >\n> >> +}\n> >> +\n> >> +} /* namespace */\n> >> +\n> >> CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >> {\n> >> \tconst CameraSensor *sensor = data_->sensor_.get();\n> >> @@ -998,10 +1025,19 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >> \t\t}\n> >> \n> >> \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> >> +\t\t\tSize adjustedSize = pipeConfig_->captureSize;\n> >> +\t\t\t/*\n> >> +\t\t\t * The converter (when present) may not be able to output\n> >> +\t\t\t * a size identical to its input size. The capture size is thus\n> >> +\t\t\t * not guaranteed to be a valid output size. In such cases, use\n> >> +\t\t\t * the smaller valid output size closest to the requested.\n> >> +\t\t\t */\n> >> +\t\t\tif (!pipeConfig_->outputSizes.contains(adjustedSize))\n> >> +\t\t\t\tadjustedSize = adjustSize(cfg.size, pipeConfig_->outputSizes);\n> >> \t\t\tLOG(SimplePipeline, Debug)\n> >> \t\t\t\t<< \"Adjusting size from \" << cfg.size\n> >> -\t\t\t\t<< \" to \" << pipeConfig_->captureSize;\n> >> -\t\t\tcfg.size = pipeConfig_->captureSize;\n> >> +\t\t\t\t<< \" to \" << adjustedSize;\n> >> +\t\t\tcfg.size = adjustedSize;\n> >> \t\t\tstatus = Adjusted;\n> >> \t\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 0E44CBE08B\n\tfor ;\n\tSun, 14 Apr 2024 20:53:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FECB63352;\n\tSun, 14 Apr 2024 22:53:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C6926333B\n\tfor ;\n\tSun, 14 Apr 2024 22:53:25 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D10D17E4;\n\tSun, 14 Apr 2024 22:52:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"k+nxXPt4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713127959;\n\tbh=6SZdI3i40QHi8ucptN69MovKAJ7QJZ3365DLp30Ev7A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=k+nxXPt4zMPbF1LBvd14E6/dz17ARWJ1ODyfc8EVPnB0Ro7w7IiNYzIBW+yiu2tsa\n\tCvaVCMY0xK2322q6ys+Pb374CDtzPKs3nEzncJO4qEojP4zOPuW2YdF1BQ9+qDkhwH\n\tieR0pslCVG/v5GNODJh8PbmrkAxZdF5IiRicGi9Q=","Date":"Sun, 14 Apr 2024 23:53:15 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tAndrey Konovalov ,\n\tAndrei Konovalov ,\n\tBryan O'Donoghue ,\n\tMaxime Ripard , Pavel Machek ,\n\tHans de Goede ,\n\tKieran Bingham ","Subject":"Re: [PATCH v7 01/18] libcamera: pipeline: simple: fix size\n\tadjustment in validate()","Message-ID":"<20240414205315.GG12561@pendragon.ideasonboard.com>","References":"<20240404084657.353464-1-mzamazal@redhat.com>\n\t<20240404084657.353464-2-mzamazal@redhat.com>\n\t<20240407010248.GF23422@pendragon.ideasonboard.com>\n\t<87zfu4fdza.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87zfu4fdza.fsf@redhat.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]