From patchwork Tue Jan 3 11:33:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 18084 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 3FAA0C322E for ; Tue, 3 Jan 2023 11:33:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0ABAB625E0; Tue, 3 Jan 2023 12:33:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672745602; bh=o4BOMLWxJvJFUCvfULtZKYyBaQJo6F9yjiZJ8Jom17E=; 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=DW2lIHcMvrOb+bLbwzhfkP/IP2fkq6mzqx2doIgzjwwOh8rFBJuV8mbP3b1J73Yie 9YpL8onvA5YrzUH6qWhqirmAlCryIFVqL7gfwh8WJHHcW3zKOgiIXM+t+E7aF7N2Bt 5vvNkk1XdwqGiX968vYKM7+Lrv7KmUAs1JIJC036aHnl2bPYwtFTY0GAiTGeHhYRAK A6KHu3E19ONYQG/Hp078TGVP3zrIwOpNHooegVmiU6QoUPxlH+lNSQt+xyWWo4OacB UEHGXu4ZpO9zwkJQfXurd1Xh+BDkEEUheI2/K4DgnWRjImzf4dV4vwIkkaIWl1DteG Lansziqff41GQ== Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 89095625D0 for ; Tue, 3 Jan 2023 12:33:18 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="guxvVaRG"; dkim-atps=neutral Received: by mail-wm1-x333.google.com with SMTP id bi26-20020a05600c3d9a00b003d3404a89faso16234212wmb.1 for ; Tue, 03 Jan 2023 03:33:18 -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=OZnPtLMfECcbSAEc2EbvCqvg4a7QEBg0zPwaEhOiIRQ=; b=guxvVaRG7EqRaF077B+IQsUZ0a4YNUomiDfcH35Js3UUxqixp8Yxle5tzDU5P24UlD w8N5xz/E4wmXci4i4lrdz/1r5XENhQVy6lMHMSiomk0Lc16QDg36lj+WIXHy8p25FvTe hms9T5I6+9bjS0UxCjcYnuEM+DQz+opg3kHfl0aEGLxn+9jdwXjbYpGMZC9FVCYyioBk n4ppMgaezgX7rXaGnd6Xcl0HeFenlSZBFybUpy05xBPa4CtcrioNpym6hcT6N66/73Z1 Vt8oIFDV/xBXXMtvnzU+UJsNnPYuq5jH7S0HSI/OgMN85Vj1GgOZGP45Jm+LkTN33obF +lUg== 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=OZnPtLMfECcbSAEc2EbvCqvg4a7QEBg0zPwaEhOiIRQ=; b=Gj3nTsDoG+D5y1NXj4Qwvr7GJYMiAkrtQ85NnQioHZUu6AvhUA2uNVffg7q/kWoYCn MVQK0DFPQlGQ/tWaE9LLhk1HcUaEVqTDuP3TNkDBtE9ki67zgj4L2QpTSvMHm2hHin18 BNUuOP+GhruWbe/tsowkCnh8UdaOp5+MAWGlCtvYUAhaQiHFuWlxj6/+o8TLaSdQ4RIs Xa0yFKlI1ntkoHq55ZiYmy2HYVbkYfwE8+txMkY4qs0fyHhSEd+RT+TpEYiOGd3NbYY4 vnD5Sgsr+7Qlq706qtqBCcujcwBMPutidrnmwItHZxJtgKLonfdnzQAf+2M/jRUt0nuk cYXQ== X-Gm-Message-State: AFqh2kqiQ9mfncAGeApnBmNZkZBuy1nLt4z4Bn4Vl8keCulhLTzvr76s zIeJ6IvI0tPLGGMVg5R/3mMvmvJOSbt6AUf4 X-Google-Smtp-Source: AMrXdXveEBUrDeI5QZnaQG0o9DwV5bxCxwBdop/1yG8plPNDuUHsOdMpNYgb6ylZ1o4BeuPr0diTqg== X-Received: by 2002:a05:600c:1d20:b0:3d2:2aaf:316 with SMTP id l32-20020a05600c1d2000b003d22aaf0316mr31451581wms.36.1672745597894; Tue, 03 Jan 2023 03:33:17 -0800 (PST) Received: from pi4-davidp.pitowers.org ([2a00:1098:3142:14:e4a2:3070:eea4:e434]) by smtp.gmail.com with ESMTPSA id h10-20020a5d4fca000000b00281eab50380sm24325767wrw.117.2023.01.03.03.33.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Jan 2023 03:33:17 -0800 (PST) To: libcamera-devel@lists.libcamera.org Date: Tue, 3 Jan 2023 11:33:13 +0000 Message-Id: <20230103113313.5423-3-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230103113313.5423-1-david.plowman@raspberrypi.com> References: <20230103113313.5423-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/2] 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. Signed-off-by: David Plowman Reviewed-by: Naushir Patuck --- .../pipeline/raspberrypi/raspberrypi.cpp | 99 ++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 8569df17..bb574afc 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,89 @@ 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) +{ + /* + * Be careful not to use this when a format might be raw because it returns + * the wrong result for mono raw streams. + */ + const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); + return info.colourEncoding == PixelFormatInfo::ColourEncodingRGB; +} + +static bool isYuv(const PixelFormat &pixFmt) +{ + const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); + return info.colourEncoding == PixelFormatInfo::ColourEncodingYUV; +} + +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) && cfg.colorSpace != ColorSpace::Raw) { + /* If there was no value here, that doesn't count as "adjusted". */ + if (cfg.colorSpace) + status = Adjusted; + cfg.colorSpace = ColorSpace::Raw; + } + + /* Next we need to find our shared colour space. The first valid one will do. */ + if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw && !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 +624,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 +634,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)