From patchwork Sat Sep 16 12:19:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 19048 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 B4356C3260 for ; Sat, 16 Sep 2023 12:19:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 68E5162922; Sat, 16 Sep 2023 14:19:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1694866790; bh=joF5SwOXNjXn9yy67Ol4wUMncn/0xR5d/g+WVYRbqCc=; 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=4ZXo51oIKOqOEk8XS7pqVnzUM9y8W02xex4WKdki92RrNvyU2PgJqtcJWdPPFeClz Jg6Peqane45z15peNehU1T/kSERzXbDozgjBH++uNdywBKKEbQfgRHUITfFOm3m0oJ dp/ZvYvW3lt97/kvSZ0Pzt2NEVeU+xeWE+0yeMbqkHjWjCjgl3oo32hzkinaQQdUVi DBvvG71ay4aQ69ULPnO5wLYUySwJVp4VMYlPxd+cS8R86YnmjMLLLzfrYK/3EQMuy3 PU9JrOQyWUlme/e4X4/BW+yaximLRGZfeSPRPQ2S82t38wT+4q+bDkR4Tvxa9L6UOW spLZGqdhciLmQ== 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 9A633628E9 for ; Sat, 16 Sep 2023 14:19:41 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="gVHP36gj"; 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 85D2F8BE; Sat, 16 Sep 2023 14:18:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1694866687; bh=joF5SwOXNjXn9yy67Ol4wUMncn/0xR5d/g+WVYRbqCc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gVHP36gjQaRgdyqb93iQRQtCwkomkTd39/Q168g/65Wl8OAfuZ3skRZ6xyAnYGJcL 5HasBl371kAAt4ajnnOVSutegKJG9P84KT/R00dxEJ4llGU4nrbIsKur0mIcce1cGt 45VhTxGApBlaVJqiyPsbfKgVnr+NoxMQzlOUMLK0= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Sep 2023 14:19:28 +0200 Message-ID: <20230916121930.29490-11-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20230916121930.29490-1-jacopo.mondi@ideasonboard.com> References: <20230916121930.29490-1-jacopo.mondi@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 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 93779556fd81..9e4411f22902 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.populated()) { 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. */