From patchwork Fri Sep 15 13:06:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 19029 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 4372BBD160 for ; Fri, 15 Sep 2023 13:07:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D4E5962925; Fri, 15 Sep 2023 15:07:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1694783240; bh=iLQ8LBHmiC3945zZsyxvl/bmfpMvg4wmJfLfordAR+I=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=O9I4lwUaFkC0F9/nwDEXjpe4NNqosSmtXKckAtoD3fXYhVeJxXQPi4f0xI0Yd3ZSV gG/jgdG+r1N/AK2AKSRakoUaNM/6PE4hgvJuY1XSB5r5tFDD08ExXE9YM+0pc2dujB lrYuv3ToyRegd7MNxghyTM9p/g8de6fF1zDb4hllBWuD1VgIa5i9EjyjuXgT97vLI1 yzHsf/2+7nOc0qSidvnOx6Sf4Vx6WuHCVwmmUpeEViBOpKieFxVksGwuF0nP3yGBhO BJN/I70Aco+b1h6DD9OKDpxqzLSH2scGAUxq3gA14jSd+TmQiznkmJn0GPcUWvrQOL zVqy4xgcUoY0w== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D4FF6291C for ; Fri, 15 Sep 2023 15:07:12 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vtXD1oCH"; dkim-atps=neutral Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BA109208C; Fri, 15 Sep 2023 15:05:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1694783139; bh=iLQ8LBHmiC3945zZsyxvl/bmfpMvg4wmJfLfordAR+I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vtXD1oCH4SAffQSKc1HYGGxDlIkmd+2XBg2Vf/kQcNkj0enNDHHkyJYqc0WHUHsbl oWnJ4hZ/GxVpgGd3uPKvHKaiXZSwpvVBdmFR+9LE5NfBd8NcrnTa6WG1uq03XiPuqt 0TrkHuuQXdjx7mpA4rT3jUPw8SGKQPsc1OTEo39g= To: libcamera-devel@lists.libcamera.org Date: Fri, 15 Sep 2023 15:06:48 +0200 Message-ID: <20230915130650.35691-11-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20230915130650.35691-1-jacopo.mondi@ideasonboard.com> References: <20230915130650.35691-1-jacopo.mondi@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 10/12] libcamera: rpi: Simplify validate() and configure() for RAW streams X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jacopo Mondi via libcamera-devel From: Jacopo Mondi Reply-To: Jacopo Mondi Cc: Jacopo Mondi Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Naushir Patuck This commit simplifies the validate() and configure() calls in the pipeline handler in a number of ways: - Only pass the RPiCameraConfiguration structure into platformValidate() and platformConfigure(). - Determine the V4L2DeviceFormat structure fields for all streams in validate(), cache them and reuse in configure() instead of re-generating this structure multiple times. - Use the recently added updateStreamConfig() and toV4L2DeviceFormat() helpers to populate fields in the V4L2DeviceFormat and StreamConfiguration structures to reduce code duplication. Signed-off-by: Naushir Patuck Reviewed-by: Jacopo Mondi Signed-off-by: Jacopo Mondi --- .../pipeline/rpi/common/pipeline_base.cpp | 48 +++++-------------- .../pipeline/rpi/common/pipeline_base.h | 5 +- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 35 ++++++++------ 3 files changed, 34 insertions(+), 54 deletions(-) diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index b523d560420a..d83811801460 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -234,16 +234,10 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() /* Further fixups on the RAW streams. */ for (auto &raw : rawStreams_) { - StreamConfiguration &cfg = config_.at(raw.index); - - V4L2DeviceFormat rawFormat; - rawFormat.fourcc = raw.dev->toV4L2PixelFormat(cfg.pixelFormat); - rawFormat.size = cfg.size; - rawFormat.colorSpace = cfg.colorSpace; - - int ret = raw.dev->tryFormat(&rawFormat); + int ret = raw.dev->tryFormat(&raw.format); if (ret) return Invalid; + /* * Some sensors change their Bayer order when they are h-flipped * or v-flipped, according to the transform. If this one does, we @@ -251,23 +245,15 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() * Note how we must fetch the "native" (i.e. untransformed) Bayer * order, because the sensor may currently be flipped! */ - V4L2PixelFormat fourcc = rawFormat.fourcc; + BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat); if (data_->flipsAlterBayerOrder_) { - BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc); bayer.order = data_->nativeBayerOrder_; bayer = bayer.transform(combinedTransform_); - fourcc = bayer.toV4L2PixelFormat(); } + raw.cfg->pixelFormat = bayer.toPixelFormat(); - PixelFormat inputPixFormat = fourcc.toPixelFormat(); - if (raw.cfg->size != rawFormat.size || raw.cfg->pixelFormat != inputPixFormat) { - raw.cfg->size = rawFormat.size; - raw.cfg->pixelFormat = inputPixFormat; + if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format)) status = Adjusted; - } - - raw.cfg->stride = rawFormat.planes[0].bpl; - raw.cfg->frameSize = rawFormat.planes[0].size; } /* Further fixups on the ISP output streams. */ @@ -544,35 +530,25 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config) * If the application has provided a sensor configuration apply it * instead of just applying a format. */ - const RPiCameraConfiguration *rpiConfig = - static_cast(config); - V4L2SubdeviceFormat sensorFormat; + RPiCameraConfiguration *rpiConfig = static_cast(config); + V4L2SubdeviceFormat *sensorFormat = &rpiConfig->sensorFormat_; if (rpiConfig->sensorConfig) { ret = data->sensor_->applyConfiguration(rpiConfig->sensorConfig, rpiConfig->combinedTransform_, - &sensorFormat); + sensorFormat); } else { - sensorFormat = rpiConfig->sensorFormat_; - ret = data->sensor_->setFormat(&sensorFormat, + ret = data->sensor_->setFormat(sensorFormat, rpiConfig->combinedTransform_); } if (ret) return ret; - /* Use the user requested packing/bit-depth. */ - std::optional packing; - if (!rpiConfig->rawStreams_.empty()) { - BayerFormat bayerFormat = - BayerFormat::fromPixelFormat(rpiConfig->rawStreams_[0].cfg->pixelFormat); - packing = bayerFormat.packing; - } - /* * Platform specific internal stream configuration. This also assigns * external streams which get configured below. */ - ret = data->platformConfigure(sensorFormat, packing, rpiConfig); + ret = data->platformConfigure(rpiConfig); if (ret) return ret; @@ -636,11 +612,11 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config) */ link->setEnabled(true); const MediaPad *sinkPad = link->sink(); - ret = device->setFormat(sinkPad->index(), &sensorFormat); + ret = device->setFormat(sinkPad->index(), sensorFormat); if (ret) { LOG(RPI, Error) << "Failed to set format on " << device->entity()->name() << " pad " << sinkPad->index() - << " with format " << sensorFormat + << " with format " << *sensorFormat << ": " << ret; return ret; } diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h index 491c5e98c4a1..0c4a59b7f590 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h @@ -58,9 +58,7 @@ public: } virtual CameraConfiguration::Status platformValidate(RPiCameraConfiguration *rpiConfig) const = 0; - virtual int platformConfigure(const V4L2SubdeviceFormat &sensorFormat, - std::optional packing, - const RPiCameraConfiguration *rpiConfig) = 0; + virtual int platformConfigure(const RPiCameraConfiguration *rpiConfig) = 0; virtual void platformStart() = 0; virtual void platformStop() = 0; @@ -269,6 +267,7 @@ public: unsigned int index; StreamConfiguration *cfg; V4L2VideoDevice *dev; + V4L2DeviceFormat format; }; std::vector rawStreams_; diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index 2308577a613b..1a03c88d7ee3 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -115,9 +115,7 @@ private: isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &ispCrop_); } - int platformConfigure(const V4L2SubdeviceFormat &sensorFormat, - std::optional packing, - const RPi::RPiCameraConfiguration *rpiConfig) override; + int platformConfigure(const RPi::RPiCameraConfiguration *rpiConfig) override; int platformConfigureIpa(ipa::RPi::ConfigParams ¶ms) override; int platformInitIpa([[maybe_unused]] ipa::RPi::InitParams ¶ms) override @@ -416,6 +414,9 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig /* Handle flips to make sure to match the RAW stream format. */ if (flipsAlterBayerOrder_) rawBayer = rawBayer.transform(rpiConfig->combinedTransform_); + + /* Apply the user requested packing. */ + rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing; PixelFormat rawFormat = rawBayer.toPixelFormat(); if (rawStream->pixelFormat != rawFormat || @@ -425,6 +426,9 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig status = CameraConfiguration::Adjusted; } + + rawStreams[0].format = + RPi::PipelineHandlerBase::toV4L2DeviceFormat(unicam_[Unicam::Image].dev(), rawStream); } /* @@ -500,23 +504,14 @@ int Vc4CameraData::platformPipelineConfigure(const std::unique_ptr & return 0; } -int Vc4CameraData::platformConfigure(const V4L2SubdeviceFormat &sensorFormat, - std::optional packing, - const RPi::RPiCameraConfiguration *rpiConfig) +int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfig) { const std::vector &rawStreams = rpiConfig->rawStreams_; const std::vector &outStreams = rpiConfig->outStreams_; int ret; - if (!packing) - packing = BayerFormat::Packing::CSI2; - V4L2VideoDevice *unicam = unicam_[Unicam::Image].dev(); - V4L2DeviceFormat unicamFormat = RPi::PipelineHandlerBase::toV4L2DeviceFormat(unicam, sensorFormat, *packing); - - ret = unicam->setFormat(&unicamFormat); - if (ret) - return ret; + V4L2DeviceFormat unicamFormat; /* * See which streams are requested, and route the user @@ -525,14 +520,24 @@ int Vc4CameraData::platformConfigure(const V4L2SubdeviceFormat &sensorFormat, if (!rawStreams.empty()) { rawStreams[0].cfg->setStream(&unicam_[Unicam::Image]); unicam_[Unicam::Image].setFlags(StreamFlag::External); + unicamFormat = rawStreams[0].format; + } else { + unicamFormat = + RPi::PipelineHandlerBase::toV4L2DeviceFormat(unicam, + rpiConfig->sensorFormat_, + BayerFormat::Packing::CSI2); } + ret = unicam->setFormat(&unicamFormat); + if (ret) + return ret; + ret = isp_[Isp::Input].dev()->setFormat(&unicamFormat); if (ret) return ret; LOG(RPI, Info) << "Sensor: " << sensor_->id() - << " - Selected sensor format: " << sensorFormat + << " - Selected sensor format: " << rpiConfig->sensorFormat_ << " - Selected unicam format: " << unicamFormat; /* Use a sensible small default size if no output streams are configured. */