[{"id":28589,"web_url":"https://patchwork.libcamera.org/comment/28589/","msgid":"<87sf2o9pfr.fsf@redhat.com>","date":"2024-01-23T12:58:32","subject":"Re: [PATCH v2 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":"Hans de Goede writes:\n\n> From: Andrey Konovalov \n>\n> SimpleCameraConfiguration::validate() adjusts the configuration\n> of its streams (if the size is not in the outputSizes) to\n> the captureSize. But the captureSize itself can be not in the\n> outputSizes, and then the adjusted configuration won't be\n> valid resulting in camera configuration failure.\n>\n> Signed-off-by: Andrey Konovalov \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n\nReviewed-by: Milan Zamazal \n\n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 911051b2..4d0e7255 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -997,10 +997,13 @@ 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\tif (!pipeConfig_->outputSizes.contains(adjustedSize))\n> +\t\t\t\tadjustedSize = pipeConfig_->outputSizes.max;\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}","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 7C9C0C323E\n\tfor ;\n\tTue, 23 Jan 2024 12:58:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7936C62944;\n\tTue, 23 Jan 2024 13:58:40 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4B678628E9\n\tfor ;\n\tTue, 23 Jan 2024 13:58:38 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-414-fY5FSMTEOz6uXimsf3JjYg-1; Tue, 23 Jan 2024 07:58:35 -0500","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-40e5769e2deso40214395e9.2\n\tfor ;\n\tTue, 23 Jan 2024 04:58:35 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tm10-20020adffa0a000000b003392172fd60sm11107483wrr.51.2024.01.23.04.58.33\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Jan 2024 04:58:33 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Dwts6XKh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1706014717;\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=xDf2IIY4T+rI+rb/Glf1z293oN0FK+eG1v3VJSS/RGo=;\n\tb=Dwts6XKhs5swxUsyZmCDp1YmSaWRkoxA50tfyR/JQSbRfzQcUpSBb6imf8BvR8CKFXE+ho\n\tx9FseWQwhQVizG/EaZJuqzW0hIvmvMsZxKBRSlOEwVQ0k9IYatFvn9BRF/sG/3bM9kqgAS\n\t7aSwt5GqtLwmn7v/ODS5WhD7awxJNQg=","X-MC-Unique":"fY5FSMTEOz6uXimsf3JjYg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706014714; x=1706619514;\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=xDf2IIY4T+rI+rb/Glf1z293oN0FK+eG1v3VJSS/RGo=;\n\tb=CZfjGR2OSGJdeUlBFqZBzpOOSw1DF313dpH5+ntKQ7FRjD/UB4pAdOISu9av1C1/qO\n\tAc8cZnJR/il7ClErrkmemoOBU8uxag8bT6IIy+9UMeIK4bOMthfLKUoMx0likb8an4Cb\n\tPG56mTHtLKoPzfmNlyuMqiueXVkRaiwfrxQPl2KAFAb0tOghuQ10Z5JVp0Y3nQfgFP8s\n\thA31iuDVx4NUjhH0rnqdmKO6DEAnMBzCcFxYA/zu7jN8dFzfo685CqGULxWguigPFk9S\n\tGP9w43HXiVFkK8H7uIXv8STtcBr8T9RVXYuNGd9THbinm/6XEBgjtXskPzITR2XkJa6z\n\tPGvw==","X-Gm-Message-State":"AOJu0YxCOBvTWSpSNx2yOlC26P+ouTzqB6RsQqZfrahevoe19ADwDm+9\n\tBo18oBM89v6a+2EFSzVjJ7XCKIa0ArIVxqSJ8zgOjg8uiOSUI4lOV3YlxP2fUHJgVPoouEVGEV6\n\tSXGaEWkE52aWBgn99DB8Bp4OM50NVpFXyYCtB2I3J+ydlouaAXqBcEBhMxfWmRoZ5yZ2Po38=","X-Received":["by 2002:a05:600c:35c2:b0:40d:4502:741c with SMTP id\n\tr2-20020a05600c35c200b0040d4502741cmr74002wmq.125.1706014714309; \n\tTue, 23 Jan 2024 04:58:34 -0800 (PST)","by 2002:a05:600c:35c2:b0:40d:4502:741c with SMTP id\n\tr2-20020a05600c35c200b0040d4502741cmr73993wmq.125.1706014714039; \n\tTue, 23 Jan 2024 04:58:34 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEkaS0aleVYYDRajPDU0N4uIUNkgp71TQ8gvgdt6Xdnao7P33oDGh2LpSZjfG4Ck4o5Edc3vg==","From":"Milan Zamazal ","To":"Hans de Goede ","Subject":"Re: [PATCH v2 01/18] libcamera: pipeline: simple: fix size\n\tadjustment in validate()","In-Reply-To":"<20240113142218.28063-2-hdegoede@redhat.com> (Hans de Goede's\n\tmessage of \"Sat, 13 Jan 2024 15:22:01 +0100\")","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-2-hdegoede@redhat.com>","Date":"Tue, 23 Jan 2024 13:58:32 +0100","Message-ID":"<87sf2o9pfr.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","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28596,"web_url":"https://patchwork.libcamera.org/comment/28596/","msgid":"<20240123140641.GA10679@pendragon.ideasonboard.com>","date":"2024-01-23T14:06:41","subject":"Re: [libcamera-devel] [PATCH v2 01/18] libcamera: pipeline: simple:\n\tfix size adjustment in validate()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Andrey,\n\nThank you for the patch.\n\nOn Sat, Jan 13, 2024 at 03:22:01PM +0100, 📷-dev wrote:\n> From: Andrey Konovalov \n> \n> SimpleCameraConfiguration::validate() adjusts the configuration\n> of its streams (if the size is not in the outputSizes) to\n> the captureSize. But the captureSize itself can be not in the\n> outputSizes, and then the adjusted configuration won't be\n> valid resulting in camera configuration failure.\n\nI've always thought the git commit message line length limit of 72\ncharacters was small, let's not make it even smaller.\n\n> Signed-off-by: Andrey Konovalov \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 7 +++++--\n> 1 file changed, 5 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 911051b2..4d0e7255 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -997,10 +997,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\t}\n> \n> \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> +\t\t\tSize adjustedSize = pipeConfig_->captureSize;\n\nThis deserves a comment to explain the logic.\n\n\t\t\t/*\n\t\t\t * The converter (when present) may not be able to\n\t\t\t * output a size identical to its input size. The\n\t\t\t * capture size is thus not guaranteed to be a valid\n\t\t\t * output size. In such cases, use the maximum output\n\t\t\t * size instead.\n\t\t\t */\n\nI'm however wondering, shouldn't we pick the max only when the requested\nsize is larger than outputSizes.max, and pick the min otherwise ?\n\n> +\t\t\tif (!pipeConfig_->outputSizes.contains(adjustedSize))\n> +\t\t\t\tadjustedSize = pipeConfig_->outputSizes.max;\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 8461AC323E\n\tfor ;\n\tTue, 23 Jan 2024 14:06:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A50CB62945;\n\tTue, 23 Jan 2024 15:06:43 +0100 (CET)","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 1E18E62916\n\tfor ;\n\tTue, 23 Jan 2024 15:06:43 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 11156BEB;\n\tTue, 23 Jan 2024 15:05:29 +0100 (CET)"],"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=\"J1TY+CDo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706018729;\n\tbh=HWYZkz7sp9f6GjBsH3MQyJz56GsaE0oJnz9H6DJL/sk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=J1TY+CDoJex89bUItnD6cCtuAm+0mK/Nt9Tum7yIyOXNdq93WFJxfjcJlalHTyzMv\n\tmMPC+ICy0OZMxSLPLxvAghgC6fiC6TQ3ukubFsOJwl3yCN+TTCuHo1RkedNrD+dxQ0\n\thFrIHrPejWY4ucb4QfW4831tnzKq4ZzAFAWv4Llk=","Date":"Tue, 23 Jan 2024 16:06:41 +0200","From":"Laurent Pinchart ","To":"Hans de Goede ","Subject":"Re: [libcamera-devel] [PATCH v2 01/18] libcamera: pipeline: simple:\n\tfix size adjustment in validate()","Message-ID":"<20240123140641.GA10679@pendragon.ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-2-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20240113142218.28063-2-hdegoede@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","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28599,"web_url":"https://patchwork.libcamera.org/comment/28599/","msgid":"<50c3a242-a345-451a-93fe-986b0270a561@gmail.com>","date":"2024-01-23T14:31:04","subject":"Re: [libcamera-devel] [PATCH v2 01/18] libcamera: pipeline: simple:\n\tfix size adjustment in validate()","submitter":{"id":179,"url":"https://patchwork.libcamera.org/api/people/179/","name":"Andrei Konovalov","email":"andrey.konovalov.ynk@gmail.com"},"content":"Hi Laurent,\n\nThanks for the review!\n\nOn 23.01.2024 17:06, Laurent Pinchart wrote:\n> Hi Andrey,\n> \n> Thank you for the patch.\n> \n> On Sat, Jan 13, 2024 at 03:22:01PM +0100, 📷-dev wrote:\n>> From: Andrey Konovalov \n>>\n>> SimpleCameraConfiguration::validate() adjusts the configuration\n>> of its streams (if the size is not in the outputSizes) to\n>> the captureSize. But the captureSize itself can be not in the\n>> outputSizes, and then the adjusted configuration won't be\n>> valid resulting in camera configuration failure.\n> \n> I've always thought the git commit message line length limit of 72\n> characters was small, let's not make it even smaller.\n\nI was always having hard time calculating the line lengths. Will fix that.\n\n>> Signed-off-by: Andrey Konovalov \n>> Signed-off-by: Hans de Goede \n>> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n>> Tested-by: Pavel Machek \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 7 +++++--\n>> 1 file changed, 5 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 911051b2..4d0e7255 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -997,10 +997,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> \t\t}\n>> \n>> \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>> +\t\t\tSize adjustedSize = pipeConfig_->captureSize;\n> \n> This deserves a comment to explain the logic.\n> \n> \t\t\t/*\n> \t\t\t * The converter (when present) may not be able to\n> \t\t\t * output a size identical to its input size. The\n> \t\t\t * capture size is thus not guaranteed to be a valid\n> \t\t\t * output size. In such cases, use the maximum output\n> \t\t\t * size instead.\n> \t\t\t */\n\nMakes sense, thanks!\n\n> I'm however wondering, shouldn't we pick the max only when the requested\n> size is larger than outputSizes.max, and pick the min otherwise ?\n\nYes, this is better than always using outputSizes.max.\n\nThanks,\nAndrei\n\n>> +\t\t\tif (!pipeConfig_->outputSizes.contains(adjustedSize))\n>> +\t\t\t\tadjustedSize = pipeConfig_->outputSizes.max;\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>> \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 DF6C4BDC71\n\tfor ;\n\tTue, 23 Jan 2024 14:31:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E8D0562945;\n\tTue, 23 Jan 2024 15:31:07 +0100 (CET)","from mail-wm1-x329.google.com (mail-wm1-x329.google.com\n\t[IPv6:2a00:1450:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 91D7E62916\n\tfor ;\n\tTue, 23 Jan 2024 15:31:06 +0100 (CET)","by mail-wm1-x329.google.com with SMTP id\n\t5b1f17b1804b1-40ec2594544so3232245e9.0\n\tfor ;\n\tTue, 23 Jan 2024 06:31:06 -0800 (PST)","from [192.168.118.26] ([87.116.160.165])\n\tby smtp.gmail.com with ESMTPSA id\n\taz8-20020a05600c600800b0040ebf626bfcsm1761860wmb.1.2024.01.23.06.31.04\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 23 Jan 2024 06:31:05 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"BQ51Tlnr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1706020266; x=1706625066;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ZIaxIteh37exLx68jl7IdXfFnshnhEY+m75GEECuBN8=;\n\tb=BQ51TlnrfXoNRsaYvy1bRUEY3W+/x5PzCfsLINj2fDii9Dd7cEVBP37rS7AHlURgHv\n\tuDbFnF8XV0hscCdg51OHzDTHHL+mj74lhZS3QJAmmFMFsl26x/O8OnJ7F3CfWz/znStQ\n\tac9ksbAU4i1MHQRkLUZqFsDPBCh5hLefMH1i3qS73HgctAMB5Q0XBr2s1E8sLj6TcCq2\n\ta+mXuCbxx9GrsmA7ZNIj1MdmY4nSSTbK8l1twNNjf4uVmxn84OKmCEPFVjhbrPkp5QsO\n\tJF+fkARCmRyihCICCDzatQ6JxI4WA+85OppySkyLL6snv06QRK/kCEZNXX5cx+Ws4yC8\n\tFIyg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706020266; x=1706625066;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ZIaxIteh37exLx68jl7IdXfFnshnhEY+m75GEECuBN8=;\n\tb=dKv/lt8LWF7pSvoshumlqrIAjL/bXIhlbT3XgxcuTSPKqFYSwLanVv7/ZBdDCXWRYP\n\tfeR3RsVqdQUYK70nKh8U82vlvcL5SdiDOLKV8O/M+dMFu53ZRGxEgUUir9110ZxASDpW\n\tx6xi8K991jYrAw9C9dT7erXXdptyyY+kj16yNPMOxN5nfIcb8Z2dOcopvsDu+m5yYfIf\n\tcNm0ebQHGObGUaEpxCajbIJfl7mvnIz4ZV7VjCAmm7whizXqK5XkJS+YUQtP70Z2o1GG\n\tNTLMfdNU7qjx3QlAkMg+Jyc5tOpCA9iTVYqUKv36pGt1saCwU4O2EtYt98iFZTiPrBlq\n\ts8JQ==","X-Gm-Message-State":"AOJu0YziSxs4wD6daPlQbnen3ZX4nQ7AUVDXX9zd1gNXUqBEuYyq45JU\n\tcT/E1GeQlSPf6GRGdmfrcsSOqF8VLMHstQuj1ij50XB6DK2vbWCz","X-Google-Smtp-Source":"AGHT+IHU9oSs+itK9O4d71DsrY1Jwan/lJlldW2fMpstDzfvDvek2q1QGXY5BQagXph944QPyRHxTg==","X-Received":"by 2002:a05:600c:1d06:b0:40e:4f46:d100 with SMTP id\n\tl6-20020a05600c1d0600b0040e4f46d100mr185468wms.101.1706020265878; \n\tTue, 23 Jan 2024 06:31:05 -0800 (PST)","Message-ID":"<50c3a242-a345-451a-93fe-986b0270a561@gmail.com>","Date":"Tue, 23 Jan 2024 17:31:04 +0300","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [libcamera-devel] [PATCH v2 01/18] libcamera: pipeline: simple:\n\tfix size adjustment in validate()","To":"Laurent Pinchart ,\n\tHans de Goede ","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-2-hdegoede@redhat.com>\n\t<20240123140641.GA10679@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Andrei Konovalov ","In-Reply-To":"<20240123140641.GA10679@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28601,"web_url":"https://patchwork.libcamera.org/comment/28601/","msgid":"<20240123144410.GE10679@pendragon.ideasonboard.com>","date":"2024-01-23T14:44:10","subject":"Re: [libcamera-devel] [PATCH v2 01/18] libcamera: pipeline: simple:\n\tfix size adjustment in validate()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 23, 2024 at 05:31:04PM +0300, Andrei Konovalov wrote:\n> On 23.01.2024 17:06, Laurent Pinchart wrote:\n> > On Sat, Jan 13, 2024 at 03:22:01PM +0100, 📷-dev wrote:\n> >> From: Andrey Konovalov \n> >>\n> >> SimpleCameraConfiguration::validate() adjusts the configuration\n> >> of its streams (if the size is not in the outputSizes) to\n> >> the captureSize. But the captureSize itself can be not in the\n> >> outputSizes, and then the adjusted configuration won't be\n> >> valid resulting in camera configuration failure.\n> > \n> > I've always thought the git commit message line length limit of 72\n> > characters was small, let's not make it even smaller.\n> \n> I was always having hard time calculating the line lengths. Will fix that.\n\nIf you use vim, it should be configured correctly by default :-)\n\n> >> Signed-off-by: Andrey Konovalov \n> >> Signed-off-by: Hans de Goede \n> >> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> >> Tested-by: Pavel Machek \n> >> ---\n> >> src/libcamera/pipeline/simple/simple.cpp | 7 +++++--\n> >> 1 file changed, 5 insertions(+), 2 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> index 911051b2..4d0e7255 100644\n> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> >> @@ -997,10 +997,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >> \t\t}\n> >> \n> >> \t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> >> +\t\t\tSize adjustedSize = pipeConfig_->captureSize;\n> > \n> > This deserves a comment to explain the logic.\n> > \n> > \t\t\t/*\n> > \t\t\t * The converter (when present) may not be able to\n> > \t\t\t * output a size identical to its input size. The\n> > \t\t\t * capture size is thus not guaranteed to be a valid\n> > \t\t\t * output size. In such cases, use the maximum output\n> > \t\t\t * size instead.\n> > \t\t\t */\n> \n> Makes sense, thanks!\n> \n> > I'm however wondering, shouldn't we pick the max only when the requested\n> > size is larger than outputSizes.max, and pick the min otherwise ?\n> \n> Yes, this is better than always using outputSizes.max.\n> \n> >> +\t\t\t\tadjustedSize = pipeConfig_->outputSizes.max;\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 8F043BDC71\n\tfor ;\n\tTue, 23 Jan 2024 14:44:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F03862947;\n\tTue, 23 Jan 2024 15:44:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81B36628E9\n\tfor ;\n\tTue, 23 Jan 2024 15:44:11 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BB8DA1890;\n\tTue, 23 Jan 2024 15:42:57 +0100 (CET)"],"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=\"fz2232uv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706020977;\n\tbh=z6rQZTfedIDW/DE5INKtlXN3lXklovw6tr0i3xl7wFM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fz2232uvHOQShLS8RYllyWirg3Qm5fX8IACzOY6Z8inXqAeSLueBZ/oJNaWzzZBFT\n\t6ggM55CLk+b6W6m8ueFn7xZ015UM5YGAmzHxAicY5K4/ZJw05jSUo90+v0actsC/R2\n\tHM+m9gC32dvrg1sFmIr4Sfqa9L6JgcHlb9UWXV/c=","Date":"Tue, 23 Jan 2024 16:44:10 +0200","From":"Laurent Pinchart ","To":"Andrei Konovalov ","Subject":"Re: [libcamera-devel] [PATCH v2 01/18] libcamera: pipeline: simple:\n\tfix size adjustment in validate()","Message-ID":"<20240123144410.GE10679@pendragon.ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-2-hdegoede@redhat.com>\n\t<20240123140641.GA10679@pendragon.ideasonboard.com>\n\t<50c3a242-a345-451a-93fe-986b0270a561@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<50c3a242-a345-451a-93fe-986b0270a561@gmail.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","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]