[{"id":37104,"web_url":"https://patchwork.libcamera.org/comment/37104/","msgid":"","date":"2025-11-28T17:45:42","subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Milan\n\nOn 2025-11-28 02:49, Milan Zamazal wrote:\n> Configuration validation computes the maximum size of all the requested\n> streams and compares it to the output sizes. When e.g. only a raw\n> stream is requested then this may result in an invalid adjustment of its\n> size. This is because the output sizes are computed for processed\n> streams and may be smaller than capture sizes. If a raw stream with the\n> capture size is requested, it may then be wrongly adjusted to a larger\n> size because the output sizes, which are irrelevant for raw streams\n> anyway, are smaller than the requested capture size.\n> \n\nThe problem is somewhere else and a quick look suggests that a wrong\npipeConfig is getting selected to start with.\n\nIs this a bug with current series? Could you post steps to reproduce\nthis ?\n\n> Let's fix the problem by tracking raw and processed streams maximum\n> sizes separately and comparing raw stream sizes against capture rather\n> than output sizes.\n\nThis should ideally be squashed with the original validation patch, no ?\nIntroducing new patches at v16, is not going to help here. It just slows\nthings down for merging (unless there is a bug interaction happening\nthen a split fixup! would've helped for easier review).\n\nI had problem with 8/9 as well as it should have been a separate patch\nand not merged with this but I let it slide already. But you do get my\npoint of initially recommending to split out the colorspace patch (which\nyou ultimately did), it progressed and got merged out of this series -\nbut then you again had a fixup! to be put back in this series. This is\njust unwanted hindrance on the reviewer's end.\n\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 23 ++++++++++++++++-------\n> 1 file changed, 16 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 2820d1254..bb67000e2 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -1153,12 +1153,18 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t}\n> \n> \t/* Find the largest stream size. */\n> -\tSize maxStreamSize;\n> -\tfor (const StreamConfiguration &cfg : config_)\n> -\t\tmaxStreamSize.expandTo(cfg.size);\n> +\tSize maxProcessedStreamSize;\n> +\tSize maxRawStreamSize;\n> +\tfor (const StreamConfiguration &cfg : config_) {\n> +\t\tif (isRaw(cfg))\n> +\t\t\tmaxRawStreamSize.expandTo(cfg.size);\n> +\t\telse\n> +\t\t\tmaxProcessedStreamSize.expandTo(cfg.size);\n> +\t}\n> \n> \tLOG(SimplePipeline, Debug)\n> -\t\t<< \"Largest stream size is \" << maxStreamSize;\n> +\t\t<< \"Largest stream size is \"\n> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n\nThis will be a very confusing log to parse.\n\n> \n> \t/* Cap the number of raw stream configurations */\n> \tunsigned int rawCount = 0;\n> @@ -1217,8 +1223,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\tconst Size &captureSize = pipeConfig->captureSize;\n> \t\tconst Size &maxOutputSize = pipeConfig->outputSizes.max;\n> \n> -\t\tif (maxOutputSize.width >= maxStreamSize.width &&\n> -\t\t maxOutputSize.height >= maxStreamSize.height) {\n> +\t\tif (maxOutputSize.width >= maxProcessedStreamSize.width &&\n> +\t\t maxOutputSize.height >= maxProcessedStreamSize.height &&\n> +\t\t captureSize.width >= maxRawStreamSize.width &&\n> +\t\t captureSize.height >= maxRawStreamSize.height) {\n> \t\t\tif (!pipeConfig_ || captureSize < pipeConfig_->captureSize)\n> \t\t\t\tpipeConfig_ = pipeConfig;\n> \t\t}\n> @@ -1236,7 +1244,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> \t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n> \t\t<< \" -> \" << pipeConfig_->captureSize\n> \t\t<< \"-\" << pipeConfig_->captureFormat\n> -\t\t<< \" for max stream size \" << maxStreamSize;\n> +\t\t<< \" for max stream size \"\n> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n> \n> \t/*\n> \t * Adjust the requested streams.","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 0E370C3257\n\tfor ;\n\tFri, 28 Nov 2025 17:45:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 61EC560A9E;\n\tFri, 28 Nov 2025 18:45:47 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D94656069A\n\tfor ;\n\tFri, 28 Nov 2025 18:45:45 +0100 (CET)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vP2XR-006XDo-0a; Fri, 28 Nov 2025 18:45:45 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vP2XO-00HPEA-Qo; Fri, 28 Nov 2025 18:45:44 +0100","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from )\n\tid 1vP2XO-003Abk-18; Fri, 28 Nov 2025 18:45:42 +0100"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"nspW0P4A\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=F3O/jdc7f4RpKSwfCQ/ltDQkySGGH0bDX7xecG3lHYA=;\n\tb=nspW0P4AI1QAO8pukRiP242+1T\n\tf866z8Vr7TNC5+cBKH2SB/vABGoz3OLAvkr72j0rK/HhcIpZrmzkOvj/AF4/2kdGPeDql6vNPBLpR\n\tZdX2hrm9UtvS1mL7JCqtkMLkAigl9sWOQsVM83sXzjO0gXeFWm0fVO4FPDlvGGlkQgM4yjeO+2g5/\n\txvgkPtOzE4F6KoKRjKcWtK+JsAmO2P82n4CYI2g72JeP34kq6A2NvjSOuEd5B00i5dMK/YNbxiknt\n\tyNzryUaJRd1elXDpa4Dfp4+ZiWdDfmwIeUX1Wbp6lFcKaBDojG3h+CwC2HPlHrQgf3kkbV/lW0QTY\n\tAO6Y6sUQ==;","MIME-Version":"1.0","Date":"Fri, 28 Nov 2025 23:15:42 +0530","From":"Umang Jain ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ,\n\tPaul Elder ","Subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","In-Reply-To":"<20251127211932.122463-10-mzamazal@redhat.com>","References":"<20251127211932.122463-1-mzamazal@redhat.com>\n\t<20251127211932.122463-10-mzamazal@redhat.com>","Message-ID":"","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Spam-Report":"NO, Score=-3.1, Tests=ALL_TRUSTED=-3,AWL=-0.900,BAYES_50=0.8","X-Spam-Score":"-30","X-Spam-Bar":"---","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":37140,"web_url":"https://patchwork.libcamera.org/comment/37140/","msgid":"<85tsyal3y2.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-12-01T13:36:05","subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Umang,\n\nthank you for review.\n\nUmang Jain writes:\n\n> Hi Milan\n>\n> On 2025-11-28 02:49, Milan Zamazal wrote:\n>> Configuration validation computes the maximum size of all the requested\n>> streams and compares it to the output sizes. When e.g. only a raw\n>> stream is requested then this may result in an invalid adjustment of its\n>> size. This is because the output sizes are computed for processed\n>> streams and may be smaller than capture sizes. If a raw stream with the\n>> capture size is requested, it may then be wrongly adjusted to a larger\n>> size because the output sizes, which are irrelevant for raw streams\n>> anyway, are smaller than the requested capture size.\n>> \n>\n> The problem is somewhere else and a quick look suggests that a wrong\n> pipeConfig is getting selected to start with.\n\nCould you elaborate?\n\n> Is this a bug with current series? Could you post steps to reproduce\n> this ?\n\nI guess it is related to the fix of selecting the right processed stream\nresolution that has been merged to master some time ago.\n\nIt can be reproduced by asking for a single raw stream with a capture\nresolution that is supported by the sensor but is lower than the maximum\ncapture resolution. For example, in case of imx219, when asking for\n1920x1080 resolution, a higher resolution 3280x246 is used instead,\nalthough 1920x1080 should be used.\n\n>> Let's fix the problem by tracking raw and processed streams maximum\n>> sizes separately and comparing raw stream sizes against capture rather\n>> than output sizes.\n>\n> This should ideally be squashed with the original validation patch, no ?\n> Introducing new patches at v16, is not going to help here. It just slows\n> things down for merging (unless there is a bug interaction happening\n> then a split fixup! would've helped for easier review).\n\nMaybe, but I don't like large patches and this change is easy to\nseparate. But I can squash it if you prefer.\n\n> I had problem with 8/9 as well as it should have been a separate patch\n> and not merged with this but I let it slide already. But you do get my\n> point of initially recommending to split out the colorspace patch (which\n> you ultimately did), it progressed and got merged out of this series -\n> but then you again had a fixup! to be put back in this series. This is\n> just unwanted hindrance on the reviewer's end.\n\nSorry about it, I'll remove that patch from v17.\n\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 23 ++++++++++++++++-------\n>> 1 file changed, 16 insertions(+), 7 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 2820d1254..bb67000e2 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -1153,12 +1153,18 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> \t}\n>> \n>> \t/* Find the largest stream size. */\n>> -\tSize maxStreamSize;\n>> -\tfor (const StreamConfiguration &cfg : config_)\n>> -\t\tmaxStreamSize.expandTo(cfg.size);\n>> +\tSize maxProcessedStreamSize;\n>> +\tSize maxRawStreamSize;\n>> +\tfor (const StreamConfiguration &cfg : config_) {\n>> +\t\tif (isRaw(cfg))\n>> +\t\t\tmaxRawStreamSize.expandTo(cfg.size);\n>> +\t\telse\n>> +\t\t\tmaxProcessedStreamSize.expandTo(cfg.size);\n>> +\t}\n>> \n>> \tLOG(SimplePipeline, Debug)\n>> -\t\t<< \"Largest stream size is \" << maxStreamSize;\n>> +\t\t<< \"Largest stream size is \"\n>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>\n> This will be a very confusing log to parse.\n\nI was thinking about putting \"processed\" and \"raw\" around but I didn't\nlike the output, so I left with the version above, it's only a debug\nmessage after all. But suggestions for improvement are welcome.\n\n>> \n>> \t/* Cap the number of raw stream configurations */\n>> \tunsigned int rawCount = 0;\n>> @@ -1217,8 +1223,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> \t\tconst Size &captureSize = pipeConfig->captureSize;\n>> \t\tconst Size &maxOutputSize = pipeConfig->outputSizes.max;\n>> \n>> -\t\tif (maxOutputSize.width >= maxStreamSize.width &&\n>> -\t\t maxOutputSize.height >= maxStreamSize.height) {\n>> +\t\tif (maxOutputSize.width >= maxProcessedStreamSize.width &&\n>> +\t\t maxOutputSize.height >= maxProcessedStreamSize.height &&\n>> +\t\t captureSize.width >= maxRawStreamSize.width &&\n>> +\t\t captureSize.height >= maxRawStreamSize.height) {\n>> \t\t\tif (!pipeConfig_ || captureSize < pipeConfig_->captureSize)\n>> \t\t\t\tpipeConfig_ = pipeConfig;\n>> \t\t}\n>> @@ -1236,7 +1244,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> \t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n>> \t\t<< \" -> \" << pipeConfig_->captureSize\n>> \t\t<< \"-\" << pipeConfig_->captureFormat\n>> -\t\t<< \" for max stream size \" << maxStreamSize;\n>> +\t\t<< \" for max stream size \"\n>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>> \n>> \t/*\n>> \t * Adjust the requested streams.","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 C5977BD80A\n\tfor ;\n\tMon, 1 Dec 2025 13:36:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D148860B30;\n\tMon, 1 Dec 2025 14:36:14 +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 406AC60A7B\n\tfor ;\n\tMon, 1 Dec 2025 14:36:12 +0100 (CET)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-464-kmRpj6NDOluvgrVHgbMZ_w-1; Mon, 01 Dec 2025 08:36:09 -0500","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-47775585257so29807655e9.1\n\tfor ;\n\tMon, 01 Dec 2025 05:36:09 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4790b0c44dcsm301741805e9.11.2025.12.01.05.36.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 01 Dec 2025 05:36:06 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"AnVmRgbx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1764596171;\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=t/Il6VO4JKVjRy6yuzCAEOyxxUzqBGFWV3SzX3DyHrw=;\n\tb=AnVmRgbxdfnw0D4RIrYlUOOJipkv/LB49jdPRY49fSgaHcuReTxv1kakDqe7QL0wbrg2KF\n\tzrQgB12Chv2oucELb0fryCXQOxVV3Ck8hSv4dapwj4Fm3mQ21OnRnmXV/1j0+x6VqaCEEr\n\tx+B3JrStRAQeortULrtZLKVfNYm2jFE=","X-MC-Unique":"kmRpj6NDOluvgrVHgbMZ_w-1","X-Mimecast-MFC-AGG-ID":"kmRpj6NDOluvgrVHgbMZ_w_1764596168","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1764596168; x=1765200968;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=t/Il6VO4JKVjRy6yuzCAEOyxxUzqBGFWV3SzX3DyHrw=;\n\tb=PTTpC2z5q1AOtmZ3xA93DNkSLYI9lt/NjAhFqb8oaX3fDQaoAz11g/9fZ+MSsaIuMI\n\txJs92nKif6vhMzEM9ztW0Z4V71u8Nx56fQ50/57mQjUDosprHm8Ok/vmdgZ6QRfmkSez\n\tU4Txh6bJC1D5Sblll3b+mB+WoAsvPhrLUQG0gkgEjm/A33WsN1/K85AmL1K306Bk9OFZ\n\tCoz1yZZRGX5QHqi45lIC4J7iP+Jqaz9/a+C/yp3+5GgV/WdLgCuPqBFFty1QmcPpjqbV\n\tdSSlUzrhB2PY3OiRFJl8xu3P3ZUbiO56WhnAwbPbbsf7PcDI7ecwk58g2aJSldatE8cA\n\t/VFg==","X-Gm-Message-State":"AOJu0Yzy1nU8gWDUWHDzvEDhhbe1O9s37ujWF4wcxdK8XQ8mn1yEjVvk\n\tz3viSOWJvfwVbxxW+u1kHQEULNXzuBs/KCQZFUAIunkccZjCAMklnj4fOdtOsXDV6IXQ/4TsCVd\n\tp7bjSp88lat1PsnWRInQ0AOxFm3v7jheng90LPoVO0GoxDO9tCLHcebwoFI/XkZt1WBwQQrNJnW\n\t0=","X-Gm-Gg":"ASbGncvvk8Ut2eH3nM21RUWrxkawVBmJ7U/Rwh/IxE9dkLMCBaydAwqyraRWbcz6LUK\n\thRDo2L7ECVqWCF9kESnQpzEV/p4zBNCCbNCdVgGEoZ2gqsNOIr6c45A32uWTrioRhER0QTA6PKh\n\tuWTaXaivAzuG1GFIK1juBewBwgq/KlPPHTmFUMjrqXDcS8WTU4KkTfxtIJZrpEyIxbIy3Qy33MZ\n\tG8FBFzDYrXvugCpOOl47boGd4RCwMgpj5yOqjdPAJlBB84fPY/a2uD5l30Ti/InwzibWU2RPomb\n\tEIl+8BVI2PKKlLZVWaNAee+AfCLQUtjdos82J3MJEG3+BKQ587cOIzH/LzRcFIAfhBPmDtuGpD4\n\tUbfC6znyzmgOYj+bAcJxNiM7pVrBMjEw6uT37ZRlyL96F9GfjHvNQD94ukbM7lTc=","X-Received":["by 2002:a05:600c:1909:b0:477:7b16:5f9f with SMTP id\n\t5b1f17b1804b1-477c11337e0mr416262265e9.31.1764596167752; \n\tMon, 01 Dec 2025 05:36:07 -0800 (PST)","by 2002:a05:600c:1909:b0:477:7b16:5f9f with SMTP id\n\t5b1f17b1804b1-477c11337e0mr416261805e9.31.1764596167293; \n\tMon, 01 Dec 2025 05:36:07 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHVQ44RPj5XrwyV21EE8bqXgIAK0Z6vvafXEJ/oLAdSBdXOsmzkQ/SymfWID7ymVrN7lJdk0w==","From":"Milan Zamazal ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","In-Reply-To":" (Umang Jain's\n\tmessage of \"Fri, 28 Nov 2025 23:15:42 +0530\")","References":"<20251127211932.122463-1-mzamazal@redhat.com>\n\t<20251127211932.122463-10-mzamazal@redhat.com>\n\t","Date":"Mon, 01 Dec 2025 14:36:05 +0100","Message-ID":"<85tsyal3y2.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"YtLojLINAfLIQpEiv-3C46etcz1LSTLslOeoiCRXBnc_1764596168","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":37142,"web_url":"https://patchwork.libcamera.org/comment/37142/","msgid":"<0ca812330cdd544c81c812e5dee85076@igalia.com>","date":"2025-12-01T14:29:09","subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2025-12-01 19:06, Milan Zamazal wrote:\n> Hi Umang,\n> \n> thank you for review.\n> \n> Umang Jain writes:\n> \n>> Hi Milan\n>>\n>> On 2025-11-28 02:49, Milan Zamazal wrote:\n>>> Configuration validation computes the maximum size of all the requested\n>>> streams and compares it to the output sizes. When e.g. only a raw\n>>> stream is requested then this may result in an invalid adjustment of its\n>>> size. This is because the output sizes are computed for processed\n>>> streams and may be smaller than capture sizes. If a raw stream with the\n>>> capture size is requested, it may then be wrongly adjusted to a larger\n>>> size because the output sizes, which are irrelevant for raw streams\n>>> anyway, are smaller than the requested capture size.\n>>> \n>>\n>> The problem is somewhere else and a quick look suggests that a wrong\n>> pipeConfig is getting selected to start with.\n> \n> Could you elaborate?\n> \n>> Is this a bug with current series? Could you post steps to reproduce\n>> this ?\n> \n> I guess it is related to the fix of selecting the right processed stream\n> resolution that has been merged to master some time ago.\n> \n> It can be reproduced by asking for a single raw stream with a capture\n> resolution that is supported by the sensor but is lower than the maximum\n> capture resolution. For example, in case of imx219, when asking for\n> 1920x1080 resolution, a higher resolution 3280x246 is used instead,\n> although 1920x1080 should be used.\n\nI quickly checked on my earlier implementation, I do not face this\nissue:\n\n```\nuajain1@uajain:~$ ./libcamera/build/src/apps/cam/cam -c1\n-srole=raw,width=1920,height=1080 -C\n[0:35:56.822871937] [451] INFO IPAManager ipa_manager.cpp:137 libcamera\nis not installed. Adding '/home/uajain1/libcamera/build/src/ipa' to the\nIPA search path\n[0:35:56.846769072] [451] INFO Camera camera_manager.cpp:326 libcamera\nv0.5.1+64-eba4ccc8\n[0:35:56.942874228] [452] INFO IPAProxy ipa_proxy.cpp:151 libcamera is\nnot installed. Loading IPA configuration from\n'/home/uajain1/libcamera/src/ipa/simple/data'\n[0:35:56.943061676] [452] WARN IPAProxy ipa_proxy.cpp:177 Configuration\nfile 'imx219.yaml' not found for IPA module 'simple', falling back to\n'uncalibrated.yaml'\n[0:35:56.943659124] [452] INFO IPAProxy ipa_proxy.cpp:151 libcamera is\nnot installed. Loading IPA configuration from\n'/home/uajain1/libcamera/src/ipa/simple/data'\nUsing camera /base/soc/i2c0mux/i2c@1/imx219@10 as cam0\n[0:35:56.950627718] [451] INFO Camera camera.cpp:1205 configuring\nstreams: (0) 1920x1080-SRGGB10/RAW\n[0:35:56.951063708] [452] INFO SimplePipeline simple.cpp:1479\ndata->useConversion_ : 0\ncam0: Capture until user interrupts by SIGINT\n2157.040698 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 4147200\n2157.074024 (30.01 fps) cam0-stream0 seq: 000001 bytesused: 4147200\n2157.107350 (30.01 fps) cam0-stream0 seq: 000002 bytesused: 4147200\n2157.140677 (30.01 fps) cam0-stream0 seq: 000003 bytesused: 4147200\n2157.174004 (30.01 fps) cam0-stream0 seq: 000004 bytesused: 4147200\n2157.207331 (30.01 fps) cam0-stream0 seq: 000005 bytesused: 4147200\n...\n```\n\nSo probably I'll take a look deeper in this week to see what's really\ngoing with the pipeConfig selection.\n\n> \n>>> Let's fix the problem by tracking raw and processed streams maximum\n>>> sizes separately and comparing raw stream sizes against capture rather\n>>> than output sizes.\n>>\n>> This should ideally be squashed with the original validation patch, no ?\n>> Introducing new patches at v16, is not going to help here. It just slows\n>> things down for merging (unless there is a bug interaction happening\n>> then a split fixup! would've helped for easier review).\n> \n> Maybe, but I don't like large patches and this change is easy to\n> separate. But I can squash it if you prefer.\n> \n>> I had problem with 8/9 as well as it should have been a separate patch\n>> and not merged with this but I let it slide already. But you do get my\n>> point of initially recommending to split out the colorspace patch (which\n>> you ultimately did), it progressed and got merged out of this series -\n>> but then you again had a fixup! to be put back in this series. This is\n>> just unwanted hindrance on the reviewer's end.\n> \n> Sorry about it, I'll remove that patch from v17.\n> \n>>> Signed-off-by: Milan Zamazal \n>>> ---\n>>> src/libcamera/pipeline/simple/simple.cpp | 23 ++++++++++++++++-------\n>>> 1 file changed, 16 insertions(+), 7 deletions(-)\n>>> \n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 2820d1254..bb67000e2 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -1153,12 +1153,18 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>> \t}\n>>> \n>>> \t/* Find the largest stream size. */\n>>> -\tSize maxStreamSize;\n>>> -\tfor (const StreamConfiguration &cfg : config_)\n>>> -\t\tmaxStreamSize.expandTo(cfg.size);\n>>> +\tSize maxProcessedStreamSize;\n>>> +\tSize maxRawStreamSize;\n>>> +\tfor (const StreamConfiguration &cfg : config_) {\n>>> +\t\tif (isRaw(cfg))\n>>> +\t\t\tmaxRawStreamSize.expandTo(cfg.size);\n>>> +\t\telse\n>>> +\t\t\tmaxProcessedStreamSize.expandTo(cfg.size);\n>>> +\t}\n>>> \n>>> \tLOG(SimplePipeline, Debug)\n>>> -\t\t<< \"Largest stream size is \" << maxStreamSize;\n>>> +\t\t<< \"Largest stream size is \"\n>>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>>\n>> This will be a very confusing log to parse.\n> \n> I was thinking about putting \"processed\" and \"raw\" around but I didn't\n> like the output, so I left with the version above, it's only a debug\n> message after all. But suggestions for improvement are welcome.\n> \n>>> \n>>> \t/* Cap the number of raw stream configurations */\n>>> \tunsigned int rawCount = 0;\n>>> @@ -1217,8 +1223,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>> \t\tconst Size &captureSize = pipeConfig->captureSize;\n>>> \t\tconst Size &maxOutputSize = pipeConfig->outputSizes.max;\n>>> \n>>> -\t\tif (maxOutputSize.width >= maxStreamSize.width &&\n>>> -\t\t maxOutputSize.height >= maxStreamSize.height) {\n>>> +\t\tif (maxOutputSize.width >= maxProcessedStreamSize.width &&\n>>> +\t\t maxOutputSize.height >= maxProcessedStreamSize.height &&\n>>> +\t\t captureSize.width >= maxRawStreamSize.width &&\n>>> +\t\t captureSize.height >= maxRawStreamSize.height) {\n>>> \t\t\tif (!pipeConfig_ || captureSize < pipeConfig_->captureSize)\n>>> \t\t\t\tpipeConfig_ = pipeConfig;\n>>> \t\t}\n>>> @@ -1236,7 +1244,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>> \t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n>>> \t\t<< \" -> \" << pipeConfig_->captureSize\n>>> \t\t<< \"-\" << pipeConfig_->captureFormat\n>>> -\t\t<< \" for max stream size \" << maxStreamSize;\n>>> +\t\t<< \" for max stream size \"\n>>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>>> \n>>> \t/*\n>>> \t * Adjust the requested streams.","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 3F26FC0F1B\n\tfor ;\n\tMon, 1 Dec 2025 14:29:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F42560B37;\n\tMon, 1 Dec 2025 15:29:16 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D29D360A7B\n\tfor ;\n\tMon, 1 Dec 2025 15:29:13 +0100 (CET)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vQ4ts-007PlL-DL; Mon, 01 Dec 2025 15:29:12 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vQ4tq-002LWM-4P; Mon, 01 Dec 2025 15:29:12 +0100","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from )\n\tid 1vQ4tp-003ftC-28; Mon, 01 Dec 2025 15:29:09 +0100"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"AKZB0yeL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=YjrReiUImJluBVU3Uj9rg8Zi1My3rWZcG/h7h9VZbw0=;\n\tb=AKZB0yeLwmoMdYXSQVxbGuKT/u\n\tmJUbTT6cNmXTqD/vMYeAD2EGvobRSWpV9FhOyaQDIVRONCE1b6GJUQiJY9by1mIJeWdzS4phxWVRv\n\tVI3NmNuM4Vl5TY5zp+cAtLcMOEDfpTn6K5DiVCMczpGBWJHVNLC+4drYeIa18vbkrcth2ZrG5/AXT\n\t2bctYTP7Y33Ljd8bTfKCXK8E/eg0PrNc/akEdHp4on68glaLQOfx8Gal6GWbgVmv1SDiddsxOaONT\n\tlmHrmCTaLMSDDhdRh+skxhm/FpRUFrKEyL77QD9Dp78UC1334Qj0fr+cNqEtWodf2JaH/F6yoC6sQ\n\txySYps9w==;","MIME-Version":"1.0","Date":"Mon, 01 Dec 2025 19:59:09 +0530","From":"Umang Jain ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ,\n\tPaul Elder ","Subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","In-Reply-To":"<85tsyal3y2.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20251127211932.122463-1-mzamazal@redhat.com>\n\t<20251127211932.122463-10-mzamazal@redhat.com>\n\t\n\t<85tsyal3y2.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Message-ID":"<0ca812330cdd544c81c812e5dee85076@igalia.com>","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Spam-Report":"NO, Score=-3.4, Tests=ALL_TRUSTED=-3, AWL=-1.159, BAYES_50=0.8,\n\tURIBL_BLOCKED=0.001","X-Spam-Score":"-33","X-Spam-Bar":"---","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":37143,"web_url":"https://patchwork.libcamera.org/comment/37143/","msgid":"<85pl8yl148.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-12-01T14:37:11","subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Umang Jain writes:\n\n> On 2025-12-01 19:06, Milan Zamazal wrote:\n>> Hi Umang,\n>> \n>\n>> thank you for review.\n>> \n>> Umang Jain writes:\n>> \n>>> Hi Milan\n>>>\n>>> On 2025-11-28 02:49, Milan Zamazal wrote:\n>>>> Configuration validation computes the maximum size of all the requested\n>>>> streams and compares it to the output sizes. When e.g. only a raw\n>>>> stream is requested then this may result in an invalid adjustment of its\n>>>> size. This is because the output sizes are computed for processed\n>>>> streams and may be smaller than capture sizes. If a raw stream with the\n>>>> capture size is requested, it may then be wrongly adjusted to a larger\n>>>> size because the output sizes, which are irrelevant for raw streams\n>>>> anyway, are smaller than the requested capture size.\n>>>> \n>>>\n>>> The problem is somewhere else and a quick look suggests that a wrong\n>>> pipeConfig is getting selected to start with.\n>> \n>> Could you elaborate?\n>> \n>>> Is this a bug with current series? Could you post steps to reproduce\n>>> this ?\n>> \n>> I guess it is related to the fix of selecting the right processed stream\n>> resolution that has been merged to master some time ago.\n>> \n>> It can be reproduced by asking for a single raw stream with a capture\n>> resolution that is supported by the sensor but is lower than the maximum\n>> capture resolution. For example, in case of imx219, when asking for\n>> 1920x1080 resolution, a higher resolution 3280x246 is used instead,\n>> although 1920x1080 should be used.\n>\n> I quickly checked on my earlier implementation, I do not face this\n> issue:\n\nDid you rebase on current master?\n\n>\n> ```\n> uajain1@uajain:~$ ./libcamera/build/src/apps/cam/cam -c1\n> -srole=raw,width=1920,height=1080 -C\n> [0:35:56.822871937] [451] INFO IPAManager ipa_manager.cpp:137 libcamera\n> is not installed. Adding '/home/uajain1/libcamera/build/src/ipa' to the\n> IPA search path\n> [0:35:56.846769072] [451] INFO Camera camera_manager.cpp:326 libcamera\n> v0.5.1+64-eba4ccc8\n> [0:35:56.942874228] [452] INFO IPAProxy ipa_proxy.cpp:151 libcamera is\n> not installed. Loading IPA configuration from\n> '/home/uajain1/libcamera/src/ipa/simple/data'\n> [0:35:56.943061676] [452] WARN IPAProxy ipa_proxy.cpp:177 Configuration\n> file 'imx219.yaml' not found for IPA module 'simple', falling back to\n> 'uncalibrated.yaml'\n> [0:35:56.943659124] [452] INFO IPAProxy ipa_proxy.cpp:151 libcamera is\n> not installed. Loading IPA configuration from\n> '/home/uajain1/libcamera/src/ipa/simple/data'\n> Using camera /base/soc/i2c0mux/i2c@1/imx219@10 as cam0\n> [0:35:56.950627718] [451] INFO Camera camera.cpp:1205 configuring\n> streams: (0) 1920x1080-SRGGB10/RAW\n> [0:35:56.951063708] [452] INFO SimplePipeline simple.cpp:1479\n> data->useConversion_ : 0\n> cam0: Capture until user interrupts by SIGINT\n> 2157.040698 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 4147200\n> 2157.074024 (30.01 fps) cam0-stream0 seq: 000001 bytesused: 4147200\n> 2157.107350 (30.01 fps) cam0-stream0 seq: 000002 bytesused: 4147200\n> 2157.140677 (30.01 fps) cam0-stream0 seq: 000003 bytesused: 4147200\n> 2157.174004 (30.01 fps) cam0-stream0 seq: 000004 bytesused: 4147200\n> 2157.207331 (30.01 fps) cam0-stream0 seq: 000005 bytesused: 4147200\n> ...\n> ```\n>\n> So probably I'll take a look deeper in this week to see what's really\n> going with the pipeConfig selection.\n\nOK, thank you.\n\n>> \n>>>> Let's fix the problem by tracking raw and processed streams maximum\n>>>> sizes separately and comparing raw stream sizes against capture rather\n>>>> than output sizes.\n>>>\n>>> This should ideally be squashed with the original validation patch, no ?\n>>> Introducing new patches at v16, is not going to help here. It just slows\n>>> things down for merging (unless there is a bug interaction happening\n>>> then a split fixup! would've helped for easier review).\n>> \n>> Maybe, but I don't like large patches and this change is easy to\n>> separate. But I can squash it if you prefer.\n>> \n>>> I had problem with 8/9 as well as it should have been a separate patch\n>>> and not merged with this but I let it slide already. But you do get my\n>>> point of initially recommending to split out the colorspace patch (which\n>>> you ultimately did), it progressed and got merged out of this series -\n>>> but then you again had a fixup! to be put back in this series. This is\n>>> just unwanted hindrance on the reviewer's end.\n>> \n>> Sorry about it, I'll remove that patch from v17.\n>> \n>>>> Signed-off-by: Milan Zamazal \n>>>> ---\n>>>> src/libcamera/pipeline/simple/simple.cpp | 23 ++++++++++++++++-------\n>>>> 1 file changed, 16 insertions(+), 7 deletions(-)\n>>>> \n>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>>> index 2820d1254..bb67000e2 100644\n>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>> @@ -1153,12 +1153,18 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>> \t}\n>>>> \n>>>> \t/* Find the largest stream size. */\n>>>> -\tSize maxStreamSize;\n>>>> -\tfor (const StreamConfiguration &cfg : config_)\n>>>> -\t\tmaxStreamSize.expandTo(cfg.size);\n>>>> +\tSize maxProcessedStreamSize;\n>>>> +\tSize maxRawStreamSize;\n>>>> +\tfor (const StreamConfiguration &cfg : config_) {\n>>>> +\t\tif (isRaw(cfg))\n>>>> +\t\t\tmaxRawStreamSize.expandTo(cfg.size);\n>>>> +\t\telse\n>>>> +\t\t\tmaxProcessedStreamSize.expandTo(cfg.size);\n>>>> +\t}\n>>>> \n>>>> \tLOG(SimplePipeline, Debug)\n>>>> -\t\t<< \"Largest stream size is \" << maxStreamSize;\n>>>> +\t\t<< \"Largest stream size is \"\n>>>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>>>\n>>> This will be a very confusing log to parse.\n>> \n>> I was thinking about putting \"processed\" and \"raw\" around but I didn't\n>> like the output, so I left with the version above, it's only a debug\n>> message after all. But suggestions for improvement are welcome.\n>> \n>>>> \n>>>> \t/* Cap the number of raw stream configurations */\n>>>> \tunsigned int rawCount = 0;\n>>>> @@ -1217,8 +1223,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>> \t\tconst Size &captureSize = pipeConfig->captureSize;\n>>>> \t\tconst Size &maxOutputSize = pipeConfig->outputSizes.max;\n>>>> \n>>>> -\t\tif (maxOutputSize.width >= maxStreamSize.width &&\n>>>> -\t\t maxOutputSize.height >= maxStreamSize.height) {\n>>>> +\t\tif (maxOutputSize.width >= maxProcessedStreamSize.width &&\n>>>> +\t\t maxOutputSize.height >= maxProcessedStreamSize.height &&\n>>>> +\t\t captureSize.width >= maxRawStreamSize.width &&\n>>>> +\t\t captureSize.height >= maxRawStreamSize.height) {\n>>>> \t\t\tif (!pipeConfig_ || captureSize < pipeConfig_->captureSize)\n>>>> \t\t\t\tpipeConfig_ = pipeConfig;\n>>>> \t\t}\n>>>> @@ -1236,7 +1244,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>> \t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n>>>> \t\t<< \" -> \" << pipeConfig_->captureSize\n>>>> \t\t<< \"-\" << pipeConfig_->captureFormat\n>>>> -\t\t<< \" for max stream size \" << maxStreamSize;\n>>>> +\t\t<< \" for max stream size \"\n>>>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>>>> \n>>>> \t/*\n>>>> \t * Adjust the requested streams.","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 B2B4CBD80A\n\tfor ;\n\tMon, 1 Dec 2025 14:37:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A196160C67;\n\tMon, 1 Dec 2025 15:37:18 +0100 (CET)","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 9764560A7B\n\tfor ;\n\tMon, 1 Dec 2025 15:37:17 +0100 (CET)","from mail-wr1-f72.google.com (mail-wr1-f72.google.com\n\t[209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-329-C-i0wKQ4PvuRBpTpxehm1Q-1; Mon, 01 Dec 2025 09:37:14 -0500","by mail-wr1-f72.google.com with SMTP id\n\tffacd0b85a97d-42b2ad29140so2137259f8f.0\n\tfor ;\n\tMon, 01 Dec 2025 06:37:14 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-42e1caae37esm26474902f8f.40.2025.12.01.06.37.12\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 01 Dec 2025 06:37:12 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"BH4ZO2Uu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1764599836;\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=Vzt+yqH5HC1L9xXHj9ZsrlcC4W85DryyWiwS/OVXo/k=;\n\tb=BH4ZO2UuT+T6CP8Vj4z/InkMMJKtUKkVprnfqEmjfnjz1DIZCgnoCEgkmdoA/ggNAZ+5sP\n\tI7h1Row0YW4XWNXrzge2U36cyMJCcCqXUEvyUI+I8yH8MGj2hxNs7MJQzbxHX38wJEPE7i\n\tGaXlMxal+65YNoUKogXysk3p034mcLk=","X-MC-Unique":"C-i0wKQ4PvuRBpTpxehm1Q-1","X-Mimecast-MFC-AGG-ID":"C-i0wKQ4PvuRBpTpxehm1Q_1764599834","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1764599833; x=1765204633;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=Vzt+yqH5HC1L9xXHj9ZsrlcC4W85DryyWiwS/OVXo/k=;\n\tb=WkPTFVzS2XcGakzF3kawSb1J4ZSqlk/ETwRMN8wgoXrtAeiepBxgvb5UqS3rm7/0Tf\n\tiV/ke/BzIouZbmZJM/zfWM1t2rpiKg0r8w5DmRtiaGb408x+xrBf/IG/rOWI8IOagmPM\n\tfqy4UK8YuRtgk6mqqrFs0Akmifc/R8m2i0tJcCmzQdxvMs2bvWilPJN1BJYDmGZ3tSrJ\n\tN2iOA4eFPXO8a3kZ6cbNtg7Wsq3lpOu42DF3bMSAeEosjFznBTIuF8M4OrWi7zyrW4yY\n\tIlsgQ6Vh6koA4bPOlQw+RsoBC9i9pCNUkCyXw7y8dhrYIn76IbwgW0lA6xU/CS9ZuTRc\n\tj6Eg==","X-Gm-Message-State":"AOJu0Yx6xpT1kijtCSWeCq45Sd35SybsrgxKkWPfGv89Zwe2F/fJbwdP\n\tP5x/nRNtbAI3/400OWNYN1lb8CSIsayqjayFoZudDPcgLwtDRbn1inWEOjTKYqAinOqPtcMdXiO\n\tStCBJQtVvwmkNnSixwFNpnjIIbKplZGAfTuss0PPczlzH0Y7zcS1ybchU/SBOZLJQ5pVCq/PyKz\n\tk=","X-Gm-Gg":"ASbGncuGeibMET+WwgdwTGYqf/pecMToDWKtxOnY0RpnsTwLAJdhg2CQn5rdQPma1xi\n\toRVgR0jpW5YYgnPiMnnDW2slWGp1lxye2xoTdY34Dmle6rfVVdFUEOJ+Wkq+ZyOIi0ga5+3a7Be\n\txyXooHD+xW8Amnu64Ecl4KbykH9/AyPLdoro5UWio9cMPm7dMxfc/ugsN6lfH6t4d9Ul5siHxxZ\n\tG8poZLKUsPFYB1Bkc45IrlnhMnd5auup+TF8nzVjXYgGPlONSBCsIChpIwRoCx8wGm8ROQYtZWm\n\twByQIg8K4z9uJeWWU/MBOySvKgbqhicLM0PwcRGBHy1Dm+X7/BoJd2tlR6KvpNnaOkNrYRJASfD\n\te7aatMgJYWsxRRLydT2aLOhYjBxPH0/iI24HVJFaRDq6YT4iKh21uFu+0RtYg+7Q=","X-Received":["by 2002:a05:6000:2010:b0:429:c7dc:e613 with SMTP id\n\tffacd0b85a97d-42cc1d0c83cmr41611143f8f.41.1764599833463; \n\tMon, 01 Dec 2025 06:37:13 -0800 (PST)","by 2002:a05:6000:2010:b0:429:c7dc:e613 with SMTP id\n\tffacd0b85a97d-42cc1d0c83cmr41611108f8f.41.1764599832926; \n\tMon, 01 Dec 2025 06:37:12 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IGP4rIm0T8DWkz2N/N2rzQEN33DOgVHu2W2SXbYjdE1zm65Nm6/ncT6RO1noIKjpZGtDlpQnw==","From":"Milan Zamazal ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","In-Reply-To":"<0ca812330cdd544c81c812e5dee85076@igalia.com> (Umang Jain's\n\tmessage of \"Mon, 01 Dec 2025 19:59:09 +0530\")","References":"<20251127211932.122463-1-mzamazal@redhat.com>\n\t<20251127211932.122463-10-mzamazal@redhat.com>\n\t\n\t<85tsyal3y2.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<0ca812330cdd544c81c812e5dee85076@igalia.com>","Date":"Mon, 01 Dec 2025 15:37:11 +0100","Message-ID":"<85pl8yl148.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"vBiv_2RGR74lMM_CXB9cAeN5pBSfp7yxayJkwPN0evQ_1764599834","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":37177,"web_url":"https://patchwork.libcamera.org/comment/37177/","msgid":"","date":"2025-12-03T03:31:45","subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2025-12-01 20:07, Milan Zamazal wrote:\n> Umang Jain writes:\n> \n>> On 2025-12-01 19:06, Milan Zamazal wrote:\n>>> Hi Umang,\n>>> \n>>\n>>> thank you for review.\n>>> \n>>> Umang Jain writes:\n>>> \n>>>> Hi Milan\n>>>>\n>>>> On 2025-11-28 02:49, Milan Zamazal wrote:\n>>>>> Configuration validation computes the maximum size of all the requested\n>>>>> streams and compares it to the output sizes. When e.g. only a raw\n>>>>> stream is requested then this may result in an invalid adjustment of its\n>>>>> size. This is because the output sizes are computed for processed\n>>>>> streams and may be smaller than capture sizes. If a raw stream with the\n>>>>> capture size is requested, it may then be wrongly adjusted to a larger\n>>>>> size because the output sizes, which are irrelevant for raw streams\n>>>>> anyway, are smaller than the requested capture size.\n>>>>> \n>>>>\n>>>> The problem is somewhere else and a quick look suggests that a wrong\n>>>> pipeConfig is getting selected to start with.\n>>> \n>>> Could you elaborate?\n>>> \n>>>> Is this a bug with current series? Could you post steps to reproduce\n>>>> this ?\n>>> \n>>> I guess it is related to the fix of selecting the right processed stream\n>>> resolution that has been merged to master some time ago.\n>>> \n>>> It can be reproduced by asking for a single raw stream with a capture\n>>> resolution that is supported by the sensor but is lower than the maximum\n>>> capture resolution. For example, in case of imx219, when asking for\n>>> 1920x1080 resolution, a higher resolution 3280x246 is used instead,\n>>> although 1920x1080 should be used.\n>>\n>> I quickly checked on my earlier implementation, I do not face this\n>> issue:\n> \n> Did you rebase on current master?\n\nAck, this was it, some changes led to due interaction with Commit\n94d32fdc55a3d (pipeline: simple: Consider output sizes when choosing\npipe config), which was missing on my non-rebased branch (and I also\ncompletely forgot about it).\n\n> \n>>\n>> ```\n>> uajain1@uajain:~$ ./libcamera/build/src/apps/cam/cam -c1\n>> -srole=raw,width=1920,height=1080 -C\n>> [0:35:56.822871937] [451] INFO IPAManager ipa_manager.cpp:137 libcamera\n>> is not installed. Adding '/home/uajain1/libcamera/build/src/ipa' to the\n>> IPA search path\n>> [0:35:56.846769072] [451] INFO Camera camera_manager.cpp:326 libcamera\n>> v0.5.1+64-eba4ccc8\n>> [0:35:56.942874228] [452] INFO IPAProxy ipa_proxy.cpp:151 libcamera is\n>> not installed. Loading IPA configuration from\n>> '/home/uajain1/libcamera/src/ipa/simple/data'\n>> [0:35:56.943061676] [452] WARN IPAProxy ipa_proxy.cpp:177 Configuration\n>> file 'imx219.yaml' not found for IPA module 'simple', falling back to\n>> 'uncalibrated.yaml'\n>> [0:35:56.943659124] [452] INFO IPAProxy ipa_proxy.cpp:151 libcamera is\n>> not installed. Loading IPA configuration from\n>> '/home/uajain1/libcamera/src/ipa/simple/data'\n>> Using camera /base/soc/i2c0mux/i2c@1/imx219@10 as cam0\n>> [0:35:56.950627718] [451] INFO Camera camera.cpp:1205 configuring\n>> streams: (0) 1920x1080-SRGGB10/RAW\n>> [0:35:56.951063708] [452] INFO SimplePipeline simple.cpp:1479\n>> data->useConversion_ : 0\n>> cam0: Capture until user interrupts by SIGINT\n>> 2157.040698 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 4147200\n>> 2157.074024 (30.01 fps) cam0-stream0 seq: 000001 bytesused: 4147200\n>> 2157.107350 (30.01 fps) cam0-stream0 seq: 000002 bytesused: 4147200\n>> 2157.140677 (30.01 fps) cam0-stream0 seq: 000003 bytesused: 4147200\n>> 2157.174004 (30.01 fps) cam0-stream0 seq: 000004 bytesused: 4147200\n>> 2157.207331 (30.01 fps) cam0-stream0 seq: 000005 bytesused: 4147200\n>> ...\n>> ```\n>>\n>> So probably I'll take a look deeper in this week to see what's really\n>> going with the pipeConfig selection.\n> \n> OK, thank you.\n> \n>>> \n>>>>> Let's fix the problem by tracking raw and processed streams maximum\n>>>>> sizes separately and comparing raw stream sizes against capture rather\n>>>>> than output sizes.\n>>>>\n>>>> This should ideally be squashed with the original validation patch, no ?\n>>>> Introducing new patches at v16, is not going to help here. It just slows\n>>>> things down for merging (unless there is a bug interaction happening\n>>>> then a split fixup! would've helped for easier review).\n>>> \n>>> Maybe, but I don't like large patches and this change is easy to\n>>> separate. But I can squash it if you prefer.\n>>> \n>>>> I had problem with 8/9 as well as it should have been a separate patch\n>>>> and not merged with this but I let it slide already. But you do get my\n>>>> point of initially recommending to split out the colorspace patch (which\n>>>> you ultimately did), it progressed and got merged out of this series -\n>>>> but then you again had a fixup! to be put back in this series. This is\n>>>> just unwanted hindrance on the reviewer's end.\n>>> \n>>> Sorry about it, I'll remove that patch from v17.\n>>> \n>>>>> Signed-off-by: Milan Zamazal \n>>>>> ---\n>>>>> src/libcamera/pipeline/simple/simple.cpp | 23 ++++++++++++++++-------\n>>>>> 1 file changed, 16 insertions(+), 7 deletions(-)\n>>>>> \n>>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>>>> index 2820d1254..bb67000e2 100644\n>>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>>> @@ -1153,12 +1153,18 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>>> \t}\n>>>>> \n>>>>> \t/* Find the largest stream size. */\n\ns/size/sizes/\n\n>>>>> -\tSize maxStreamSize;\n>>>>> -\tfor (const StreamConfiguration &cfg : config_)\n>>>>> -\t\tmaxStreamSize.expandTo(cfg.size);\n>>>>> +\tSize maxProcessedStreamSize;\n>>>>> +\tSize maxRawStreamSize;\n>>>>> +\tfor (const StreamConfiguration &cfg : config_) {\n>>>>> +\t\tif (isRaw(cfg))\n>>>>> +\t\t\tmaxRawStreamSize.expandTo(cfg.size);\n>>>>> +\t\telse\n>>>>> +\t\t\tmaxProcessedStreamSize.expandTo(cfg.size);\n>>>>> +\t}\n>>>>> \n>>>>> \tLOG(SimplePipeline, Debug)\n>>>>> -\t\t<< \"Largest stream size is \" << maxStreamSize;\n>>>>> +\t\t<< \"Largest stream size is \"\n>>>>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>>>>\n>>>> This will be a very confusing log to parse.\n>>> \n>>> I was thinking about putting \"processed\" and \"raw\" around but I didn't\n>>> like the output, so I left with the version above, it's only a debug\n>>> message after all. But suggestions for improvement are welcome.\n\nIMO, it is completely fine to see:\n Largest processed stream size \n Largest RAW stream size \n\nThe problem with our version is that, it is only printing not the\nformat (this makes differentiation between raw/non-raw). So it's\nconfusing if someone is parsing the debug log with as the only\nparameter.\n\nRest now looks good to me. Probably this can be squashed with validation\npatch.\n\n>>> \n>>>>> \n>>>>> \t/* Cap the number of raw stream configurations */\n>>>>> \tunsigned int rawCount = 0;\n>>>>> @@ -1217,8 +1223,10 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>>> \t\tconst Size &captureSize = pipeConfig->captureSize;\n>>>>> \t\tconst Size &maxOutputSize = pipeConfig->outputSizes.max;\n>>>>> \n>>>>> -\t\tif (maxOutputSize.width >= maxStreamSize.width &&\n>>>>> -\t\t maxOutputSize.height >= maxStreamSize.height) {\n>>>>> +\t\tif (maxOutputSize.width >= maxProcessedStreamSize.width &&\n>>>>> +\t\t maxOutputSize.height >= maxProcessedStreamSize.height &&\n>>>>> +\t\t captureSize.width >= maxRawStreamSize.width &&\n>>>>> +\t\t captureSize.height >= maxRawStreamSize.height) {\n>>>>> \t\t\tif (!pipeConfig_ || captureSize < pipeConfig_->captureSize)\n>>>>> \t\t\t\tpipeConfig_ = pipeConfig;\n>>>>> \t\t}\n>>>>> @@ -1236,7 +1244,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>>> \t\t<< V4L2SubdeviceFormat{ pipeConfig_->code, pipeConfig_->sensorSize, {} }\n>>>>> \t\t<< \" -> \" << pipeConfig_->captureSize\n>>>>> \t\t<< \"-\" << pipeConfig_->captureFormat\n>>>>> -\t\t<< \" for max stream size \" << maxStreamSize;\n>>>>> +\t\t<< \" for max stream size \"\n>>>>> +\t\t<< maxProcessedStreamSize << \"/\" << maxRawStreamSize;\n>>>>> \n>>>>> \t/*\n>>>>> \t * Adjust the requested streams.","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 A786ABD80A\n\tfor ;\n\tWed, 3 Dec 2025 03:31:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6F4260E4C;\n\tWed, 3 Dec 2025 04:31:51 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 79DC3606D5\n\tfor ;\n\tWed, 3 Dec 2025 04:31:49 +0100 (CET)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vQdam-0086kR-GC; Wed, 03 Dec 2025 04:31:48 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vQdak-004WPh-6L; Wed, 03 Dec 2025 04:31:48 +0100","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from )\n\tid 1vQdaj-0041q7-2L; Wed, 03 Dec 2025 04:31:46 +0100"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"U6Uvif+z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=BcBVXYEuQKX+GTPpbbINkOOjLcEgeN5N8PdHc+Lh9tI=;\n\tb=U6Uvif+zqOlWXxIcEBYPwhf4A3\n\tPxEaHaIxjJC+287wN4kwj0rkiKYAyZkLgpptDy+Q6KkKIoihEIVw+5N7tBYLesbPYLOzvq+4OPNaD\n\t4UgGmRk+oQS8V15/5aHx4WiR/mImZGZIPrR4QQSCK2+xRP3KSVFuAcPQy71gliHcBB0xSXV5gUa5D\n\tKFVhQyLLuPQmRgi4dBKQAPSdk0nITV62FngYHgNjbky3gSEgSWXNcF7RW0xxoXGiQ4Krrj94jRy4l\n\t2Zvh/C1lbaIuVgfyY4p4Bp/u4p//Z6MRKj3Qk7tH+ot2JDIFXd6G2lK85t5+dO0XnmqNd9dlhkOKT\n\ttmyLHLnw==;","MIME-Version":"1.0","Date":"Wed, 03 Dec 2025 09:01:45 +0530","From":"Umang Jain ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ,\n\tPaul Elder ","Subject":"Re: [PATCH v16 9/9] libcamera: simple: Compute separate max stream\n\tsizes","In-Reply-To":"<85pl8yl148.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20251127211932.122463-1-mzamazal@redhat.com>\n\t<20251127211932.122463-10-mzamazal@redhat.com>\n\t\n\t<85tsyal3y2.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<0ca812330cdd544c81c812e5dee85076@igalia.com>\n\t<85pl8yl148.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Message-ID":"","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Spam-Report":"NO, Score=-3.6, Tests=ALL_TRUSTED=-3,AWL=-1.366,BAYES_50=0.8","X-Spam-Score":"-35","X-Spam-Bar":"---","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\" "}}]