From patchwork Sat Sep 16 12:19:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 19050 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 D610FBD160 for ; Sat, 16 Sep 2023 12:19:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7BB2F6292B; Sat, 16 Sep 2023 14:19:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1694866791; bh=c2GvTrMZTJMcatCGIVz4vOVZW5KTZwYWXeD5Zy0NktE=; 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=Ut/RqphhMnBwM8uj3SCSHy4YbqZPExcQuGrUsdojRr2OfF55Pi1U1Vws2Xxfufl6R GYIorF59rGe/WpRIgIl6B5n9P2fCkuPWtO8ZiTWYFINYDjW7tRXDVPIts6dnQzdFVq q3Ru2HDNjXwwnmX6DLZxwa/V9qn1bFPxiyIvlDUadOkVBW0+RvpWnLc3o7ftyI/4Hj BS01wsy52y8WvQz6WWj4FCUPeRzmjPkMd6yE5w+GfZE9KJK4d7Foj3ajW6M2jYm/IU 7GU1tCfmb6GZXKIzvX9pVDN0iGDAjFzGw1NhYRZwDqQZmJx8Y5gACJ+/QIxSPxd50m WAxw/E7UvBkbw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 73DBF6292C for ; Sat, 16 Sep 2023 14:19:42 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="R36eqtMi"; 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 5E5181536; Sat, 16 Sep 2023 14:18:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1694866688; bh=c2GvTrMZTJMcatCGIVz4vOVZW5KTZwYWXeD5Zy0NktE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=R36eqtMi4VHmG4vRfdvSLey2yTwIizrrJ6EIiPE1PPO9kWVSuTt1LtonbwBQj5kQI n6Yg1H8MxxtVUe+xWCmiuYKXBjsMnx93VN3Kz8FdnUR422O1N7b9qQO92iMJ3xM8QU CUI/ubpxSuaA610om+ZTnhQtn0fptz37KIrlZgcs= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Sep 2023 14:19:30 +0200 Message-ID: <20230916121930.29490-13-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 12/12] libcamera: rpi: Simplify validate() and configure() for YUV/RGB 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: - Determine the V4L2DeviceFormat structure fields for all streams in validate(), cache them and reuse in configure() instead of re-generating this structure multiple times. - Remove setting a default pixel format in validate(), this code patch will not be used. - 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 | 49 ++++--------------- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 15 ++---- 2 files changed, 12 insertions(+), 52 deletions(-) diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index c69e076aac71..653faff12353 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -258,56 +258,25 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() /* Further fixups on the ISP output streams. */ for (auto &out : outStreams_) { - StreamConfiguration &cfg = config_.at(out.index); - PixelFormat &cfgPixFmt = cfg.pixelFormat; - V4L2VideoDevice::Formats fmts = out.dev->formats(); - - if (fmts.find(out.dev->toV4L2PixelFormat(cfgPixFmt)) == fmts.end()) { - /* If we cannot find a native format, use a default one. */ - cfgPixFmt = formats::YUV420; - status = Adjusted; - } - - V4L2DeviceFormat format; - format.fourcc = out.dev->toV4L2PixelFormat(cfg.pixelFormat); - format.size = cfg.size; /* - * platformValidate may have worked out the correct stride so we - * must pass it in. This also needs the planesCount to be set - * correctly or the stride will be ignored. + * We want to send the associated YCbCr info through to the driver. + * + * But for RGB streams, the YCbCr info gets overwritten on the way back + * so we must check against what the stream cfg says, not what we actually + * requested (which carefully included the YCbCr info)! */ - const PixelFormat &pixFormat = format.fourcc.toPixelFormat(); - const PixelFormatInfo &info = PixelFormatInfo::info(pixFormat); - format.planesCount = info.numPlanes(); - format.planes[0].bpl = cfg.stride; - - /* We want to send the associated YCbCr info through to the driver. */ - format.colorSpace = yuvColorSpace_; + out.format.colorSpace = yuvColorSpace_; LOG(RPI, Debug) - << "Try color space " << ColorSpace::toString(cfg.colorSpace); + << "Try color space " << ColorSpace::toString(out.cfg->colorSpace); - int ret = out.dev->tryFormat(&format); + int ret = out.dev->tryFormat(&out.format); if (ret) return Invalid; - /* - * But for RGB streams, the YCbCr info gets overwritten on the way back - * so we must check against what the stream cfg says, not what we actually - * requested (which carefully included the YCbCr info)! - */ - if (cfg.colorSpace != format.colorSpace) { + if (RPi::PipelineHandlerBase::updateStreamConfig(out.cfg, out.format)) status = Adjusted; - LOG(RPI, Debug) - << "Color space changed from " - << ColorSpace::toString(cfg.colorSpace) << " to " - << ColorSpace::toString(format.colorSpace); - } - - cfg.colorSpace = format.colorSpace; - cfg.stride = format.planes[0].bpl; - cfg.frameSize = format.planes[0].size; } return status; diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index 1a03c88d7ee3..a1813ba65725 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -457,6 +457,8 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig * have that fixed up in the code above. */ outStreams[i].dev = isp_[i == 0 ? Isp::Output0 : Isp::Output1].dev(); + + outStreams[i].format = RPi::PipelineHandlerBase::toV4L2DeviceFormat(outStreams[i].dev, outStreams[i].cfg); } return status; @@ -549,11 +551,7 @@ int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi /* The largest resolution gets routed to the ISP Output 0 node. */ RPi::Stream *stream = i == 0 ? &isp_[Isp::Output0] : &isp_[Isp::Output1]; - - V4L2PixelFormat fourcc = stream->dev()->toV4L2PixelFormat(cfg->pixelFormat); - format.size = cfg->size; - format.fourcc = fourcc; - format.colorSpace = cfg->colorSpace; + format = outStreams[i].format; LOG(RPI, Debug) << "Setting " << stream->name() << " to " << format; @@ -562,13 +560,6 @@ int Vc4CameraData::platformConfigure(const RPi::RPiCameraConfiguration *rpiConfi if (ret) return -EINVAL; - if (format.size != cfg->size || format.fourcc != fourcc) { - LOG(RPI, Error) - << "Failed to set requested format on " << stream->name() - << ", returned " << format; - return -EINVAL; - } - LOG(RPI, Debug) << "Stream " << stream->name() << " has color space " << ColorSpace::toString(cfg->colorSpace);