[{"id":33962,"web_url":"https://patchwork.libcamera.org/comment/33962/","msgid":"","date":"2025-04-18T06:56:40","subject":"Re: [PATCH v4 05/11] libcamera: simple: Add plain output\n\tconfigurations","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patches.\n\nOn Mon, Apr 07, 2025 at 10:56:31AM +0200, Milan Zamazal wrote:\n> Output configurations in simple pipeline are added depending on whether\n> a converter, software ISP or none of them are used. If a converter or\n> software ISP is used, no raw output configurations are added.\n> \n> In order to support raw output at least with software ISP, let's always\n> add raw output configurations. A flag is added to\n> SimpleCameraData::Configuration indicating whether it's for a raw or a\n> converted output. We later filter formats and output sizes for\n> particular stream configurations according to the new configuration\n> flag.\n> \n> This is just preparation and raw output is still not supported. At the\n> moment, we simply filter out raw configurations unconditionally to keep\n> the current code working; this will be changed in followup patches.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 31 +++++++++++++++---------\n> 1 file changed, 19 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 60b7e1b1..82d7a9a5 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -319,6 +319,7 @@ public:\n> \t\tSize captureSize;\n> \t\tstd::vector outputFormats;\n> \t\tSizeRange outputSizes;\n> +\t\tbool raw;\n> \t};\n> \n> \tstd::vector streams_;\n> @@ -708,27 +709,33 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n> \t\t\tcontinue;\n> \n> \t\tConfiguration config;\n> +\n> +\t\t/* Raw configuration */\n> \t\tconfig.code = code;\n> \t\tconfig.sensorSize = size;\n> \t\tconfig.captureFormat = pixelFormat;\n> \t\tconfig.captureSize = format.size;\n> +\t\tconfig.outputFormats = { pixelFormat };\n> +\t\tconfig.outputSizes = config.captureSize;\n> +\t\tconfig.raw = true;\n> +\t\tconfigs_.push_back(config);\n> \n> +\t\t/* Modified, non-raw, configuration */\n> +\t\tconfig.raw = false;\n> \t\tif (converter_) {\n> \t\t\tconfig.outputFormats = converter_->formats(pixelFormat);\n> \t\t\tconfig.outputSizes = converter_->sizes(format.size);\n> \t\t} else if (swIsp_) {\n> -\t\t\tconfig.outputFormats = swIsp_->formats(pixelFormat);\n> -\t\t\tconfig.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n> -\t\t\tif (config.outputFormats.empty()) {\n> +\t\t\tstd::vector outputFormats = swIsp_->formats(pixelFormat);\n> +\t\t\tif (outputFormats.empty()) {\n> \t\t\t\t/* Do not use swIsp for unsupported pixelFormat's. */\n> -\t\t\t\tconfig.outputFormats = { pixelFormat };\n> -\t\t\t\tconfig.outputSizes = config.captureSize;\n> +\t\t\t\tcontinue;\n> \t\t\t}\n> +\t\t\tconfig.outputFormats = outputFormats;\n> +\t\t\tconfig.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n> \t\t} else {\n> -\t\t\tconfig.outputFormats = { pixelFormat };\n> -\t\t\tconfig.outputSizes = config.captureSize;\n> +\t\t\tcontinue;\n> \t\t}\n> -\n> \t\tconfigs_.push_back(config);\n> \t}\n> }\n> @@ -1301,10 +1308,10 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span \t/* Create the formats map. */\n> \tstd::map> formats;\n> \n> -\tfor (const SimpleCameraData::Configuration &cfg : data->configs_) {\n> -\t\tfor (PixelFormat format : cfg.outputFormats)\n> -\t\t\tformats[format].push_back(cfg.outputSizes);\n> -\t}\n> +\tfor (const SimpleCameraData::Configuration &cfg : data->configs_)\n> +\t\tif (!cfg.raw)\n> +\t\t\tfor (PixelFormat format : cfg.outputFormats)\n> +\t\t\t\tformats[format].push_back(cfg.outputSizes);\n\nThis can break bisection (at runtime), since if the pipeline only\nsupports raw formats then !cfg.raw will never be satisfied and formats\nwill be empty, thus later on causing\n\n\tfor (StreamRole role : roles) {\n\t\tStreamConfiguration cfg{ StreamFormats{ formats } };\n\t\tcfg.pixelFormat = formats.begin()->first;\n\t\tcfg.size = formats.begin()->second[0].max; <-- this\n\nthis to segfault.\n\nI see that later on in the series this is resolved but I'd prefer\navoiding breaking bisection.\n\n\nThanks,\n\nPaul\n\n> \n> \t/* Sort the sizes and merge any consecutive overlapping ranges. */\n> \tfor (auto &[format, sizes] : formats) {\n> -- \n> 2.49.0\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 ED3DFC3213\n\tfor ;\n\tFri, 18 Apr 2025 06:56:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF88F68AC7;\n\tFri, 18 Apr 2025 08:56:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 130B2617E6\n\tfor ;\n\tFri, 18 Apr 2025 08:56:48 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:8f5d:55c8:fd32:b527])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 52B41F6;\n\tFri, 18 Apr 2025 08:54:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JEAa8ou3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1744959283;\n\tbh=0sqyMGDhM2xpYYh9lSMmrj31iAXcGz2tg0nyG6IJ/Gk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JEAa8ou3xXMjH7cGkMiq98CsQpi7wiTq1Xh9g3XhNGra5XXthuZu5VL0xQOD3Qzvh\n\tiz3eVsSt/VhfBn+dTZB1OFDpVocBP5DtThvOJDTcheRRNDPDq2TY1DjJ7VOos6ttR5\n\taxgUtrt4t19VxDhLww3WsvJiX7sq/irEUNKcCL7Q=","Date":"Fri, 18 Apr 2025 15:56:40 +0900","From":"Paul Elder ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ,\n\tKieran Bingham ","Subject":"Re: [PATCH v4 05/11] libcamera: simple: Add plain output\n\tconfigurations","Message-ID":"","References":"<20250407085639.16180-1-mzamazal@redhat.com>\n\t<20250407085639.16180-6-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250407085639.16180-6-mzamazal@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34061,"web_url":"https://patchwork.libcamera.org/comment/34061/","msgid":"<85bjsizqqi.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-04-26T19:07:17","subject":"Re: [PATCH v4 05/11] libcamera: simple: Add plain output\n\tconfigurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Paul,\n\nPaul Elder writes:\n\n> Hi Milan,\n>\n> Thank you for the patches.\n>\n> On Mon, Apr 07, 2025 at 10:56:31AM +0200, Milan Zamazal wrote:\n>> Output configurations in simple pipeline are added depending on whether\n>> a converter, software ISP or none of them are used. If a converter or\n>> software ISP is used, no raw output configurations are added.\n>> \n>> In order to support raw output at least with software ISP, let's always\n>> add raw output configurations. A flag is added to\n>> SimpleCameraData::Configuration indicating whether it's for a raw or a\n>> converted output. We later filter formats and output sizes for\n>> particular stream configurations according to the new configuration\n>> flag.\n>> \n>> This is just preparation and raw output is still not supported. At the\n>> moment, we simply filter out raw configurations unconditionally to keep\n>> the current code working; this will be changed in followup patches.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 31 +++++++++++++++---------\n>> 1 file changed, 19 insertions(+), 12 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 60b7e1b1..82d7a9a5 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -319,6 +319,7 @@ public:\n>> \t\tSize captureSize;\n>> \t\tstd::vector outputFormats;\n>> \t\tSizeRange outputSizes;\n>> +\t\tbool raw;\n>> \t};\n>> \n>> \tstd::vector streams_;\n>> @@ -708,27 +709,33 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n>> \t\t\tcontinue;\n>> \n>> \t\tConfiguration config;\n>> +\n>> +\t\t/* Raw configuration */\n>> \t\tconfig.code = code;\n>> \t\tconfig.sensorSize = size;\n>> \t\tconfig.captureFormat = pixelFormat;\n>> \t\tconfig.captureSize = format.size;\n>> +\t\tconfig.outputFormats = { pixelFormat };\n>> +\t\tconfig.outputSizes = config.captureSize;\n>> +\t\tconfig.raw = true;\n>> +\t\tconfigs_.push_back(config);\n>> \n>> +\t\t/* Modified, non-raw, configuration */\n>> +\t\tconfig.raw = false;\n>> \t\tif (converter_) {\n>> \t\t\tconfig.outputFormats = converter_->formats(pixelFormat);\n>> \t\t\tconfig.outputSizes = converter_->sizes(format.size);\n>> \t\t} else if (swIsp_) {\n>> -\t\t\tconfig.outputFormats = swIsp_->formats(pixelFormat);\n>> -\t\t\tconfig.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n>> -\t\t\tif (config.outputFormats.empty()) {\n>> +\t\t\tstd::vector outputFormats = swIsp_->formats(pixelFormat);\n>> +\t\t\tif (outputFormats.empty()) {\n>> \t\t\t\t/* Do not use swIsp for unsupported pixelFormat's. */\n>> -\t\t\t\tconfig.outputFormats = { pixelFormat };\n>> -\t\t\t\tconfig.outputSizes = config.captureSize;\n>> +\t\t\t\tcontinue;\n>> \t\t\t}\n>> +\t\t\tconfig.outputFormats = outputFormats;\n>> +\t\t\tconfig.outputSizes = swIsp_->sizes(pixelFormat, format.size);\n>> \t\t} else {\n>> -\t\t\tconfig.outputFormats = { pixelFormat };\n>> -\t\t\tconfig.outputSizes = config.captureSize;\n>> +\t\t\tcontinue;\n>> \t\t}\n>> -\n>> \t\tconfigs_.push_back(config);\n>> \t}\n>> }\n>> @@ -1301,10 +1308,10 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span> \t/* Create the formats map. */\n>> \tstd::map> formats;\n>> \n>> -\tfor (const SimpleCameraData::Configuration &cfg : data->configs_) {\n>> -\t\tfor (PixelFormat format : cfg.outputFormats)\n>> -\t\t\tformats[format].push_back(cfg.outputSizes);\n>> -\t}\n>> +\tfor (const SimpleCameraData::Configuration &cfg : data->configs_)\n>> +\t\tif (!cfg.raw)\n>> +\t\t\tfor (PixelFormat format : cfg.outputFormats)\n>> +\t\t\t\tformats[format].push_back(cfg.outputSizes);\n>\n> This can break bisection (at runtime), since if the pipeline only\n> supports raw formats then !cfg.raw will never be satisfied and formats\n> will be empty, thus later on causing\n>\n> \tfor (StreamRole role : roles) {\n> \t\tStreamConfiguration cfg{ StreamFormats{ formats } };\n> \t\tcfg.pixelFormat = formats.begin()->first;\n> \t\tcfg.size = formats.begin()->second[0].max; <-- this\n>\n> this to segfault.\n>\n> I see that later on in the series this is resolved but I'd prefer\n> avoiding breaking bisection.\n\nSure, thank you for pointing this out. I'll fix it in v5.\n\n> Thanks,\n>\n> Paul\n>\n>> \n>> \t/* Sort the sizes and merge any consecutive overlapping ranges. */\n>> \tfor (auto &[format, sizes] : formats) {\n>> -- \n>> 2.49.0\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 A3538BE08B\n\tfor ;\n\tSat, 26 Apr 2025 19:07:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 99CA268ACD;\n\tSat, 26 Apr 2025 21:07:26 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E8402617E3\n\tfor ;\n\tSat, 26 Apr 2025 21:07:24 +0200 (CEST)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-393-qXSriB9fMVK_w8QAlKsO4g-1; Sat, 26 Apr 2025 15:07:20 -0400","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-39d8e5ca9c2so1775757f8f.0\n\tfor ;\n\tSat, 26 Apr 2025 12:07:20 -0700 (PDT)","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-4409d2d8479sm106547785e9.29.2025.04.26.12.07.17\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 26 Apr 2025 12:07:18 -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=\"dvXarI25\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1745694443;\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=THJTyqaWaF7IX1XG4sowbEM3KGBZp0aqTatDjNqE1cU=;\n\tb=dvXarI25j1mpskUZAZU3GRqW4ID0z/jSaLCgo7R5dqehNLSAgNLySuPQxAVhCSRONqfMYE\n\tpCzIDllTk9WFYDipEoVIH+eMfDNsgjGrVmMx5yra2N/0XU5Go2LS64TXpHDKg8hNu7BPjb\n\tpFfpaYxaC8gNQIUDfjoI7aO6Fbdh1iY=","X-MC-Unique":"qXSriB9fMVK_w8QAlKsO4g-1","X-Mimecast-MFC-AGG-ID":"qXSriB9fMVK_w8QAlKsO4g_1745694439","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1745694439; x=1746299239;\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=THJTyqaWaF7IX1XG4sowbEM3KGBZp0aqTatDjNqE1cU=;\n\tb=TrL41Rz6Zpw8SBnpR2MGJItINYLwCbQG8WsvUM7RBoL0H6A49uoIq5MX3YD1b9aKW1\n\tKjeuYlEc8E988b7NKUeSUVW4hEtB5olvHG2dNfJANmUE66LqilXFTzwHefiFEHIdCk81\n\t2aSxqEGMnaT81AQ2jiiFV3ewtUAGoviw67W9fDaZuzuOLMg2Ui8xbcl+Fsky9w6nIEyv\n\tRvMEccWOm47Zvc54Cdg4KU4nawURnPtfIvJ8EHRQHcLiD3LYo5sGOuMTLammYhL6LBT3\n\tSTmHHMXIYKp49jxkic70wXIBYgrJnWFkw2Hsz189v6wbHcWcSRdY1vr9sBU9G1ZvsZRg\n\tynlw==","X-Gm-Message-State":"AOJu0YwBJ+uwGSY9bPGrWr47nSJZV9EkfOCPTF3y53FRIGkxfxBc1pv6\n\tsPUm2A9t8BCwTdWDn7LPTaSVLK2XTC2AyNjM3OOS1YL0mCT0+nWlv0NZjMXlrb97FuSMnIQX394\n\tJjLrVIu01k7eYQODRF6GSUY+tjwiyTTb/Z5TqJ5QVnrAJ1F7oSrWo/9w6p5rwMwKjIx7PYxg=","X-Gm-Gg":"ASbGncvCb9fsdzJZVSfQ5SziATtwQjdi4EFjYg8HZCMs1vp5YEPgym9i+2a1TkNEPa2\n\tM6u0NUWfVikfxT43mxMmODfkv4nM4fJiVvrAIoG9plRaFaB9WKHZMxvsNasXCqF8UkfxTslXJYr\n\t6N11czItpt81cNckxMlPMemFuM7Ou84so1ogPSODPb1oc8oKJz6c0HF8UwrbxGuN5SAB4QXV5Vv\n\tJQG71vM83WSMWHY06n7Lu5ud81XfHYp0BwR/RnXRkwxMsWt2SS44SckwJKZOTKuXLIu8qh7iOSK\n\tW0qj0Z3NDLmThneAGrU/kH5ll20ZEG9NYH9vj1i2cLZ6mHyWNaMO8yfVCuq95ut2","X-Received":["by 2002:a05:6000:248a:b0:39c:141a:6c67 with SMTP id\n\tffacd0b85a97d-3a074f2f48amr5296095f8f.45.1745694439035; \n\tSat, 26 Apr 2025 12:07:19 -0700 (PDT)","by 2002:a05:6000:248a:b0:39c:141a:6c67 with SMTP id\n\tffacd0b85a97d-3a074f2f48amr5296082f8f.45.1745694438648; \n\tSat, 26 Apr 2025 12:07:18 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHYsLz7LhPg1iTTX1OCb7ivAJSGGfH0Bs5hXhEaJyDqPZvrf8SK5JRMPUMhy/k+ITR4lWLHhQ==","From":"Milan Zamazal ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Kieran Bingham\n\t","Subject":"Re: [PATCH v4 05/11] libcamera: simple: Add plain output\n\tconfigurations","In-Reply-To":" (Paul Elder's message of\n\t\"Fri, 18 Apr 2025 15:56:40 +0900\")","References":"<20250407085639.16180-1-mzamazal@redhat.com>\n\t<20250407085639.16180-6-mzamazal@redhat.com>\n\t","Date":"Sat, 26 Apr 2025 21:07:17 +0200","Message-ID":"<85bjsizqqi.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":"b5261cMcueZUT3iZcr37XlqNw0fiUWliADEIAskvfWk_1745694439","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\" "}}]