[{"id":31729,"web_url":"https://patchwork.libcamera.org/comment/31729/","msgid":"<87msj7gj3g.fsf@redhat.com>","date":"2024-10-14T08:54:43","subject":"Re: [PATCH] pipeline: simple: Consider output sizes when choosing\n\tpipe config","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Robert,\n\nthank you for the patch.\n\nRobert Mader <robert.mader@collabora.com> writes:\n\n> In order to avoid having to adjust the size further down below which\n> again can break user assumptions. Notably, without this the capture size\n> of 1920x1080 gets adjusted to 1912x1080 when used with the swISP using a\n> bayer pattern width of 4, breaking users like Gstreamer down the line.\n>\n> Closes https://bugs.libcamera.org/show_bug.cgi?id=236\n>\n> Signed-off-by: Robert Mader <robert.mader@collabora.com>\n>\n> ---\n>\n> I'm not really sure if this is the correct approach, but sending it out\n> already for feedback. So far this gives me promissing results on tested\n> devices.\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 3 ++-\n>  1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 3ddce71d..2d185b90 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -1048,7 +1048,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>  \t\tconst Size &size = pipeConfig->captureSize;\n>  \n>  \t\tif (size.width >= maxStreamSize.width &&\n> -\t\t    size.height >= maxStreamSize.height) {\n> +\t\t    size.height >= maxStreamSize.height &&\n> +\t\t    pipeConfig->outputSizes.contains(maxStreamSize)) {\n\nSome more explanation is needed.  The original code simply selects the\nsmallest size that can cover the output.  Now with this change, what if\n`size' is sufficiently high but outputSizes doesn't contain exactly\nmaxStreamSize but it contains some larger value?  And what happens if\nsoftware ISP is not enabled for the pipeline?\n\n>  \t\t\tif (!pipeConfig_ || size < pipeConfig_->captureSize)\n>  \t\t\t\tpipeConfig_ = pipeConfig;\n>  \t\t}","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 182B5C32F4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Oct 2024 08:54:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E365965369;\n\tMon, 14 Oct 2024 10:54:51 +0200 (CEST)","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 59FBE65369\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Oct 2024 10:54:49 +0200 (CEST)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-60-dg-47uN3OgqZI14NISK6xA-1; Mon, 14 Oct 2024 04:54:47 -0400","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-a99efc7d881so111348366b.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Oct 2024 01:54:46 -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\ta640c23a62f3a-a9a1cf76b1asm7708466b.132.2024.10.14.01.54.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 14 Oct 2024 01:54:44 -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=\"cX+KDXY0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1728896088;\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=IWQL365M60rCXCqpN7Kq0mihfIZHCzsQcY6cBfyPrbA=;\n\tb=cX+KDXY0lkyHdfueKPOTCyEgj4KRMHJuyYYM7q/qeqZUQiUH6EIiyifBgmnNv8wN21bGAn\n\tsoCp98hjNqIZmFQP+YN7EFH/Qr1wHvZTaIfgJHY8a7fo6zVzG8A6LcZBHxXhe4dvYDyRlO\n\tvjhTRR5irg1crajPBofUCen/17y5J7E=","X-MC-Unique":"dg-47uN3OgqZI14NISK6xA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1728896085; x=1729500885;\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=IWQL365M60rCXCqpN7Kq0mihfIZHCzsQcY6cBfyPrbA=;\n\tb=wfIViYnHH6iwxmB6lLlnPVabwIfZPELKkSyGT3WgDDZob4Lm6TbDRZ3cDWHggVodQ+\n\taefrecx+PSBC/Mfros7T4SsnvdSwNN4DYgRZ1UQhvte9vN4O+4Jl6zXSh+oDRrmC3khy\n\truN9ILaiCO6AAlEFeKIQ4ceLPMb29uZFhixqDRzg87iJk5lKP57pA9i6qS6RDYEEsxXk\n\tPhQZKYrKY7Az91xCvV+oQWdh0Lay/0j7vUpv1dEOsuYED9X/a2JEraWcY6+9LvdhtHdI\n\tIalrC4kItpy64IQdZ/KHLgNMrYiLHEwJ434vcv1WsGIZbx8LZzOD2dy1y2n4LTHm1cQx\n\tvk/g==","X-Gm-Message-State":"AOJu0Yyn7IZPI7BA2DzImD1HTQ95FnUq1PlygncdqoHFlHsV4oOOfdLw\n\t1bsnjxKnxxtQmuXsVS5S+68TtyzQBc7DoXo49ml7N1uYU0vjymuX50+kPMWM5dkmZWYxU/7ePhQ\n\t+ueJGHlpCQYrAdTyUxS8qu1MWHs35+U4dkQF9aKUbpzkZ1U0CHO182z4sX7mJAotle9h6EUvvXR\n\tnxQAKXLssPMUS4afldZT9VDnfYtnW81OFLBWSGDT1QVuFZpYselGpthKI=","X-Received":["by 2002:a17:906:dc91:b0:a9a:1253:4d81 with SMTP id\n\ta640c23a62f3a-a9a125350e2mr185802666b.47.1728896085573; \n\tMon, 14 Oct 2024 01:54:45 -0700 (PDT)","by 2002:a17:906:dc91:b0:a9a:1253:4d81 with SMTP id\n\ta640c23a62f3a-a9a125350e2mr185801066b.47.1728896085235; \n\tMon, 14 Oct 2024 01:54:45 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEERR1D/0hQej/BEXAW5pUd+/QxTxamgW0jaNB1osAs3vCfT1v8aQBjUan4hn8Y7HTuwAmbxQ==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Robert Mader <robert.mader@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] pipeline: simple: Consider output sizes when choosing\n\tpipe config","In-Reply-To":"<20241011184600.17118-1-robert.mader@collabora.com> (Robert\n\tMader's message of \"Fri, 11 Oct 2024 20:46:00 +0200\")","References":"<20241011184600.17118-1-robert.mader@collabora.com>","Date":"Mon, 14 Oct 2024 10:54:43 +0200","Message-ID":"<87msj7gj3g.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":"<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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":36505,"web_url":"https://patchwork.libcamera.org/comment/36505/","msgid":"<ccbe2b2a-5fa3-43a3-a9fb-39fb502cae97@collabora.com>","date":"2025-10-27T23:23:19","subject":"Re: [PATCH] pipeline: simple: Consider output sizes when choosing\n\tpipe config","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Hi Milan,\n\nOn 14.10.24 10:54, Milan Zamazal wrote:\n> Hi Robert,\n>\n> thank you for the patch.\n>\n> Robert Mader<robert.mader@collabora.com> writes:\n>\n>> In order to avoid having to adjust the size further down below which\n>> again can break user assumptions. Notably, without this the capture size\n>> of 1920x1080 gets adjusted to 1912x1080 when used with the swISP using a\n>> bayer pattern width of 4, breaking users like Gstreamer down the line.\n>>\n>> Closeshttps://bugs.libcamera.org/show_bug.cgi?id=236\n>>\n>> Signed-off-by: Robert Mader<robert.mader@collabora.com>\n>>\n>> ---\n>>\n>> I'm not really sure if this is the correct approach, but sending it out\n>> already for feedback. So far this gives me promissing results on tested\n>> devices.\n>> ---\n>>   src/libcamera/pipeline/simple/simple.cpp | 3 ++-\n>>   1 file changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 3ddce71d..2d185b90 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -1048,7 +1048,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>   \t\tconst Size &size = pipeConfig->captureSize;\n>>   \n>>   \t\tif (size.width >= maxStreamSize.width &&\n>> -\t\t    size.height >= maxStreamSize.height) {\n>> +\t\t    size.height >= maxStreamSize.height &&\n>> +\t\t    pipeConfig->outputSizes.contains(maxStreamSize)) {\n> Some more explanation is needed.  The original code simply selects the\n> smallest size that can cover the output.  Now with this change, what if\n> `size' is sufficiently high but outputSizes doesn't contain exactly\n> maxStreamSize but it contains some larger value?  And what happens if\n> software ISP is not enabled for the pipeline?\nI agree this solution wasn't very intuitive and probably wrong in some \ncases. I send out a v2 which hopefully answers your questions.\n>>   \t\t\tif (!pipeConfig_ || size < pipeConfig_->captureSize)\n>>   \t\t\t\tpipeConfig_ = pipeConfig;\n>>   \t\t}","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 C8B96BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Oct 2025 23:23:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 111846078B;\n\tTue, 28 Oct 2025 00:23:30 +0100 (CET)","from sender4-pp-f112.zoho.com (sender4-pp-f112.zoho.com\n\t[136.143.188.112])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC46E606A0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 28 Oct 2025 00:23:27 +0100 (CET)","by mx.zohomail.com with SMTPS id 1761607402498628.6230775096287;\n\tMon, 27 Oct 2025 16:23:22 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"B5deXKww\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1761607405; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=FW8qmE/y6AmXluVnPAmYdARiSa1qx4rLcjoDlcSUjKrjfQLSL0RChWqrPTZLESnT5uPSbOO+1Q9T2my0kctU6K7UTWRubpi6lLZKrszxnHw0LJXifGM3SnREO/MQqG0jQH7Qlv6SyPd6xmTDK9DE0bQ8kCD7G+D7scVcrKeB1GI=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1761607405;\n\th=Content-Type:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To;\n\tbh=LdeTWYLvqwJZ9/m7vIkVDLXaFeYYuVPqQ2f396aapKQ=; \n\tb=RuejsB/3kNLmKQuqT14L5Qc4ndEA3V3LGAThIiOQSerNuCfJ80HyHi7DQHzLJrOUU3EVar1InZgC2OWmpkv1ytAF8HymxxXr2pSfIQq6xeo0Yiieo5GQ9OGmglNgJ3wwOc7z3oeSe3NQ3bZOLl7DiZmCgl1D490wXrbGi6cCgos=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass  header.i=collabora.com;\n\tspf=pass  smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=<robert.mader@collabora.com>","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1761607405;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Content-Type:Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:Cc:Cc:References:From:From:In-Reply-To:Message-Id:Reply-To;\n\tbh=LdeTWYLvqwJZ9/m7vIkVDLXaFeYYuVPqQ2f396aapKQ=;\n\tb=B5deXKwwfQp2cnEvnAIn7QMHaCYcUX2BoYPJGruRHlspxTVxnVedPe6E7Yphg+bh\n\teqzuOZIQzH3wLJJTCqPEXTXvNkBP/PwWvGyptBSd6EcAJpdNsJE0qpxRzfKYO0D3oZE\n\thPbC201OOJDih86Sh+U3CawSNN2pvfAxRkUibZTI=","Content-Type":"multipart/alternative;\n\tboundary=\"------------8y7YlHJyWJBj7RuJSzPJGrgx\"","Message-ID":"<ccbe2b2a-5fa3-43a3-a9fb-39fb502cae97@collabora.com>","Date":"Tue, 28 Oct 2025 00:23:19 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH] pipeline: simple: Consider output sizes when choosing\n\tpipe config","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20241011184600.17118-1-robert.mader@collabora.com>\n\t<87msj7gj3g.fsf@redhat.com>","Content-Language":"en-US, de-DE, en-GB","From":"Robert Mader <robert.mader@collabora.com>","In-Reply-To":"<87msj7gj3g.fsf@redhat.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]