From patchwork Fri Sep 15 13:06:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 19030 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 A3A13C32B0 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 65AF46292F; Fri, 15 Sep 2023 15:07:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1694783241; bh=t05fFLPiScMNjDYmygJ5MwL6tWEYKYUaHsg2TlIfDeY=; 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=N/r4kDjYC9qBrWtkk/93oMI2VGFCp4L3/ZjNcta4SvE9xTWBqFQ0oqqbRhebQiENT naB5vPo9owUR8JktJ0aZjTs/eyb6OfwLRTYgdV7Xiijxi7uk3iCfzKA8+E1SXs3BjY 3jDTcXla2Hqtov+6vTC0d0y0kLShi87hZtyAlId2rH1x1opYANWj08xzXAduKFMnav aJCNsGOTpA8k9N7xizvZ6Vq+188DNTWaXDBYyY8Hp+LfiaBTiCZrwCNhCpHpU7N8B5 GHBFXwKJsoZSAFQK6XD3rbbqWEFzV8euROTj9QBfWbkjwHshrGVqEukpbAbEGucAxq qK05khYxi68SA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EE3F362928 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="MDXXHnRO"; 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 896A62F6C; Fri, 15 Sep 2023 15:05:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1694783139; bh=t05fFLPiScMNjDYmygJ5MwL6tWEYKYUaHsg2TlIfDeY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MDXXHnROG/E5ttB4RGuQvj1SRoIogwuia5tevmA91cQfCKgkMANTPXDHZeLhlijtc ZV2PU+5MosT+HCGlmeIzAj/wn1sDSG1KTizVMLrPW7spymZ1h0w4dQRGpoTWh+mvjC UrA0g/aYlPuxh9buEqCVOGNl2gB67efkRsZNUijA= To: libcamera-devel@lists.libcamera.org Date: Fri, 15 Sep 2023 15:06:50 +0200 Message-ID: <20230915130650.35691-13-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 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 0de91123ee50..51d54e57ceaf 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);