From patchwork Fri Jan 20 16:22:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 18169 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 CC058C3295 for ; Fri, 20 Jan 2023 16:23:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AF10F625E8; Fri, 20 Jan 2023 17:23:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1674231785; bh=xu6QiBb8ZaqJ53QUDRFV5+0Fmt6NmkLQwqQKAam2u3g=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Qu6if45+y4yOCIU8dKGZYLS6jKWZn0B6g9tn3/rplS4Tc3RuRoqIsbLg4HyFGrnKd Ktz4Ra/XkEXRDh6BaYIS11fHVeaG1hG73dybWUTHyOotrptTo4qLKP8RQmi8VK4gNs Q9SyS7+FI3XPRqadkl24bdFvocxV9sGhdxXO2pxKly5cIgma2FoBF5YAlDUY5j+a6b BmuAO53OkCS57JWahmXtLcVDNEQHgED/6aZq3KEvTZ3ihJVE8BM+23r123/h4kW65Q YmkT5tc99ttDZcQPtCghStcU2inF21uqsaMV3hGY+2DbtRhY0tH4j9a4pomMVGTYUu AA3ctLD07z3cw== Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 132C261EFD for ; Fri, 20 Jan 2023 17:23:03 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="FMy3namU"; dkim-atps=neutral Received: by mail-wr1-x431.google.com with SMTP id h12so1309766wrv.10 for ; Fri, 20 Jan 2023 08:23:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ODoZgKfpxPA6bRL6CULiMN9sH5VsFqJLHMoJRBga7sk=; b=FMy3namUiun+CAWCXD92JQ+sQtZMuu7kWQnt9CNCha9PwySZuRG2ne2IU5SNXm7RdR GgA3j+XjwNlu4HktbVOy7t/6cD8NrMTNPW2XsIX4ZFiw6i+3hFreQ+5pc8sy0kLvr8IG 448vIdMETRGOC8nxXcnFwStrBlqH4YQ6urQE2sQXuz24PYONg0gVomrvpGSt6qC32d0/ JdNdkxVvcijp7RaDTIBAkWtCb4iQ5zmJD23jspghhhnwT+iuScfsQiwZIlsb0QqDynUK dkG6AIRRg0UwdDk55V2CTiKYBpg9fa537DWg/8hhTz+2gR3vW/n0gFusW7rZOy5yQ4h7 DPbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ODoZgKfpxPA6bRL6CULiMN9sH5VsFqJLHMoJRBga7sk=; b=Cb9EPlcqAk7ycsF7ZKKFyzl4Tq4ExpZQx35ukAnIlZA5eK4MEeDp1BLzDeDgnbBfdO 8UyI1znszbivxP707hHwTBKLttvAkNq2QWYrNyNNlmXyZTHVwlNr5wuZG7JsAyvlb/94 4BP/5EcVyqXk+aTHoj5GQpQYwIW+y88Y+ZVqrsUG0nGk6J0Rc6fjZFi8QnozG7CIQ85a AbW13OzpNWaD1dpiOg6EXfxhyzRnCQBpXMSnmmEL9jPdcuR/Lhb0+LdbW9uXbZgKAl6D OSfEjDzRxnTCrgNBjOLmI5IrTkGRZj0T/VoUkL1FqRdH4gcyaS7OD0lhF3jRwP1a5eSt nGeA== X-Gm-Message-State: AFqh2krXgR3DSFvcI86NTwJBy++LmZ0OI9LcvIY3uwk8wVP410SABtxT o+80FLTcrr4+tKRDsqJA3vNPzQbELJcHc//1 X-Google-Smtp-Source: AMrXdXued6fR/1NvmA5giejGzwYQfKNfp0bIzqivTeB0G1/b69KEpkDMMxeDh3dQMOYkXnbjvdxFuw== X-Received: by 2002:adf:fe03:0:b0:2be:516a:e6da with SMTP id n3-20020adffe03000000b002be516ae6damr3743653wrr.14.1674231782437; Fri, 20 Jan 2023 08:23:02 -0800 (PST) Received: from pi4-davidp.pitowers.org ([2a00:1098:3142:14:e4a2:3070:eea4:e434]) by smtp.gmail.com with ESMTPSA id co22-20020a0560000a1600b002a01e64f7a1sm12886596wrb.88.2023.01.20.08.23.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Jan 2023 08:23:02 -0800 (PST) To: libcamera-devel@lists.libcamera.org Date: Fri, 20 Jan 2023 16:22:58 +0000 Message-Id: <20230120162258.7039-2-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230120162258.7039-1-david.plowman@raspberrypi.com> References: <20230120162258.7039-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 1/1] pipeline: raspberrypi: Fix handling of colour spaces 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: David Plowman via libcamera-devel From: David Plowman Reply-To: David Plowman Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" We implement a custom validateColorSpaces method that forces all (non-raw) streams to same colour space, whilst distinguishing RGB streams from YUV ones, as the former must have the YCbCr encoding and range over-written. When we apply the colour space, we always send the full YUV version as that gets converted correctly to what our hardware drivers expect. It is also careful to check what comes back as the YCbCr information gets overwritten again on the way back. Signed-off-by: David Plowman Reviewed-by: Naushir Patuck Reviewed-by: Kieran Bingham Reviewed-by: Umang Jain --- .../pipeline/raspberrypi/raspberrypi.cpp | 115 +++++++++++++++++- 1 file changed, 114 insertions(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 8569df17..f768dced 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -310,6 +310,7 @@ class RPiCameraConfiguration : public CameraConfiguration public: RPiCameraConfiguration(const RPiCameraData *data); + CameraConfiguration::Status validateColorSpaces(ColorSpaceFlags flags); Status validate() override; /* Cache the combinedTransform_ that will be applied to the sensor */ @@ -317,6 +318,13 @@ public: private: const RPiCameraData *data_; + + /* + * Store the colour spaces that all our streams will have. RGB format streams + * will be the same but need the YCbCr fields cleared. + */ + std::optional yuvColorSpace_; + std::optional rgbColorSpace_; }; class PipelineHandlerRPi : public PipelineHandler @@ -357,6 +365,105 @@ RPiCameraConfiguration::RPiCameraConfiguration(const RPiCameraData *data) { } +static const std::vector validColorSpaces = { + ColorSpace::Sycc, + ColorSpace::Smpte170m, + ColorSpace::Rec709 +}; + +static std::optional findValidColorSpace(const ColorSpace &colourSpace) +{ + for (auto cs : validColorSpaces) { + if (colourSpace.primaries == cs.primaries && + colourSpace.transferFunction == cs.transferFunction) + return cs; + } + + return std::nullopt; +} + +static bool isRgb(const PixelFormat &pixFmt) +{ + const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); + return info.colourEncoding == PixelFormatInfo::ColourEncodingRGB; +} + +static bool isYuv(const PixelFormat &pixFmt) +{ + /* The code below would return true for raw mono streams, so weed those out first. */ + if (isRaw(pixFmt)) + return false; + + const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); + return info.colourEncoding == PixelFormatInfo::ColourEncodingYUV; +} + +/* + * Raspberry Pi drivers expect the following colour spaces: + * - V4L2_COLORSPACE_RAW for raw streams. + * - One of V4L2_COLORSPACE_JPEG, V4L2_COLORSPACE_SMPTE170M, V4L2_COLORSPACE_REC709 for + * non-raw streams. Other fields such as transfer function, YCbCr encoding and + * quantisation are not used. + * + * The libcamera colour spaces that we wish to use corresponding to these are therefore: + * - ColorSpace::Raw for V4L2_COLORSPACE_RAW + * - ColorSpace::Sycc for V4L2_COLORSPACE_JPEG + * - ColorSpace::Smpte170m for V4L2_COLORSPACE_SMPTE170M + * - ColorSpace::Rec709 for V4L2_COLORSPACE_REC709 + */ + +CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_unused]] ColorSpaceFlags flags) +{ + Status status = Valid; + yuvColorSpace_.reset(); + + for (auto cfg : config_) { + /* First fix up raw streams to have the "raw" colour space. */ + if (isRaw(cfg.pixelFormat)) { + /* If there was no value here, that doesn't count as "adjusted". */ + if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw) { + status = Adjusted; + cfg.colorSpace = ColorSpace::Raw; + } + continue; + } + + /* Next we need to find our shared colour space. The first valid one will do. */ + if (cfg.colorSpace && !yuvColorSpace_) + yuvColorSpace_ = findValidColorSpace(cfg.colorSpace.value()); + } + + /* If no colour space was given anywhere, choose sYCC. */ + if (!yuvColorSpace_) + yuvColorSpace_ = ColorSpace::Sycc; + + /* Note the version of this that any RGB streams will have to use. */ + rgbColorSpace_ = yuvColorSpace_; + rgbColorSpace_->ycbcrEncoding = ColorSpace::YcbcrEncoding::None; + rgbColorSpace_->range = ColorSpace::Range::Full; + + /* Go through the streams again and force everyone to the same colour space. */ + for (auto cfg : config_) { + if (cfg.colorSpace == ColorSpace::Raw) + continue; + + if (isYuv(cfg.pixelFormat) && cfg.colorSpace != yuvColorSpace_) { + /* Again, no value means "not adjusted". */ + if (cfg.colorSpace) + status = Adjusted; + cfg.colorSpace = yuvColorSpace_; + } + if (isRgb(cfg.pixelFormat) && cfg.colorSpace != rgbColorSpace_) { + /* Be nice, and let the YUV version count as non-adjusted too. */ + if (cfg.colorSpace && cfg.colorSpace != yuvColorSpace_) + status = Adjusted; + cfg.colorSpace = rgbColorSpace_; + } + } + + return status; +} + CameraConfiguration::Status RPiCameraConfiguration::validate() { Status status = Valid; @@ -533,7 +640,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() V4L2DeviceFormat format; format.fourcc = dev->toV4L2PixelFormat(cfg.pixelFormat); format.size = cfg.size; - format.colorSpace = cfg.colorSpace; + /* We want to send the associated YCbCr info through to the driver. */ + format.colorSpace = yuvColorSpace_; LOG(RPI, Debug) << "Try color space " << ColorSpace::toString(cfg.colorSpace); @@ -542,6 +650,11 @@ CameraConfiguration::Status RPiCameraConfiguration::validate() 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) { status = Adjusted; LOG(RPI, Debug)