[{"id":34165,"web_url":"https://patchwork.libcamera.org/comment/34165/","msgid":"","date":"2025-05-09T10:59:56","subject":"Re: [PATCH v4 03/11] libcamera: formats: Add a helper to check for a\n\traw pixel format","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 04. 07. 10:56 keltezéssel, Milan Zamazal írta:\n> There are several places with the same pattern to check whether a given\n> pixel format is a raw format:\n> \n> return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n> libcamera::PixelFormatInfo::ColourEncodingRAW;\n> \n> Let's move the corresponding isFormatRaw helper from mali-c55.cpp to\n> formats.cpp and use it where applicable. This also avoids a need to\n> introduce the same helper (or the same pattern) in the followup patches.\n\nAs far as I can see, there are a lot of other places where this check is \"open coded\",\nI believe either all of them or none of them should be replaced. For example,\n`IPARkISP1::configure()`, `ISICameraConfiguration::validate()`,\n`IPU3CameraConfiguration::validate()`, etc.\n\nAnd there is also `PipelineHandlerBase::isRaw()` in the rpi pipeline.\n\nI am wondering if adding `PixelFormatInfo::isRaw()` could be useful as well.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> include/libcamera/internal/formats.h | 2 ++\n> src/libcamera/formats.cpp | 11 +++++++++++\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 10 ----------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++-----\n> 5 files changed, 17 insertions(+), 17 deletions(-)\n> \n> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n> index 6a3e9c16..bc4417d0 100644\n> --- a/include/libcamera/internal/formats.h\n> +++ b/include/libcamera/internal/formats.h\n> @@ -62,4 +62,6 @@ public:\n> \tstd::array planes;\n> };\n> \n> +bool isFormatRaw(const libcamera::PixelFormat &pixFmt);\n> +\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index bfcdfc08..c6e0a262 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -1215,4 +1215,15 @@ unsigned int PixelFormatInfo::numPlanes() const\n> \treturn count;\n> }\n> \n> +/**\n> + * \\brief Return whether the given pixel format is a raw format\n> + * \\param[in] pixFmt The pixel format to examine\n> + * \\return True iff the given format is a raw format\n> + */\n> +bool isFormatRaw(const libcamera::PixelFormat &pixFmt)\n> +{\n> +\treturn libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n> +\t libcamera::PixelFormatInfo::ColourEncodingRAW;\n> +}\n> +\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 4e66b336..9ff11a41 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -24,6 +24,7 @@\n> #include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> @@ -312,8 +313,7 @@ unsigned int ISICameraData::getYuvMediaBusFormat(const PixelFormat &pixelFormat)\n> \n> unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const\n> {\n> -\tif (PixelFormatInfo::info(*pixelFormat).colourEncoding ==\n> -\t PixelFormatInfo::ColourEncodingRAW)\n> +\tif (isFormatRaw(*pixelFormat))\n> \t\treturn getRawMediaBusFormat(pixelFormat);\n> \n> \treturn getYuvMediaBusFormat(*pixelFormat);\n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index a05e11fc..3721fb30 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -42,16 +42,6 @@\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> -namespace {\n> -\n> -bool isFormatRaw(const libcamera::PixelFormat &pixFmt)\n> -{\n> -\treturn libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n> -\t libcamera::PixelFormatInfo::ColourEncodingRAW;\n> -}\n> -\n> -} /* namespace */\n> -\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(MaliC55)\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 52633fe3..a5b613bb 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -536,8 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \t */\n> \tif (config_.size() > 1) {\n> \t\tfor (const auto &cfg : config_) {\n> -\t\t\tif (PixelFormatInfo::info(cfg.pixelFormat).colourEncoding ==\n> -\t\t\t PixelFormatInfo::ColourEncodingRAW) {\n> +\t\t\tif (isFormatRaw(cfg.pixelFormat)) {\n> \t\t\t\tconfig_.resize(1);\n> \t\t\t\tstatus = Adjusted;\n> \t\t\t\tbreak;\n> @@ -551,9 +550,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \t\t * Platforms with dewarper support, such as i.MX8MP, support\n> \t\t * only a single stream. We can inspect config_[0] only here.\n> \t\t */\n> -\t\tbool isRaw = PixelFormatInfo::info(config_[0].pixelFormat).colourEncoding ==\n> -\t\t\t PixelFormatInfo::ColourEncodingRAW;\n> -\t\tif (!isRaw)\n> +\t\tif (!isFormatRaw(config_[0].pixelFormat))\n> \t\t\tuseDewarper = true;\n> \t}\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5FE71C3226\n\tfor ;\n\tFri, 9 May 2025 11:00:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5ED3F68B45;\n\tFri, 9 May 2025 13:00:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB5B168B25\n\tfor ;\n\tFri, 9 May 2025 13:00:01 +0200 (CEST)","from [192.168.33.12] (185.221.140.100.nat.pool.zt.hu\n\t[185.221.140.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B16508DB;\n\tFri, 9 May 2025 12:59:47 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"e7EHoFp7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1746788389;\n\tbh=mLo9mKKosAcnuPi5r+hFEq8e1PvgU0ySSW3UozBzvy8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=e7EHoFp7reDT0fPBzhhTxxWHN8YeR98MZJsyRS6hUdUku54s2/PQoIYYt7CHMrOY6\n\taPWeIzWmUkP++kM+rm49JBX/LaPMvODJIlF2zOojW2C+WuMPpld3RrCLIJEmePa6Fk\n\tSb1AmuJBxb9FoXGK9qpDRIDFoAvzXzT/FIDvFSPA=","Message-ID":"","Date":"Fri, 9 May 2025 12:59:56 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 03/11] libcamera: formats: Add a helper to check for a\n\traw pixel format","To":"Milan Zamazal ","Cc":"Laurent Pinchart ,\n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250407085639.16180-1-mzamazal@redhat.com>\n\t<20250407085639.16180-4-mzamazal@redhat.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250407085639.16180-4-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34214,"web_url":"https://patchwork.libcamera.org/comment/34214/","msgid":"<85jz6lhqxr.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-05-13T08:10:08","subject":"Re: [PATCH v4 03/11] libcamera: formats: Add a helper to check for\n\ta raw pixel format","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Barnabás,\n\nthank you for review.\n\nBarnabás Pőcze writes:\n\n> Hi\n>\n> 2025. 04. 07. 10:56 keltezéssel, Milan Zamazal írta:\n>> There are several places with the same pattern to check whether a given\n>> pixel format is a raw format:\n>> return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n>> libcamera::PixelFormatInfo::ColourEncodingRAW;\n>> Let's move the corresponding isFormatRaw helper from mali-c55.cpp to\n>> formats.cpp and use it where applicable. This also avoids a need to\n>> introduce the same helper (or the same pattern) in the followup patches.\n>\n> As far as I can see, there are a lot of other places where this check is \"open coded\",\n> I believe either all of them or none of them should be replaced. For example,\n> `IPARkISP1::configure()`, `ISICameraConfiguration::validate()`,\n> `IPU3CameraConfiguration::validate()`, etc.\n>\n> And there is also `PipelineHandlerBase::isRaw()` in the rpi pipeline.\n\nIIRC I tried to replace only the given pattern and not simpler\nconstructs like when the info is already retrieved, to not complicate\nthe patches more than necessary. But the rest can certainly be also\nreplaced, I can do it in v5 if there are no objections.\n\n> I am wondering if adding `PixelFormatInfo::isRaw()` could be useful as well.\n\nI think so, with the broader replacement.\n\n> Regards,\n> Barnabás Pőcze\n>\n>\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> include/libcamera/internal/formats.h | 2 ++\n>> src/libcamera/formats.cpp | 11 +++++++++++\n>> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 4 ++--\n>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 10 ----------\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++-----\n>> 5 files changed, 17 insertions(+), 17 deletions(-)\n>> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n>> index 6a3e9c16..bc4417d0 100644\n>> --- a/include/libcamera/internal/formats.h\n>> +++ b/include/libcamera/internal/formats.h\n>> @@ -62,4 +62,6 @@ public:\n>> \tstd::array planes;\n>> };\n>> +bool isFormatRaw(const libcamera::PixelFormat &pixFmt);\n>> +\n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n>> index bfcdfc08..c6e0a262 100644\n>> --- a/src/libcamera/formats.cpp\n>> +++ b/src/libcamera/formats.cpp\n>> @@ -1215,4 +1215,15 @@ unsigned int PixelFormatInfo::numPlanes() const\n>> \treturn count;\n>> }\n>> +/**\n>> + * \\brief Return whether the given pixel format is a raw format\n>> + * \\param[in] pixFmt The pixel format to examine\n>> + * \\return True iff the given format is a raw format\n>> + */\n>> +bool isFormatRaw(const libcamera::PixelFormat &pixFmt)\n>> +{\n>> +\treturn libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n>> +\t libcamera::PixelFormatInfo::ColourEncodingRAW;\n>> +}\n>> +\n>> } /* namespace libcamera */\n>> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> index 4e66b336..9ff11a41 100644\n>> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n>> @@ -24,6 +24,7 @@\n>> #include \"libcamera/internal/camera.h\"\n>> #include \"libcamera/internal/camera_sensor.h\"\n>> #include \"libcamera/internal/device_enumerator.h\"\n>> +#include \"libcamera/internal/formats.h\"\n>> #include \"libcamera/internal/media_device.h\"\n>> #include \"libcamera/internal/pipeline_handler.h\"\n>> #include \"libcamera/internal/v4l2_subdevice.h\"\n>> @@ -312,8 +313,7 @@ unsigned int ISICameraData::getYuvMediaBusFormat(const PixelFormat &pixelFormat)\n>> unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const\n>> {\n>> -\tif (PixelFormatInfo::info(*pixelFormat).colourEncoding ==\n>> -\t PixelFormatInfo::ColourEncodingRAW)\n>> +\tif (isFormatRaw(*pixelFormat))\n>> \t\treturn getRawMediaBusFormat(pixelFormat);\n>> \treturn getYuvMediaBusFormat(*pixelFormat);\n>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> index a05e11fc..3721fb30 100644\n>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>> @@ -42,16 +42,6 @@\n>> #include \"libcamera/internal/v4l2_subdevice.h\"\n>> #include \"libcamera/internal/v4l2_videodevice.h\"\n>> -namespace {\n>> -\n>> -bool isFormatRaw(const libcamera::PixelFormat &pixFmt)\n>> -{\n>> -\treturn libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n>> -\t libcamera::PixelFormatInfo::ColourEncodingRAW;\n>> -}\n>> -\n>> -} /* namespace */\n>> -\n>> namespace libcamera {\n>> LOG_DEFINE_CATEGORY(MaliC55)\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 52633fe3..a5b613bb 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -536,8 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>> \t */\n>> \tif (config_.size() > 1) {\n>> \t\tfor (const auto &cfg : config_) {\n>> -\t\t\tif (PixelFormatInfo::info(cfg.pixelFormat).colourEncoding ==\n>> -\t\t\t PixelFormatInfo::ColourEncodingRAW) {\n>> +\t\t\tif (isFormatRaw(cfg.pixelFormat)) {\n>> \t\t\t\tconfig_.resize(1);\n>> \t\t\t\tstatus = Adjusted;\n>> \t\t\t\tbreak;\n>> @@ -551,9 +550,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>> \t\t * Platforms with dewarper support, such as i.MX8MP, support\n>> \t\t * only a single stream. We can inspect config_[0] only here.\n>> \t\t */\n>> -\t\tbool isRaw = PixelFormatInfo::info(config_[0].pixelFormat).colourEncoding ==\n>> -\t\t\t PixelFormatInfo::ColourEncodingRAW;\n>> -\t\tif (!isRaw)\n>> +\t\tif (!isFormatRaw(config_[0].pixelFormat))\n>> \t\t\tuseDewarper = true;\n>> \t}\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 10C5BC3200\n\tfor ;\n\tTue, 13 May 2025 08:10:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 15AC768B55;\n\tTue, 13 May 2025 10:10:17 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7276F68B51\n\tfor ;\n\tTue, 13 May 2025 10:10:15 +0200 (CEST)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-374-1CjLcFydMcanzyqfg5plRw-1; Tue, 13 May 2025 04:10:12 -0400","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-442d472cf84so24434255e9.2\n\tfor ;\n\tTue, 13 May 2025 01:10:12 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb ([85.93.96.130])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-442d67efd26sm157217205e9.23.2025.05.13.01.10.08\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 13 May 2025 01:10:08 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"fdHuxA69\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1747123814;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=rn1n4tNENW0x4SlAjJ8pfqNtvtAaA9G8HtYTFn6jZX0=;\n\tb=fdHuxA69ONKvK4QTtIIGzAom6V499HSIwfr2/bKrA7O0Y7w1uOU406Z5vjFIABEideO1+r\n\tOxqumnO4ElqpYZmT64SoItNnXjiwJfn0Or0lDGPymMFxz6USSw9AsegnEi7A15lioQ1ogo\n\tokZyvpQYEwdqKbOghwlo506e+oV0XHE=","X-MC-Unique":"1CjLcFydMcanzyqfg5plRw-1","X-Mimecast-MFC-AGG-ID":"1CjLcFydMcanzyqfg5plRw_1747123811","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747123811; x=1747728611;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=YMD3G8gndxpfJ7B1YrzwHD5M00FDM9iabhOkEXB/U1c=;\n\tb=riKf2SCAZkCYnf2kCHlv65HKaddX1H/Y9VZdEWK5uRbHnrSeknfpwQ9szV7OaGg/I3\n\tKSUqcgWQREpmN8ZiDLzCysB3fx8m1uQBqAwzhhgO0QokdcS8Agj0/TgfbRQlYjVfI3YI\n\taS1BmV+djKQWgcctQqHRb2tg8FUul+0qbW1g5n4FLjbBrMt7Xw3G2KWDZqhUEJEzINkM\n\t6KOdOfLcPMFIaGl+c9qB4Wb8PB3Vqy6oUNemiAJg/X6dU9LG0esVawp9d1StSzyaX8/O\n\t2e9PLcvL4sWONIOH0CVweyq2WaL+5cip92rk2i9XlLfgrZb5DJVRQwM/FtVC6C4iBxCw\n\tomjQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUwc9GW9zMOQ0wUmhs9FyLvaeXa6+em7bjDNiqa0E9cMxz8mEfQc0Plx2aVuZ2a8lxaj3Ql9vHS2FHg5Gj5nbM=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yzch6cey+IwIXApeHZonTtdUw9l0OpPG/S9QSfAFDhRX0fmQJbj\n\tzWotbHCdRhQs0tRJXDSVdQbjhaQDnY4MZERl6RCnxgYhLnxxz42iSCviExPAJaXSOEL+1XojIOD\n\tASFjEimO5orUPV+0cXzIbYl5Cggp/qX8N9FsGwjVPic9utqAaYHKWLexeOS0Zisi340AdUceCE5\n\tzzIMwNFvdPQciwoyhTsBWrv2G+EdTmbmhtMLEQsVLQ43oONgZf8UX5BuI=","X-Gm-Gg":"ASbGnct/QuLEotjZ2BWHmmvXtvgK70aTT6pRHcgaqkFvuhmoZ/vVqYm3weWkiTasbEl\n\tTsoXRWqBl5oB5gvPqtJB+pUDbqNr3NmnzmZ7Q0xugru7eQDkTpGxmqFpCreZisG/jMbNej+uDEr\n\tA2e0i9+uMIWmAVPYw5yqIIt30s+qcQNsSW7/hLCw/ocznBULMQrowt79Q2yXVZxg3qsZRGtQ5xM\n\tvGd+KK4a7KMMjFDQqxhewddWDDepb4tNln3Uf6OXboVYeuVYBXNeEHJ0zy7bvbcHKrck6222naG\n\tEt+LdP2Jla2XtJrnO/gUKtYWvSNnElPsFtK/","X-Received":["by 2002:a05:600c:528e:b0:43d:77c5:9c1a with SMTP id\n\t5b1f17b1804b1-442d6d19253mr138280745e9.4.1747123810855; \n\tTue, 13 May 2025 01:10:10 -0700 (PDT)","by 2002:a05:600c:528e:b0:43d:77c5:9c1a with SMTP id\n\t5b1f17b1804b1-442d6d19253mr138279795e9.4.1747123809399; \n\tTue, 13 May 2025 01:10:09 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IE1rcE9DXc+v6NVljcKZFkGNAPb0PfMxi+LOd246koWO1Xzk1R576fTH6pG2ncths+W+kZ6+Q==","From":"Milan Zamazal ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart , Kieran Bingham\n\t, libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 03/11] libcamera: formats: Add a helper to check for\n\ta raw pixel format","In-Reply-To":" (\n\t=?utf-8?b?IkJhcm5hYsOhcyBQxZFjemUiJ3M=?= message of \"Fri,\n\t9 May 2025 12:59:56 +0200\")","References":"<20250407085639.16180-1-mzamazal@redhat.com>\n\t<20250407085639.16180-4-mzamazal@redhat.com>\n\t","Date":"Tue, 13 May 2025 10:10:08 +0200","Message-ID":"<85jz6lhqxr.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"Ni7OIiejCTjd2CeeGG2YsHqTI8iWRgT5gPHa7NR69Zg_1747123811","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34221,"web_url":"https://patchwork.libcamera.org/comment/34221/","msgid":"<174713446143.3660700.18375685910138080088@ping.linuxembedded.co.uk>","date":"2025-05-13T11:07:41","subject":"Re: [PATCH v4 03/11] libcamera: formats: Add a helper to check for a\n\traw pixel format","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2025-05-13 09:10:08)\n> Hi Barnabás,\n> \n> thank you for review.\n> \n> Barnabás Pőcze writes:\n> \n> > Hi\n> >\n> > 2025. 04. 07. 10:56 keltezéssel, Milan Zamazal írta:\n> >> There are several places with the same pattern to check whether a given\n> >> pixel format is a raw format:\n> >> return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n> >> libcamera::PixelFormatInfo::ColourEncodingRAW;\n> >> Let's move the corresponding isFormatRaw helper from mali-c55.cpp to\n> >> formats.cpp and use it where applicable. This also avoids a need to\n> >> introduce the same helper (or the same pattern) in the followup patches.\n> >\n> > As far as I can see, there are a lot of other places where this check is \"open coded\",\n> > I believe either all of them or none of them should be replaced. For example,\n> > `IPARkISP1::configure()`, `ISICameraConfiguration::validate()`,\n> > `IPU3CameraConfiguration::validate()`, etc.\n> >\n> > And there is also `PipelineHandlerBase::isRaw()` in the rpi pipeline.\n> \n> IIRC I tried to replace only the given pattern and not simpler\n> constructs like when the info is already retrieved, to not complicate\n> the patches more than necessary. But the rest can certainly be also\n> replaced, I can do it in v5 if there are no objections.\n> \n> > I am wondering if adding `PixelFormatInfo::isRaw()` could be useful as well.\n> \n> I think so, with the broader replacement.\n\nI'm sure I recall this used to be a global helper - and somehow/ for\nsome reason it got moved to being handled by pipeline handlers\nspecfically. I think the intention was that pipeline handlers would know\nwhich formats are raw or not ... but I can't find anything specific in\nthe history yet..\n\nEven if that was the case, maybe that needs to be revisited if in the\nend every pipeline handler ends up implementing it in exactly the same\nway...\n\n--\nKieran\n\n\n> \n> > Regards,\n> > Barnabás Pőcze\n> >\n> >\n> >> +/**\n> >> + * \\brief Return whether the given pixel format is a raw format\n> >> + * \\param[in] pixFmt The pixel format to examine\n> >> + * \\return True iff the given format is a raw format\n> >> + */\n> >> +bool isFormatRaw(const libcamera::PixelFormat &pixFmt)\n> >> +{\n> >> + return libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n> >> + libcamera::PixelFormatInfo::ColourEncodingRAW;\n> >> +}\n> >> +","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id ECD84C3200\n\tfor ;\n\tTue, 13 May 2025 11:07:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CF2668B40;\n\tTue, 13 May 2025 13:07:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 462DB6175C\n\tfor ;\n\tTue, 13 May 2025 13:07:44 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9475D4C9;\n\tTue, 13 May 2025 13:07:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Vs9TL8Ls\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747134448;\n\tbh=MWtrj3VBLBkE6Tp0DDqC3nn8blE2iiKlaKhiZV4RzxI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Vs9TL8LsXqTj08IYbeSZoCW0/eOEeLEOC/PfDx/XARQsexaVKTvSlTmzBzBuHz8iY\n\t2JhwpGVom5cQQzzhESi+DT9gy52AuTdq2dDTaMeCConoBXB4n38I08VDn9tq6uFYTw\n\tQGvICKwWHrckUGtQgKuYDo1dJKeZOVbVUw56qRPI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<85jz6lhqxr.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20250407085639.16180-1-mzamazal@redhat.com>\n\t<20250407085639.16180-4-mzamazal@redhat.com>\n\t\n\t<85jz6lhqxr.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Subject":"Re: [PATCH v4 03/11] libcamera: formats: Add a helper to check for a\n\traw pixel format","From":"Kieran Bingham ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tMilan Zamazal ","Date":"Tue, 13 May 2025 12:07:41 +0100","Message-ID":"<174713446143.3660700.18375685910138080088@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]