[{"id":27545,"web_url":"https://patchwork.libcamera.org/comment/27545/","msgid":"","date":"2023-07-12T11:28:39","subject":"Re: [libcamera-devel] [PATCH v1 3/3] libcamera: rpi: pipeline_base:\n\tCache sensor format","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThank you for your work!\n\nOn Wed, 12 Jul 2023 at 11:55, Naushir Patuck wrote:\n>\n> From: Jacopo Mondi \n>\n> The format to be applied on the sensor is selected by two criteria: the\n> desired output size and the bit depth. As the selection depends on the\n> presence of a RAW stream and the streams configuration is handled in\n> validate() there is no need to re-compute the format in configure().\n>\n> Centralize the computation of the sensor format in validate() and remove\n> it from configure().\n>\n> Signed-off-by: Jacopo Mondi \n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 46 +++++++------------\n> .../pipeline/rpi/common/pipeline_base.h | 2 +\n> 2 files changed, 19 insertions(+), 29 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 725c1db0d527..d96277a2794e 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -214,10 +214,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> bitDepth = bayerFormat.bitDepth;\n> }\n>\n> - V4L2SubdeviceFormat sensorFormat =\n> - data_->findBestFormat(rawStreams.empty() ? outStreams[0].cfg->size\n> - : rawStreams[0].cfg->size,\n> - bitDepth);\n> + sensorFormat_ = data_->findBestFormat(rawStreams.empty() ? outStreams[0].cfg->size\n> + : rawStreams[0].cfg->size,\n> + bitDepth);\n>\n> status = data_->platformValidate(rawStreams, outStreams);\n> if (status == Invalid)\n> @@ -230,10 +229,10 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>\n> const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> bitDepth = info.isValid() ? info.bitsPerPixel : defaultRawBitDepth;\n> - sensorFormat = data_->findBestFormat(cfg.size, bitDepth);\n> + V4L2SubdeviceFormat subdevFormat = data_->findBestFormat(cfg.size, bitDepth);\n>\n> BayerFormat::Packing packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> - rawFormat = PipelineHandlerBase::toV4L2DeviceFormat(raw.dev, sensorFormat, packing);\n> + rawFormat = PipelineHandlerBase::toV4L2DeviceFormat(raw.dev, subdevFormat, packing);\n\nAs per the comments on the previous patch, if we agree that subdevFormat can be\nremoved, we can replace it with sensorFormat_ in the above call to\nPipelineHandlerBase::toV4L2DeviceFormat().\n\nReviewed-by: Naushir Patuck \n\nRegards,\nNaush\n\n>\n> int ret = raw.dev->tryFormat(&rawFormat);\n> if (ret)\n> @@ -453,8 +452,6 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> stream->clearFlags(StreamFlag::External);\n>\n> std::vector rawStreams, ispStreams;\n> - std::optional packing;\n> - unsigned int bitDepth = defaultRawBitDepth;\n>\n> for (unsigned i = 0; i < config->size(); i++) {\n> StreamConfiguration *cfg = &config->at(i);\n> @@ -472,32 +469,23 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)\n> std::sort(ispStreams.begin(), ispStreams.end(),\n> [](auto &l, auto &r) { return l.cfg->size > r.cfg->size; });\n>\n> - /*\n> - * Calculate the best sensor mode we can use based on the user's request,\n> - * and apply it to the sensor with the cached tranform, if any.\n> - *\n> - * If we have been given a RAW stream, use that size for setting up the sensor.\n> - */\n> - if (!rawStreams.empty()) {\n> - BayerFormat bayerFormat = BayerFormat::fromPixelFormat(rawStreams[0].cfg->pixelFormat);\n> - /* Replace the user requested packing/bit-depth. */\n> - packing = bayerFormat.packing;\n> - bitDepth = bayerFormat.bitDepth;\n> - }\n> -\n> - V4L2SubdeviceFormat sensorFormat =\n> - data->findBestFormat(rawStreams.empty() ? ispStreams[0].cfg->size\n> - : rawStreams[0].cfg->size,\n> - bitDepth);\n> + /* Apply the format on the sensor with any cached transform. */\n> + const RPiCameraConfiguration *rpiConfig =\n> + static_cast(config);\n> + V4L2SubdeviceFormat sensorFormat = rpiConfig->sensorFormat_;\n>\n> - /* Apply any cached transform. */\n> - const RPiCameraConfiguration *rpiConfig = static_cast(config);\n> -\n> - /* Then apply the format on the sensor. */\n> ret = data->sensor_->setFormat(&sensorFormat, rpiConfig->combinedTransform_);\n> if (ret)\n> return ret;\n>\n> + /* Use the user requested packing/bit-depth. */\n> + std::optional packing;\n> + if (!rawStreams.empty()) {\n> + BayerFormat bayerFormat =\n> + BayerFormat::fromPixelFormat(rawStreams[0].cfg->pixelFormat);\n> + packing = bayerFormat.packing;\n> + }\n> +\n> /*\n> * Platform specific internal stream configuration. This also assigns\n> * external streams which get configured below.\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index 2eda3cd89812..a139c98a5a2b 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -262,6 +262,8 @@ public:\n>\n> /* Cache the combinedTransform_ that will be applied to the sensor */\n> Transform combinedTransform_;\n> + /* The sensor format computed in validate() */\n> + V4L2SubdeviceFormat sensorFormat_;\n>\n> private:\n> const CameraData *data_;\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 F05D8BEFBE\n\tfor ;\n\tWed, 12 Jul 2023 11:28:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 525C9628BC;\n\tWed, 12 Jul 2023 13:28:59 +0200 (CEST)","from mail-yw1-x112e.google.com (mail-yw1-x112e.google.com\n\t[IPv6:2607:f8b0:4864:20::112e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9161D628BB\n\tfor ;\n\tWed, 12 Jul 2023 13:28:57 +0200 (CEST)","by mail-yw1-x112e.google.com with SMTP id\n\t00721157ae682-57045429f76so75646547b3.0\n\tfor ;\n\tWed, 12 Jul 2023 04:28:57 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689161339;\n\tbh=ouV5pF1s6BMZvwoxJuSF0+xaIcofzy0Y4Ie2wXvaGwI=;\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=zXvRdYlxtx0ingFL/Sz8vXD7PhG6l6+26AI7yEXitKxFO9tgqYb8gFMxCtknGTAO4\n\tI/N3LPxAX3U8THo0BHDKbwShhWYsKUKkC7KF/4JaHDekUi6ed9ntL5ImxVt5iTQGUO\n\t/9LyrFcmWapI50VFM8BF/4OxUHrZJeQ+0ZUYq5u7/1X1Bvn6/wSHonFaGf+ih125Dx\n\tg+zlng4jiP+5Wum57u6Tu31ceglmZXesnEgNM1kr2njP0Tg3YbtxTKFBTRDh6v3YBL\n\tHRJJ04IR+uN5cNBjhYRfd7q5H0aaflzfNwBCsw44eMFUqGG7MOfG7w2Smc2+EhjpuY\n\tQ+wsALXpo515g==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1689161336; x=1691753336;\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=HxMxghEslzaPtcdjKzZg8g3T76dcNRfrumBwNRuAJtE=;\n\tb=RNFJ+OdA9p7PB3V453ji+SxVKgCJur823rAtnYOzwsFCOlrlBKcDG/yyG+9fdZkOnX\n\t69PdVqVG3q5pV4zTBpGO94MPUg3KIL3tC+Yzju44S0sxDALy5S3GJPThfnqkAFxKRw8L\n\tmIBXbEmfgh0wcTq0a9iIPIrfsBrtihER9zRMTvbSnwgIXJFfJvwqwNTkfmjvcDOtJvLd\n\t1So/RFR3D1MUVmxBPjBk+OJ58FE0qbR6ytFk1Tg2JCg+4NixUHiD5iruo+QwY2P+O502\n\tvMQ7qv1kVRZc8xHmEd6vl8jvooVdrXs/mH+EmSlbegd45ezBQ/egEVMvA6su5eyFfR8e\n\tClCw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"RNFJ+OdA\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1689161336; x=1691753336;\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=HxMxghEslzaPtcdjKzZg8g3T76dcNRfrumBwNRuAJtE=;\n\tb=D5Lo+/5cjKzYD7VpO3N5YUWCA1abAiyvii6iVVA+RelLwMc33EENPWg/NcSg+JHHFo\n\t/muSez3pyzKcgo1XDNjf2xiLnUdPSvJ3rHTQyMaOODLjb9R4yD2gf9Hh0QYPbhngHY9O\n\tyqMXF66KmLaLxEwFqxOSDYwCJtmXyXN0AE0CdqVzYMPfxnESvNvOXcwqMOFw3H90I2OH\n\twneohIej34HVT14+9zCjmjQmPRRxWnfbnO5Y/rGbQeQVU3WCyhaQA9IxBABs0seWfD50\n\twjrAyhrPDnqCmOChp5BpxHAuTD+RmDJSuSzFL6b3VBac/L0G7by+igPSAMQmYFH9zQ6t\n\tTkZQ==","X-Gm-Message-State":"ABy/qLapUOQ7M0vO9yCBnp3WqhwWqfkM4M2ODpDyNDVRPLrATquq4u4M\n\tNOZLO4wbeWlvjGNQ82xYhsAahbQy2d7nkzAiwk2HJWcegq013SptJyZQRQ==","X-Google-Smtp-Source":"APBJJlH4uReVoBk3tyS8gxl0Yx7L4wkGhxO5tFBuLrDAnGTTCNGG/h8V1bzItkwRR3Zldz1fCPrnensz8jonFt2YO+8=","X-Received":"by 2002:a81:6c97:0:b0:561:d1ef:3723 with SMTP id\n\th145-20020a816c97000000b00561d1ef3723mr16786736ywc.38.1689161334659;\n\tWed, 12 Jul 2023 04:28:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20230712105510.14658-1-naush@raspberrypi.com>\n\t<20230712105510.14658-4-naush@raspberrypi.com>","In-Reply-To":"<20230712105510.14658-4-naush@raspberrypi.com>","Date":"Wed, 12 Jul 2023 12:28:39 +0100","Message-ID":"","To":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 3/3] libcamera: rpi: pipeline_base:\n\tCache sensor format","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":"Jacopo Mondi ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]