From patchwork Thu Jan 12 12:10:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 18105 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 AA0A8C3240 for ; Thu, 12 Jan 2023 12:10:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 71206625D0; Thu, 12 Jan 2023 13:10:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1673525450; bh=hizRl+QQXRusQc9M2VGOo2HO8oWrMuXiKXH7ZL/mdhk=; 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=VdVp7ZrpScQ8ni8YQ1iUyHD4C0ga9JmKKTMGqQFRxWAXFDoMCT6+R9cC8yvd5aaZM zmAU4UrRXoqD8gjb6GW5139X+mhycVKERCdKNwvSgBJpbdXxrcAafuPCY7xZqADK4p kLt3ec0RpZo9YpXIj33jCijUY6LTM3lQZpNenx+XPTsP7iU1aACGfpbKVTZ7BZHRRZ rvbY2rb1aVdta3c5nrWXK9VW9IDkPgDLckZv6zB0+A0cEmS2pGyPVoYA7NwxK6H1W4 oILf8KXc0+33dG7K+lEf+Lm6v146r1AkqVtVxSMadJb5NC6l5PZ1BXkia9eudapjKh 4CWhQbTyIo0+w== Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 77E3E625D0 for ; Thu, 12 Jan 2023 13:10:48 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="Sb600y+w"; dkim-atps=neutral Received: by mail-wr1-x434.google.com with SMTP id h16so17865688wrz.12 for ; Thu, 12 Jan 2023 04:10:48 -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=tSvScteCJTU51CG11US5YExJOPxoCn3drjUiubOkSBc=; b=Sb600y+wGGkxbxvdZib8R13ysvwjHYx1N654XI2oBgzr3I2r8nHPktfr8h1LOXYMUe M3RxoBnffkojHMeT9mZpKeQEETxpNwrEIQEVw6/CjTpiS0vl69ZUfdQsfjQq8URXCkjh J53X95rZgy0ZQ1Qb13L9GjKBU8HhGKQIzsulN/xjggKgkTOxbecK5ZRtBmaT+tCKI2YE bZVBEUjnsvQN1Qys4FsXbrU9lxaO7yOQUwPiDmjIipQtaqqN/E+FEgBe6wzMnbxTSb0y HhJYfuWJkLEK0Aex7uZvS332ornVde1dc9ks3l0EFu5feLxnTs9Qc+ExAlhCubY4DKuZ 27Bw== 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=tSvScteCJTU51CG11US5YExJOPxoCn3drjUiubOkSBc=; b=r45z+hVFpTwHZ0+IEFFvcofSG54Os2WPXP/VmQMleYNsOjcwYuQG5UWOeevAy4Xsvh /1eInz3+YLWu/DkvjGM736lfy/VG31zmD6yJTADl9/7GrDS5WMzyCXSSih5ysCzhbnnz 1qGUgfxkWApTNjR3k6z+BkYZ6gk61GR1ITPZFEVXlCP9D82fKPRR5OAzDFTmSOqBzUPb 6NKXKsKkQjhb9lsrkPyV5MqCo1Xlw4SDQ7u7oF4dEHVagjjmlHL1dOenMOGsSD2z9L25 DF7DVFtTPVsNdBO/ci811y27yWgq7V1GWVFobIqT3dgBaH956Yv1m7EYUgJ7YyAQLUAM Xklg== X-Gm-Message-State: AFqh2kpcC5oyNDBklfwQaGf1DRW93vX/xyni5w7CTXFXHMAQiSOc0GNk 86stVVS0qpd8S32JzPxRAXpf9eLYnOhuUahK X-Google-Smtp-Source: AMrXdXs6eyzKbht06BKq9zbbQwOcZ/yiMsZ778jfjcaAm6ZuV0YYTDtHdp+OQmjge4yYZlatocuJ7A== X-Received: by 2002:a5d:5506:0:b0:2bb:dd02:46ef with SMTP id b6-20020a5d5506000000b002bbdd0246efmr10536038wrv.52.1673525447823; Thu, 12 Jan 2023 04:10:47 -0800 (PST) Received: from pi4-davidp.pitowers.org ([2a00:1098:3142:14:e4a2:3070:eea4:e434]) by smtp.gmail.com with ESMTPSA id l7-20020a5d6747000000b002b57bae7174sm16318578wrw.5.2023.01.12.04.10.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Jan 2023 04:10:47 -0800 (PST) To: libcamera-devel@lists.libcamera.org Date: Thu, 12 Jan 2023 12:10:44 +0000 Message-Id: <20230112121044.28003-3-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230112121044.28003-1-david.plowman@raspberrypi.com> References: <20230112121044.28003-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 on the way back. 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..135024e7 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) +{ + 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; +} + +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)