[{"id":27973,"web_url":"https://patchwork.libcamera.org/comment/27973/","msgid":"","date":"2023-10-17T11:54:41","subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Move flip handling\n\tvalidation code","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Fri, Oct 13, 2023 at 10:10:03AM +0100, Naushir Patuck via libcamera-devel wrote:\n> Move the handling of Bayer order changes due to flips to run before\n> platformValidate(). This removes the need for this code to be split\n> between platformValidate() and validate() as it is right now.\n>\n> Also add some validation to ensure the vc4 pipeline handler only\n> supports CSI2 packing or no packing.\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 43 +++++++++++++------\n> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 15 ++++---\n> 2 files changed, 38 insertions(+), 20 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 825eae80d014..7c88b87e0608 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -231,16 +231,12 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \t\t}\n> \t}\n>\n> -\t/* Do any platform specific fixups. */\n> -\tstatus = data_->platformValidate(this);\n> -\tif (status == Invalid)\n> -\t\treturn Invalid;\n> -\n> -\t/* Further fixups on the RAW streams. */\n> +\t/* Start with some initial generic RAW stream adjustments. */\n> \tfor (auto &raw : rawStreams_) {\n> -\t\tint ret = raw.dev->tryFormat(&raw.format);\n> -\t\tif (ret)\n> -\t\t\treturn Invalid;\n> +\t\tStreamConfiguration *rawStream = raw.cfg;\n> +\n> +\t\t/* Adjust the RAW stream to match the computed sensor format. */\n> +\t\tBayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.mbus_code);\n>\n> \t\t/*\n> \t\t * Some sensors change their Bayer order when they are h-flipped\n> @@ -249,12 +245,33 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \t\t * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> \t\t * order, because the sensor may currently be flipped!\n> \t\t */\n> -\t\tBayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> \t\tif (data_->flipsAlterBayerOrder_) {\n> -\t\t\tbayer.order = data_->nativeBayerOrder_;\n> -\t\t\tbayer = bayer.transform(combinedTransform_);\n> +\t\t\tsensorBayer.order = data_->nativeBayerOrder_;\n> +\t\t\tsensorBayer = sensorBayer.transform(combinedTransform_);\n> +\t\t}\n> +\n> +\t\t/* Apply the sensor adjusted Bayer order to the user request. */\n> +\t\tBayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);\n> +\t\tcfgBayer.order = sensorBayer.order;\n> +\n> +\t\tif (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {\n> +\t\t\trawStream->pixelFormat = cfgBayer.toPixelFormat();\n> +\t\t\tstatus = Adjusted;\n> \t\t}\n> -\t\traw.cfg->pixelFormat = bayer.toPixelFormat();\n> +\t}\n> +\n> +\t/* Do any platform specific fixups. */\n> +\tStatus st = data_->platformValidate(this);\n> +\tif (st == Invalid)\n> +\t\treturn Invalid;\n> +\telse if (st == Adjusted)\n> +\t\tstatus = Adjusted;\n> +\n> +\t/* Further fixups on the RAW streams. */\n> +\tfor (auto &raw : rawStreams_) {\n> +\t\tint ret = raw.dev->tryFormat(&raw.format);\n> +\t\tif (ret)\n> +\t\t\treturn Invalid;\n>\n> \t\tif (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n> \t\t\tstatus = Adjusted;\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 233473e2fe2b..425ab9ae6ea5 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -409,16 +409,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n>\n> \t\t/* Adjust the RAW stream to match the computed sensor format. */\n> \t\tStreamConfiguration *rawStream = rawStreams[0].cfg;\n> -\t\tBayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n> +\t\tBayerFormat rawBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);\n>\n> -\t\t/* Handle flips to make sure to match the RAW stream format. */\n> -\t\tif (flipsAlterBayerOrder_)\n> -\t\t\trawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> +\t\t/* Apply the sensor bitdepth. */\n> +\t\trawBayer.bitDepth = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code).bitDepth;\n>\n> -\t\t/* Apply the user requested packing. */\n> -\t\trawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> -\t\tPixelFormat rawFormat = rawBayer.toPixelFormat();\n> +\t\t/* Default to CSI2 packing if the user request is unsupported. */\n> +\t\tif (rawBayer.packing != BayerFormat::Packing::CSI2 &&\n> +\t\t rawBayer.packing != BayerFormat::Packing::None)\n> +\t\t\trawBayer.packing = BayerFormat::Packing::CSI2;\n\nDo you need to return Adjusted in this case ?\n\nThanks\n j\n\n>\n> +\t\tPixelFormat rawFormat = rawBayer.toPixelFormat();\n> \t\tif (rawStream->pixelFormat != rawFormat ||\n> \t\t rawStream->size != rpiConfig->sensorFormat_.size) {\n> \t\t\trawStream->pixelFormat = rawFormat;\n> --\n> 2.34.1\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 07F16C3272\n\tfor ;\n\tTue, 17 Oct 2023 11:54:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 57C676297C;\n\tTue, 17 Oct 2023 13:54:47 +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 DFDA96295F\n\tfor ;\n\tTue, 17 Oct 2023 13:54:45 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 36D90E4;\n\tTue, 17 Oct 2023 13:54:39 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697543687;\n\tbh=g07OrqA4uPfADehAH6+9KL9ROBJMaiksKf4CEVFwfRY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GkcA8fFSFgkyDbdUsLM62p9VFUFIus0wU8HD2s3vd7R1RahSqjZvLan78AeMQLCGR\n\tvC25M0XjWrj2hpnkfrVFHZBUrjOodJzGcydtw222DnzpdjkiYrgGKo3OUdbsNlDnnA\n\tEjyBsXhsLuHoe/ZHbVcwPheD1vJ61HA3ro8pM5PwX8vT7uoEkOrxtGd4POR6BD1F9Z\n\taYxvirShLTxu4UUxQtmDS9maICv5ilwiAjz5YpAzDz/sERCQVxx5PQrJJnLdMAdXRG\n\tEyt1FXXf0oSI0BRLqJNZJ+IKx3dW6VRdK4LmZ4Qpcr7tAVEzMwPajgFW+pXgNqTmmn\n\tiolb3BViIajgA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697543679;\n\tbh=g07OrqA4uPfADehAH6+9KL9ROBJMaiksKf4CEVFwfRY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Rs22M4M9rYEkaOusP5H7Is4tCDGXu396ZGhDRBgNgukofg8xou4tJq1gviKuT9Inq\n\t13e0e/hf8rJ0X1ABlOi6p2hBuaVJIAIE7Sf6LXSg9llbvdoUjC05AYvw0/zaZp4rrr\n\tk/Glhnzs5+ACeeiu4U5vWhYb7MA8vPtMFzlNH7Io="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Rs22M4M9\"; dkim-atps=neutral","Date":"Tue, 17 Oct 2023 13:54:41 +0200","To":"Naushir Patuck ","Message-ID":"","References":"<20231013074841.16972-15-naush@raspberrypi.com>\n\t<20231013091003.2571-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231013091003.2571-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Move flip handling\n\tvalidation code","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27974,"web_url":"https://patchwork.libcamera.org/comment/27974/","msgid":"","date":"2023-10-17T12:02:54","subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Move flip handling\n\tvalidation code","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nOn Tue, 17 Oct 2023 at 12:54, Jacopo Mondi\n wrote:\n>\n> Hi Naush\n>\n> On Fri, Oct 13, 2023 at 10:10:03AM +0100, Naushir Patuck via libcamera-devel wrote:\n> > Move the handling of Bayer order changes due to flips to run before\n> > platformValidate(). This removes the need for this code to be split\n> > between platformValidate() and validate() as it is right now.\n> >\n> > Also add some validation to ensure the vc4 pipeline handler only\n> > supports CSI2 packing or no packing.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > ---\n> > .../pipeline/rpi/common/pipeline_base.cpp | 43 +++++++++++++------\n> > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 15 ++++---\n> > 2 files changed, 38 insertions(+), 20 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 825eae80d014..7c88b87e0608 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -231,16 +231,12 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > }\n> > }\n> >\n> > - /* Do any platform specific fixups. */\n> > - status = data_->platformValidate(this);\n> > - if (status == Invalid)\n> > - return Invalid;\n> > -\n> > - /* Further fixups on the RAW streams. */\n> > + /* Start with some initial generic RAW stream adjustments. */\n> > for (auto &raw : rawStreams_) {\n> > - int ret = raw.dev->tryFormat(&raw.format);\n> > - if (ret)\n> > - return Invalid;\n> > + StreamConfiguration *rawStream = raw.cfg;\n> > +\n> > + /* Adjust the RAW stream to match the computed sensor format. */\n> > + BayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.mbus_code);\n> >\n> > /*\n> > * Some sensors change their Bayer order when they are h-flipped\n> > @@ -249,12 +245,33 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > * order, because the sensor may currently be flipped!\n> > */\n> > - BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> > if (data_->flipsAlterBayerOrder_) {\n> > - bayer.order = data_->nativeBayerOrder_;\n> > - bayer = bayer.transform(combinedTransform_);\n> > + sensorBayer.order = data_->nativeBayerOrder_;\n> > + sensorBayer = sensorBayer.transform(combinedTransform_);\n> > + }\n> > +\n> > + /* Apply the sensor adjusted Bayer order to the user request. */\n> > + BayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);\n> > + cfgBayer.order = sensorBayer.order;\n> > +\n> > + if (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {\n> > + rawStream->pixelFormat = cfgBayer.toPixelFormat();\n> > + status = Adjusted;\n> > }\n> > - raw.cfg->pixelFormat = bayer.toPixelFormat();\n> > + }\n> > +\n> > + /* Do any platform specific fixups. */\n> > + Status st = data_->platformValidate(this);\n> > + if (st == Invalid)\n> > + return Invalid;\n> > + else if (st == Adjusted)\n> > + status = Adjusted;\n> > +\n> > + /* Further fixups on the RAW streams. */\n> > + for (auto &raw : rawStreams_) {\n> > + int ret = raw.dev->tryFormat(&raw.format);\n> > + if (ret)\n> > + return Invalid;\n> >\n> > if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n> > status = Adjusted;\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > index 233473e2fe2b..425ab9ae6ea5 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > @@ -409,16 +409,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n> >\n> > /* Adjust the RAW stream to match the computed sensor format. */\n> > StreamConfiguration *rawStream = rawStreams[0].cfg;\n> > - BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n> > + BayerFormat rawBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);\n> >\n> > - /* Handle flips to make sure to match the RAW stream format. */\n> > - if (flipsAlterBayerOrder_)\n> > - rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> > + /* Apply the sensor bitdepth. */\n> > + rawBayer.bitDepth = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code).bitDepth;\n> >\n> > - /* Apply the user requested packing. */\n> > - rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> > - PixelFormat rawFormat = rawBayer.toPixelFormat();\n> > + /* Default to CSI2 packing if the user request is unsupported. */\n> > + if (rawBayer.packing != BayerFormat::Packing::CSI2 &&\n> > + rawBayer.packing != BayerFormat::Packing::None)\n> > + rawBayer.packing = BayerFormat::Packing::CSI2;\n>\n> Do you need to return Adjusted in this case ?\n\nYes we do, that (I think!) is handled in the if() clause below where\nwe convert from Bayer back to PixelFormat and do the comparison with\nthe config PixelFormat.\n\nRegards,\nNaush\n\n\n>\n> Thanks\n> j\n>\n> >\n> > + PixelFormat rawFormat = rawBayer.toPixelFormat();\n> > if (rawStream->pixelFormat != rawFormat ||\n> > rawStream->size != rpiConfig->sensorFormat_.size) {\n> > rawStream->pixelFormat = rawFormat;\n> > --\n> > 2.34.1\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 3B04ABDCBD\n\tfor ;\n\tTue, 17 Oct 2023 12:03:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 802D06297F;\n\tTue, 17 Oct 2023 14:03:23 +0200 (CEST)","from mail-yb1-xb2d.google.com (mail-yb1-xb2d.google.com\n\t[IPv6:2607:f8b0:4864:20::b2d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B9DF56295F\n\tfor ;\n\tTue, 17 Oct 2023 14:03:21 +0200 (CEST)","by mail-yb1-xb2d.google.com with SMTP id\n\t3f1490d57ef6-d9c66e70ebdso115521276.2\n\tfor ;\n\tTue, 17 Oct 2023 05:03:21 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697544203;\n\tbh=gXeVS8jlJGWTsAhKkKPurJ/UXsWjVSIoNmFOQmSiE+U=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=W/zUAgMXzJt85cXxEzneaFr7Z9EPuU1vwujEs9q22tsqBk7ThrrlzTCUFkdxxZh+H\n\tgHgoowMlSoDXEE3J6PIBKiV4YlF+jYzm22AeTQkPXCmudfBkt7qzq5bBBhazYtKuEC\n\tXzoLvSvQv1HNHgRvvkGrIGhNZver5u37BlXLosmjhy7v965qz26pTOO9ReCN9ToPp1\n\tpe6uC/lmP5LBfOIx/rE3waDKTGnaxKl2PvBM7eGdXWEnlU5YMe7HCQlh8Xvt1OoH1B\n\trseXlPI+RulFzqq+vVE4Dj4QoyI97NPTbPzgsEj2lSFEsJP0I3WlyxyYyhYamHDx7B\n\t06GBDRGLFOtMQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1697544200; x=1698149000;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=WuHnT2CDrb33qzc5T6H35zYfrCNJPSP5Mdm9W9eSEGE=;\n\tb=IbnkCEHDy6lWEHzWSHBs78f926DG1e/ZOY+q2DNTRcF4HvLh3HmQTIEoMnu9wq9pyq\n\tcukJDK8pIKtw/ouRpg52vBGrAcwf16P8MqYzLDyB8lOP6DwmfPse22jjGwX8/c8QmNCV\n\t/byxPzXHSVQngG4/gj+KkvZUx2Egg8+cfMa7yn9HjT1FYLLntXl9ALT/CaFV+yz+X/PS\n\tbrjbLT2MNHqNNxujqQXpJxibMEOKncnOadI8GeuRSGvFBWg2dJiidjryKm5G+xtjJuwF\n\tE5JoNwDKUmoCZA7Ndcl67JJoch2n8viqua1qLngMBVoc2TBCwm4xstg0UBg+erzmOmUz\n\tAtmg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"IbnkCEHD\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1697544200; x=1698149000;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=WuHnT2CDrb33qzc5T6H35zYfrCNJPSP5Mdm9W9eSEGE=;\n\tb=OjBTa9DABhy0EbxBeasRcQ+wwSLiNXtUmh46rG5dbP6MmRC/d/qhnG2Xn4096lO0f2\n\tSSCQWVaeKTf7NLVawvLFYahKpbN5CB7z2d8fSL4Ax49+cUBccHZDK7lppypBsexgPGul\n\tFWgkwUGt/UgZj+gDNzX9o9wF1PepxXCzt131FTzWS6BlNg6I2HfrLmmpXycf1Uc2LNwj\n\tcWaqg0j0K3FN8mjLOm6Hix/kYeXAeImsCGnFa23zn9R5rzrr00cGaAJf8SGIhA8VsA+6\n\tiTFHpcHUPpP+Yt/A8lp8jhemqOy+dw9BMXbQAegb08Aapaansqc10O1WPI5Kxe9HmKXV\n\tIFmA==","X-Gm-Message-State":"AOJu0YyLuqw9ESPtKoHwFlvtnrKH+YZkYBPzfymERgKLnVP+o/acY3xr\n\tIiBRA32Mp3HBdj0HxINQ7L8TWvdgyK/F/yqpuYqgRwAoNXTuwhf6IaQ=","X-Google-Smtp-Source":"AGHT+IGU+M9Rz0mXljB2sL8WBnOxhYJHQDsLjYrXa4/0G2eM76xLMBs1Mq/PS+Jllsz/0g7p1SrJX8C/Q8w7RPZBrPs=","X-Received":"by 2002:a25:e7d3:0:b0:d81:5d5a:25a3 with SMTP id\n\te202-20020a25e7d3000000b00d815d5a25a3mr1542825ybh.43.1697544200093;\n\tTue, 17 Oct 2023 05:03:20 -0700 (PDT)","MIME-Version":"1.0","References":"<20231013074841.16972-15-naush@raspberrypi.com>\n\t<20231013091003.2571-1-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Tue, 17 Oct 2023 13:02:54 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Move flip handling\n\tvalidation code","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27975,"web_url":"https://patchwork.libcamera.org/comment/27975/","msgid":"","date":"2023-10-17T12:30:04","subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Move flip handling\n\tvalidation code","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, Oct 17, 2023 at 01:02:54PM +0100, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> On Tue, 17 Oct 2023 at 12:54, Jacopo Mondi\n> wrote:\n> >\n> > Hi Naush\n> >\n> > On Fri, Oct 13, 2023 at 10:10:03AM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > Move the handling of Bayer order changes due to flips to run before\n> > > platformValidate(). This removes the need for this code to be split\n> > > between platformValidate() and validate() as it is right now.\n> > >\n> > > Also add some validation to ensure the vc4 pipeline handler only\n> > > supports CSI2 packing or no packing.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > Reviewed-by: David Plowman \n> > > ---\n> > > .../pipeline/rpi/common/pipeline_base.cpp | 43 +++++++++++++------\n> > > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 15 ++++---\n> > > 2 files changed, 38 insertions(+), 20 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index 825eae80d014..7c88b87e0608 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -231,16 +231,12 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > }\n> > > }\n> > >\n> > > - /* Do any platform specific fixups. */\n> > > - status = data_->platformValidate(this);\n> > > - if (status == Invalid)\n> > > - return Invalid;\n> > > -\n> > > - /* Further fixups on the RAW streams. */\n> > > + /* Start with some initial generic RAW stream adjustments. */\n> > > for (auto &raw : rawStreams_) {\n> > > - int ret = raw.dev->tryFormat(&raw.format);\n> > > - if (ret)\n> > > - return Invalid;\n> > > + StreamConfiguration *rawStream = raw.cfg;\n> > > +\n> > > + /* Adjust the RAW stream to match the computed sensor format. */\n> > > + BayerFormat sensorBayer = BayerFormat::fromMbusCode(sensorFormat_.mbus_code);\n> > >\n> > > /*\n> > > * Some sensors change their Bayer order when they are h-flipped\n> > > @@ -249,12 +245,33 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > > * order, because the sensor may currently be flipped!\n> > > */\n> > > - BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> > > if (data_->flipsAlterBayerOrder_) {\n> > > - bayer.order = data_->nativeBayerOrder_;\n> > > - bayer = bayer.transform(combinedTransform_);\n> > > + sensorBayer.order = data_->nativeBayerOrder_;\n> > > + sensorBayer = sensorBayer.transform(combinedTransform_);\n> > > + }\n> > > +\n> > > + /* Apply the sensor adjusted Bayer order to the user request. */\n> > > + BayerFormat cfgBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);\n> > > + cfgBayer.order = sensorBayer.order;\n> > > +\n> > > + if (rawStream->pixelFormat != cfgBayer.toPixelFormat()) {\n> > > + rawStream->pixelFormat = cfgBayer.toPixelFormat();\n> > > + status = Adjusted;\n> > > }\n> > > - raw.cfg->pixelFormat = bayer.toPixelFormat();\n> > > + }\n> > > +\n> > > + /* Do any platform specific fixups. */\n> > > + Status st = data_->platformValidate(this);\n> > > + if (st == Invalid)\n> > > + return Invalid;\n> > > + else if (st == Adjusted)\n> > > + status = Adjusted;\n> > > +\n> > > + /* Further fixups on the RAW streams. */\n> > > + for (auto &raw : rawStreams_) {\n> > > + int ret = raw.dev->tryFormat(&raw.format);\n> > > + if (ret)\n> > > + return Invalid;\n> > >\n> > > if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n> > > status = Adjusted;\n> > > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > index 233473e2fe2b..425ab9ae6ea5 100644\n> > > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > @@ -409,16 +409,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n> > >\n> > > /* Adjust the RAW stream to match the computed sensor format. */\n> > > StreamConfiguration *rawStream = rawStreams[0].cfg;\n> > > - BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n> > > + BayerFormat rawBayer = BayerFormat::fromPixelFormat(rawStream->pixelFormat);\n> > >\n> > > - /* Handle flips to make sure to match the RAW stream format. */\n> > > - if (flipsAlterBayerOrder_)\n> > > - rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> > > + /* Apply the sensor bitdepth. */\n> > > + rawBayer.bitDepth = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code).bitDepth;\n> > >\n> > > - /* Apply the user requested packing. */\n> > > - rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> > > - PixelFormat rawFormat = rawBayer.toPixelFormat();\n> > > + /* Default to CSI2 packing if the user request is unsupported. */\n> > > + if (rawBayer.packing != BayerFormat::Packing::CSI2 &&\n> > > + rawBayer.packing != BayerFormat::Packing::None)\n> > > + rawBayer.packing = BayerFormat::Packing::CSI2;\n> >\n> > Do you need to return Adjusted in this case ?\n>\n> Yes we do, that (I think!) is handled in the if() clause below where\n> we convert from Bayer back to PixelFormat and do the comparison with\n> the config PixelFormat.\n\nOh indeed, tha packing information gets reflected in the PixelFormat\nreturned by\n PixelFormat rawFormat = rawBayer.toPixelFormat();\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> Regards,\n> Naush\n>\n>\n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > + PixelFormat rawFormat = rawBayer.toPixelFormat();\n> > > if (rawStream->pixelFormat != rawFormat ||\n> > > rawStream->size != rpiConfig->sensorFormat_.size) {\n> > > rawStream->pixelFormat = rawFormat;\n> > > --\n> > > 2.34.1\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 39896C3272\n\tfor ;\n\tTue, 17 Oct 2023 12:30:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0763C6297C;\n\tTue, 17 Oct 2023 14:30:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6803F6295F\n\tfor ;\n\tTue, 17 Oct 2023 14:30:07 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:72c3:346:a663:c82d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BF7C82B3;\n\tTue, 17 Oct 2023 14:30:00 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697545809;\n\tbh=BHC3oHzIMit3KKKSkFR+vs3UiKWiB4cqWBNx7dn8tXQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=T/LygHkua2qF0oE+SOovwqSseknt0M4Gd6Me0j3/myTsgxShxxK4ElVIfb3lgcTGy\n\tLDOsfDXn+gcogLlunbY3PDvXKOmnMlsEREhxusg/u2xGayXaSgC5UO5JgqI+Tw4mNE\n\tm7OahgjauHz0mzMwgsWH0nXgKED6WhC+HqeDQwSXWs4mXuol+YSSFYGbMs8A6I0HRI\n\tSie7gA8xIqd00GqeKZPlDDltr/zfjF5p2ZxybJG4FnQX+loDPUEzgOCgPP9nZb8NHA\n\tLEfzcUZWx10y83sSF3rtF+BY4DYbjQHiEneB96SIxqhKahlXi3b0dhWhNyDC9VYIJ7\n\tReHJMED0/8fiA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697545800;\n\tbh=BHC3oHzIMit3KKKSkFR+vs3UiKWiB4cqWBNx7dn8tXQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LEwtzaGJr+oMP7YenFAJv5d3vUc1vLACivtxqrBGQoTJDo7P7C96TI5p4l3swBdHw\n\tWxWrQdeapnJIAjwGgqy/+yLNyv3kjCqbmfX2d9LL+yOx1LmwfO0L9X2Gw3R1OFhCRB\n\tTqCuBed+BdJO+k/3vt2ZHoXaxDkfKvHvqSco9+AQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LEwtzaGJ\"; dkim-atps=neutral","Date":"Tue, 17 Oct 2023 14:30:04 +0200","To":"Naushir Patuck ","Message-ID":"","References":"<20231013074841.16972-15-naush@raspberrypi.com>\n\t<20231013091003.2571-1-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] pipeline: rpi: Move flip handling\n\tvalidation code","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]