[{"id":31485,"web_url":"https://patchwork.libcamera.org/comment/31485/","msgid":"<sdn4uqakjux52juctp3kkhbi6uvjgoori5xzpytytsp7rb5cnk@qy3hq6whtub2>","date":"2024-10-01T07:47:55","subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Umang\n\nOn Mon, Sep 30, 2024 at 08:50:38PM GMT, Umang Jain wrote:\n> Add a isRaw() helper to the PixelFormat class, to know whether the\n> pixel format has RAW encoding.\n>\n> This will used by validation and configuration code paths in pipeline\n> handlers, to know whether a pixel format is a raw format or not.\n>\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n\nDo you know why RPi does instead\n\nbool PipelineHandlerBase::isRaw(const PixelFormat &pixFmt)\n{\n\t/* This test works for both Bayer and raw mono formats. */\n\treturn BayerFormat::fromPixelFormat(pixFmt).isValid();\n}\n\ncc RPi which has upstreamed BayerFormat to know if there's a specfic reason\nwhy they do this instead of using PixelFormatInfo.\n\nSeems like we have two maps of raw formants, one part of the canonical\nPixelFormatInfo and one in BayerFormat. They store more or less the\nsame information, BayerFormat has a 'packing' field that\nPixelFormatInfo doesn't have, but I wonder why we have duplicated\nthis instead of properly extending PixelFormatInfo\n\nPixelFormatInfo:\n\n\t{ formats::RGGB_PISP_COMP1, {\n\t\t.name = \"RGGB_PISP_COMP1\",\n\t\t.format = formats::RGGB_PISP_COMP1,\n\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n\t\t.bitsPerPixel = 8,\n\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n\t\t.packed = true,\n\t\t.pixelsPerGroup = 2,\n\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n\t} },\n\nBayerFormat:\n\t{ { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n\t\t{ formats::RGGB_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n\nTrue, BayerFormat can be constructed from an mbusCode (something that\nhas proven to be problematic in the past, as multiple codes map to the\nsame BayerFormat), I wonder if that's the main reason.\n\n> ---\n>  include/libcamera/pixel_format.h |  1 +\n>  src/libcamera/pixel_format.cpp   | 11 +++++++++++\n>  2 files changed, 12 insertions(+)\n>\n> diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h\n> index 1b4d8c7c..aed53ea6 100644\n> --- a/include/libcamera/pixel_format.h\n> +++ b/include/libcamera/pixel_format.h\n> @@ -37,6 +37,7 @@ public:\n>  \tconstexpr uint64_t modifier() const { return modifier_; }\n>\n>  \tstd::string toString() const;\n> +\tbool isRaw() const;\n>\n>  \tstatic PixelFormat fromString(const std::string &name);\n>\n> diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp\n> index 314179a8..436ef5fb 100644\n> --- a/src/libcamera/pixel_format.cpp\n> +++ b/src/libcamera/pixel_format.cpp\n> @@ -100,6 +100,17 @@ bool PixelFormat::operator<(const PixelFormat &other) const\n>   * \\return DRM modifier\n>   */\n>\n> +/**\n> + * \\brief Checks if \\a this is a RAW pixel format\n> + * \\return True if \\a this is a RAW pixel format, false otherwise\n> + */\n> +bool PixelFormat::isRaw() const\n> +{\n> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(*this);\n> +\n> +\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> +}\n> +\n>  /**\n>   * \\brief Assemble and return a string describing the pixel format\n>   * \\return A string describing the pixel format\n> --\n> 2.45.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 BAB88BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Oct 2024 07:48:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A77B60553;\n\tTue,  1 Oct 2024 09:48:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8743360553\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Oct 2024 09:48:00 +0200 (CEST)","from ideasonboard.com (unknown [5.179.150.95])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 73A22669;\n\tTue,  1 Oct 2024 09:46: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=\"gLnfx8cM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727768788;\n\tbh=Fu6v+QjKl/+Dvlk0WgEPLUBw3C0twFD55dNaF7YBVyE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gLnfx8cMky9/t+fI24PTE8hMoiAJjMdrznOJ4I0so0X/NnvdzN1YHF4yYPi7RsUuv\n\tzOScz1C50Y8i8bycJjJNLgNB153aX/K4q2EW5IkCsz+c9Z9OAgcwfjsZ6qXhnFSCme\n\treV4Jh2pvgEczuFzTpQk+pCbsotL3n9z3rIDdnko=","Date":"Tue, 1 Oct 2024 09:47:55 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>, \n\tNaushir Patuck <naush@raspberrypi.com>,\n\t\"david.plowman@raspberrypi.com\" <david.plowman@raspberrypi.com>","Subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","Message-ID":"<sdn4uqakjux52juctp3kkhbi6uvjgoori5xzpytytsp7rb5cnk@qy3hq6whtub2>","References":"<20240930152039.72459-1-umang.jain@ideasonboard.com>\n\t<20240930152039.72459-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240930152039.72459-2-umang.jain@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31487,"web_url":"https://patchwork.libcamera.org/comment/31487/","msgid":"<CAEmqJPozXiCki-22nZ-1Tw=Kmyh7m8=U=2TKZH3Qn640US5eQw@mail.gmail.com>","date":"2024-10-01T07:53:09","subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nYou read my mind - I was going to respond to this next.\n\nOn Tue, 1 Oct 2024 at 08:48, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:\n>\n> Hi Umang\n>\n> On Mon, Sep 30, 2024 at 08:50:38PM GMT, Umang Jain wrote:\n> > Add a isRaw() helper to the PixelFormat class, to know whether the\n> > pixel format has RAW encoding.\n> >\n> > This will used by validation and configuration code paths in pipeline\n> > handlers, to know whether a pixel format is a raw format or not.\n> >\n> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n>\n> Do you know why RPi does instead\n>\n> bool PipelineHandlerBase::isRaw(const PixelFormat &pixFmt)\n> {\n>         /* This test works for both Bayer and raw mono formats. */\n>         return BayerFormat::fromPixelFormat(pixFmt).isValid();\n> }\n>\n> cc RPi which has upstreamed BayerFormat to know if there's a specfic reason\n> why they do this instead of using PixelFormatInfo.\n\nI assume RAW in this case means Bayer right?\n\nI think looking at colourEncoding == ColourEncodingRAW might be incomplete as\nmono formats (correctly) set colourEncoding to ColourEncodingYUV.\n\nRegards,\nNaush\n\n>\n> Seems like we have two maps of raw formants, one part of the canonical\n> PixelFormatInfo and one in BayerFormat. They store more or less the\n> same information, BayerFormat has a 'packing' field that\n> PixelFormatInfo doesn't have, but I wonder why we have duplicated\n> this instead of properly extending PixelFormatInfo\n>\n> PixelFormatInfo:\n>\n>         { formats::RGGB_PISP_COMP1, {\n>                 .name = \"RGGB_PISP_COMP1\",\n>                 .format = formats::RGGB_PISP_COMP1,\n>                 .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n>                 .bitsPerPixel = 8,\n>                 .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n>                 .packed = true,\n>                 .pixelsPerGroup = 2,\n>                 .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n>         } },\n>\n> BayerFormat:\n>         { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n>                 { formats::RGGB_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n>\n> True, BayerFormat can be constructed from an mbusCode (something that\n> has proven to be problematic in the past, as multiple codes map to the\n> same BayerFormat), I wonder if that's the main reason.\n>\n> > ---\n> >  include/libcamera/pixel_format.h |  1 +\n> >  src/libcamera/pixel_format.cpp   | 11 +++++++++++\n> >  2 files changed, 12 insertions(+)\n> >\n> > diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h\n> > index 1b4d8c7c..aed53ea6 100644\n> > --- a/include/libcamera/pixel_format.h\n> > +++ b/include/libcamera/pixel_format.h\n> > @@ -37,6 +37,7 @@ public:\n> >       constexpr uint64_t modifier() const { return modifier_; }\n> >\n> >       std::string toString() const;\n> > +     bool isRaw() const;\n> >\n> >       static PixelFormat fromString(const std::string &name);\n> >\n> > diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp\n> > index 314179a8..436ef5fb 100644\n> > --- a/src/libcamera/pixel_format.cpp\n> > +++ b/src/libcamera/pixel_format.cpp\n> > @@ -100,6 +100,17 @@ bool PixelFormat::operator<(const PixelFormat &other) const\n> >   * \\return DRM modifier\n> >   */\n> >\n> > +/**\n> > + * \\brief Checks if \\a this is a RAW pixel format\n> > + * \\return True if \\a this is a RAW pixel format, false otherwise\n> > + */\n> > +bool PixelFormat::isRaw() const\n> > +{\n> > +     const PixelFormatInfo &info = PixelFormatInfo::info(*this);\n> > +\n> > +     return info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> > +}\n> > +\n> >  /**\n> >   * \\brief Assemble and return a string describing the pixel format\n> >   * \\return A string describing the pixel format\n> > --\n> > 2.45.2\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C8181C3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Oct 2024 07:53:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C8A6263515;\n\tTue,  1 Oct 2024 09:53:41 +0200 (CEST)","from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com\n\t[IPv6:2607:f8b0:4864:20::b32])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 48A4E6350E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Oct 2024 09:53:39 +0200 (CEST)","by mail-yb1-xb32.google.com with SMTP id\n\t3f1490d57ef6-e25e32e8baaso584054276.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 01 Oct 2024 00:53:39 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"gAHgDnEe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1727769218; x=1728374018;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=tjcyBZmysCV+AeZZTKmuxLMvXsqnVeF4sFew0Qf1gXU=;\n\tb=gAHgDnEeSXJ9CK1KYDVMbhZfFwGrAKMOMEF8By9i1MLKxw06QBlMke0Biv4Hd43EOF\n\tJhDAKQxK0+OO9AryVX8sX8feX/5bOJgP6sinB1XKIZO5YLXKSkuWMOQtLqlO1aDthIYH\n\t4Ps2fMdRJ891l52pUKXsalnldhYMgfS5EwAgA4zAluf2Faj4yvzl0XhXpIG/+XtdU9OK\n\tJkTxaoKyrfVLo1CmVFciLd/L2JCTlsWUN4YNfOMNqOsJnZ7M0zI2zuFkPkxo2TpF4SVk\n\tkj6ZJKotXtEfzhhuCzJ2fnhgYvT4Fr8bi8XD3g4M6SERiQSitoC7BUYBUxDT57P9IDoY\n\tiieA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1727769218; x=1728374018;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=tjcyBZmysCV+AeZZTKmuxLMvXsqnVeF4sFew0Qf1gXU=;\n\tb=fxtv7jGpMDGoRyqxPhvnbW1d8ru+TxhM7P4Hc5C7yRpPdOs19+HwOl1QQTTqDOzyq8\n\tHe+whdT79nYzIP8/5ZD95w2fLHg1rJhQoNvTQeWrgHH3XAt5n+ey8xlgbI5MLbIb601e\n\tgDiJNctuufIY0nve5B7/5Oqvdmt7G/LYumsIo8zprhC5kG10xyDh7ie7KNnMSDt1n+vQ\n\t2aYaY8aIQ+w3Q5RWPSY2ZC/zfPettrHSDm/YlW+rKaFbHwEK5ICDSJnhhpDKIOD/7YN7\n\tNPVnCwno0bsu1axA/Sqe3TL9i3hS9V6nbeKT9oFAqGcGbFmddpzhTHvn7Dr3ovCq198o\n\tWj8Q==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWJQlJUzhPsjlX5CXzZc3w8kUBAgV3DVai2nudAMuNiFYUtYaFSLmrQDnPqNCM2iBf2867jE6b1PfhKERai+jQ=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxKGxx8r+f34eRb78T0pm/+aXZepIaiT/szQvbgbieVVZOg/kpx\n\tSocoeUkeFK5DMYoYfhRA04ftddR17STytzEkXwDmvswls0QImWFZ7HNA4jyF2rmfPF7Eb6pyIoZ\n\t4/Q1hsGHqJss4/jt6zw0BVrrGg6C2AiQWXziOFpoyXtp+VJPltd0=","X-Google-Smtp-Source":"AGHT+IEHQIPAqXv2VKNRhFt0T4RSnfwMkzuMXR+4shDLuF1j0j49Fcb43tE+RM1G6AaP25AlyEYkN9txCGqZLeLyQMA=","X-Received":"by 2002:a05:690c:b:b0:6e2:15aa:f62c with SMTP id\n\t00721157ae682-6e258e16f9dmr35903617b3.5.1727769217939;\n\tTue, 01 Oct 2024 00:53:37 -0700 (PDT)","MIME-Version":"1.0","References":"<20240930152039.72459-1-umang.jain@ideasonboard.com>\n\t<20240930152039.72459-2-umang.jain@ideasonboard.com>\n\t<sdn4uqakjux52juctp3kkhbi6uvjgoori5xzpytytsp7rb5cnk@qy3hq6whtub2>","In-Reply-To":"<sdn4uqakjux52juctp3kkhbi6uvjgoori5xzpytytsp7rb5cnk@qy3hq6whtub2>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Tue, 1 Oct 2024 08:53:09 +0100","Message-ID":"<CAEmqJPozXiCki-22nZ-1Tw=Kmyh7m8=U=2TKZH3Qn640US5eQw@mail.gmail.com>","Subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Umang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>, \n\tKieran Bingham <kieran.bingham@ideasonboard.com>, \n\tDaniel Scally <dan.scally@ideasonboard.com>, \n\t\"david.plowman@raspberrypi.com\" <david.plowman@raspberrypi.com>","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31492,"web_url":"https://patchwork.libcamera.org/comment/31492/","msgid":"<6ex2lqt7jg4oa55g6pydp5mu54l7zbxwlaaq7txdyxgckyx7j5@4wxhxeo2aqjs>","date":"2024-10-01T09:06:01","subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, Oct 01, 2024 at 08:53:09AM GMT, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> You read my mind - I was going to respond to this next.\n>\n> On Tue, 1 Oct 2024 at 08:48, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:\n> >\n> > Hi Umang\n> >\n> > On Mon, Sep 30, 2024 at 08:50:38PM GMT, Umang Jain wrote:\n> > > Add a isRaw() helper to the PixelFormat class, to know whether the\n> > > pixel format has RAW encoding.\n> > >\n> > > This will used by validation and configuration code paths in pipeline\n> > > handlers, to know whether a pixel format is a raw format or not.\n> > >\n> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> >\n> > Do you know why RPi does instead\n> >\n> > bool PipelineHandlerBase::isRaw(const PixelFormat &pixFmt)\n> > {\n> >         /* This test works for both Bayer and raw mono formats. */\n> >         return BayerFormat::fromPixelFormat(pixFmt).isValid();\n> > }\n> >\n> > cc RPi which has upstreamed BayerFormat to know if there's a specfic reason\n> > why they do this instead of using PixelFormatInfo.\n>\n> I assume RAW in this case means Bayer right?\n>\n> I think looking at colourEncoding == ColourEncodingRAW might be incomplete as\n> mono formats (correctly) set colourEncoding to ColourEncodingYUV.\n\nOk, thanks for clarifying it.\n\nWe should then replace all current usages of  colourEncoding ==\nColourEncodingRAW and leave BayerFormat be. That's what Umang did\nalready and there won't be regressions introduced this way.\n>\n> Regards,\n> Naush\n>\n> >\n> > Seems like we have two maps of raw formants, one part of the canonical\n> > PixelFormatInfo and one in BayerFormat. They store more or less the\n> > same information, BayerFormat has a 'packing' field that\n> > PixelFormatInfo doesn't have, but I wonder why we have duplicated\n> > this instead of properly extending PixelFormatInfo\n> >\n> > PixelFormatInfo:\n> >\n> >         { formats::RGGB_PISP_COMP1, {\n> >                 .name = \"RGGB_PISP_COMP1\",\n> >                 .format = formats::RGGB_PISP_COMP1,\n> >                 .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> >                 .bitsPerPixel = 8,\n> >                 .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> >                 .packed = true,\n> >                 .pixelsPerGroup = 2,\n> >                 .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> >         } },\n> >\n> > BayerFormat:\n> >         { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n> >                 { formats::RGGB_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n> >\n> > True, BayerFormat can be constructed from an mbusCode (something that\n> > has proven to be problematic in the past, as multiple codes map to the\n> > same BayerFormat), I wonder if that's the main reason.\n> >\n> > > ---\n> > >  include/libcamera/pixel_format.h |  1 +\n> > >  src/libcamera/pixel_format.cpp   | 11 +++++++++++\n> > >  2 files changed, 12 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h\n> > > index 1b4d8c7c..aed53ea6 100644\n> > > --- a/include/libcamera/pixel_format.h\n> > > +++ b/include/libcamera/pixel_format.h\n> > > @@ -37,6 +37,7 @@ public:\n> > >       constexpr uint64_t modifier() const { return modifier_; }\n> > >\n> > >       std::string toString() const;\n> > > +     bool isRaw() const;\n> > >\n> > >       static PixelFormat fromString(const std::string &name);\n> > >\n> > > diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp\n> > > index 314179a8..436ef5fb 100644\n> > > --- a/src/libcamera/pixel_format.cpp\n> > > +++ b/src/libcamera/pixel_format.cpp\n> > > @@ -100,6 +100,17 @@ bool PixelFormat::operator<(const PixelFormat &other) const\n> > >   * \\return DRM modifier\n> > >   */\n> > >\n> > > +/**\n> > > + * \\brief Checks if \\a this is a RAW pixel format\n> > > + * \\return True if \\a this is a RAW pixel format, false otherwise\n> > > + */\n> > > +bool PixelFormat::isRaw() const\n> > > +{\n> > > +     const PixelFormatInfo &info = PixelFormatInfo::info(*this);\n> > > +\n> > > +     return info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> > > +}\n> > > +\n> > >  /**\n> > >   * \\brief Assemble and return a string describing the pixel format\n> > >   * \\return A string describing the pixel format\n> > > --\n> > > 2.45.2\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 907FDC3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Oct 2024 09:06:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71F2B6350F;\n\tTue,  1 Oct 2024 11:06:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 588F762C92\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Oct 2024 11:06:05 +0200 (CEST)","from ideasonboard.com (unknown [5.179.150.95])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4C14C669;\n\tTue,  1 Oct 2024 11:04:32 +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=\"MOV49h2r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727773473;\n\tbh=utd1vArRZm4aZrFIt1yPEUJ8c+t9AAL9mCJCbapOC5o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MOV49h2rhlrpl2e3rs8xWJ1Nvv0q68Nl4RJl6uc5hd81By0A62nuQTtTHJYlyVFlO\n\tidz2V1WIU426guOEv6PeRNkNt0AmEZOHyB3spWSJiwmHF6aRLcySUmfQrTwqRzk6u7\n\twqpTLNAIKLt8O4UkAX/+YEjcpzjDXV2Rj0Aew1oo=","Date":"Tue, 1 Oct 2024 11:06:01 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tUmang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, \n\tDaniel Scally <dan.scally@ideasonboard.com>,\n\t\"david.plowman@raspberrypi.com\" <david.plowman@raspberrypi.com>","Subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","Message-ID":"<6ex2lqt7jg4oa55g6pydp5mu54l7zbxwlaaq7txdyxgckyx7j5@4wxhxeo2aqjs>","References":"<20240930152039.72459-1-umang.jain@ideasonboard.com>\n\t<20240930152039.72459-2-umang.jain@ideasonboard.com>\n\t<sdn4uqakjux52juctp3kkhbi6uvjgoori5xzpytytsp7rb5cnk@qy3hq6whtub2>\n\t<CAEmqJPozXiCki-22nZ-1Tw=Kmyh7m8=U=2TKZH3Qn640US5eQw@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPozXiCki-22nZ-1Tw=Kmyh7m8=U=2TKZH3Qn640US5eQw@mail.gmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31514,"web_url":"https://patchwork.libcamera.org/comment/31514/","msgid":"<36v2zdw4iqqzbrajsvgsaca74zldzmz3arg3a6gjy7lmamnkce@k5z4x4vdmcna>","date":"2024-10-01T18:41:51","subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Umang\n\nOn Mon, Sep 30, 2024 at 08:50:38PM GMT, Umang Jain wrote:\n> Add a isRaw() helper to the PixelFormat class, to know whether the\n> pixel format has RAW encoding.\n>\n> This will used by validation and configuration code paths in pipeline\n> handlers, to know whether a pixel format is a raw format or not.\n>\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>  include/libcamera/pixel_format.h |  1 +\n>  src/libcamera/pixel_format.cpp   | 11 +++++++++++\n>  2 files changed, 12 insertions(+)\n>\n> diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h\n> index 1b4d8c7c..aed53ea6 100644\n> --- a/include/libcamera/pixel_format.h\n> +++ b/include/libcamera/pixel_format.h\n> @@ -37,6 +37,7 @@ public:\n>  \tconstexpr uint64_t modifier() const { return modifier_; }\n>\n>  \tstd::string toString() const;\n> +\tbool isRaw() const;\n>\n>  \tstatic PixelFormat fromString(const std::string &name);\n>\n> diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp\n> index 314179a8..436ef5fb 100644\n> --- a/src/libcamera/pixel_format.cpp\n> +++ b/src/libcamera/pixel_format.cpp\n> @@ -100,6 +100,17 @@ bool PixelFormat::operator<(const PixelFormat &other) const\n>   * \\return DRM modifier\n>   */\n>\n> +/**\n> + * \\brief Checks if \\a this is a RAW pixel format\n\nWell, I'm not sure I want to go there, as talking about colour spaces\nand colour encoding for RAW formats is a bit of a minefield. Indeed we\nuse the colour encoding of a format to identify it as RAW, so the\n\\brief here is somewhat correct.\n\nHowever I would mention briefly that we check if a pixel format is RAW\nby checking its color encoding.\n\n\n/**\n * \\brief Check if \\this is RAW pixel format\n *\n * Check if a pixel format is RAW by validating that its colour\n * encoding is ColourEncodingRAW.\n *\n * \\return True if \\a this has RAW colour encoding, false otherwise\n */\n\nWith this, if you think it's appropriate:\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n> + * \\return True if \\a this is a RAW pixel format, false otherwise\n> + */\n> +bool PixelFormat::isRaw() const\n> +{\n> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(*this);\n> +\n> +\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> +}\n> +\n>  /**\n>   * \\brief Assemble and return a string describing the pixel format\n>   * \\return A string describing the pixel format\n> --\n> 2.45.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 748A3BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Oct 2024 18:41:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5111163524;\n\tTue,  1 Oct 2024 20:41:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DC915618D7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Oct 2024 20:41:56 +0200 (CEST)","from ideasonboard.com (unknown [5.77.89.72])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 68230A1A;\n\tTue,  1 Oct 2024 20:40:24 +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=\"FmyBereG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727808024;\n\tbh=swajcZ81OsfPRxX8GV6GOoTt885o4FW8ZjjvMzpUg6s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FmyBereGapjI3RO/UZOXYYcpkRwT0JSnkL5zN4UV+abaIms5pw6oew4HnIYAO9D2x\n\tb6UtvGHNppDe1vIrmvdZYL1iaiqW4iNA53n/KdopF7xpcvWLxD4stat90mm3EpSgTZ\n\t9GaIvJX+TVhbkn/Lkb3FkitPAdYwQ5qOEektN7q8=","Date":"Tue, 1 Oct 2024 20:41:51 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>","Subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","Message-ID":"<36v2zdw4iqqzbrajsvgsaca74zldzmz3arg3a6gjy7lmamnkce@k5z4x4vdmcna>","References":"<20240930152039.72459-1-umang.jain@ideasonboard.com>\n\t<20240930152039.72459-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240930152039.72459-2-umang.jain@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31569,"web_url":"https://patchwork.libcamera.org/comment/31569/","msgid":"<20241003152759.GA32537@pendragon.ideasonboard.com>","date":"2024-10-03T15:27:59","subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Oct 01, 2024 at 08:41:51PM +0200, Jacopo Mondi wrote:\n> Hi Umang\n> \n> On Mon, Sep 30, 2024 at 08:50:38PM GMT, Umang Jain wrote:\n> > Add a isRaw() helper to the PixelFormat class, to know whether the\n> > pixel format has RAW encoding.\n> >\n> > This will used by validation and configuration code paths in pipeline\n> > handlers, to know whether a pixel format is a raw format or not.\n> >\n> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > ---\n> >  include/libcamera/pixel_format.h |  1 +\n> >  src/libcamera/pixel_format.cpp   | 11 +++++++++++\n> >  2 files changed, 12 insertions(+)\n> >\n> > diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h\n> > index 1b4d8c7c..aed53ea6 100644\n> > --- a/include/libcamera/pixel_format.h\n> > +++ b/include/libcamera/pixel_format.h\n> > @@ -37,6 +37,7 @@ public:\n> >  \tconstexpr uint64_t modifier() const { return modifier_; }\n> >\n> >  \tstd::string toString() const;\n> > +\tbool isRaw() const;\n> >\n> >  \tstatic PixelFormat fromString(const std::string &name);\n> >\n> > diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp\n> > index 314179a8..436ef5fb 100644\n> > --- a/src/libcamera/pixel_format.cpp\n> > +++ b/src/libcamera/pixel_format.cpp\n> > @@ -100,6 +100,17 @@ bool PixelFormat::operator<(const PixelFormat &other) const\n> >   * \\return DRM modifier\n> >   */\n> >\n> > +/**\n> > + * \\brief Checks if \\a this is a RAW pixel format\n> \n> Well, I'm not sure I want to go there, as talking about colour spaces\n> and colour encoding for RAW formats is a bit of a minefield. Indeed we\n> use the colour encoding of a format to identify it as RAW, so the\n> \\brief here is somewhat correct.\n\nI'm also concerned, sorry :-(\n\n\"RAW\" is ill-defined here. It's actually not defined at all in this\npatch :-) The PixelFormat is a public class, so its API needs to be\nclearly defined, we can't just implement PixelFormat::isRaw() as \"what\nlibcamera does now internally in most place\". And once you try to define\nthe concept, you'll likely realize that it's not an easy exercize. A\ngood example is the R8 format, which is used for 8-bit greyscale images.\nAn image in that format can be a raw frame captured directly from the\nsnesor, or a processed frame at the output of the ISP.\n\n> However I would mention briefly that we check if a pixel format is RAW\n> by checking its color encoding.\n> \n> \n> /**\n>  * \\brief Check if \\this is RAW pixel format\n>  *\n>  * Check if a pixel format is RAW by validating that its colour\n>  * encoding is ColourEncodingRAW.\n>  *\n>  * \\return True if \\a this has RAW colour encoding, false otherwise\n>  */\n> \n> With this, if you think it's appropriate:\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> \n> > + * \\return True if \\a this is a RAW pixel format, false otherwise\n> > + */\n> > +bool PixelFormat::isRaw() const\n> > +{\n> > +\tconst PixelFormatInfo &info = PixelFormatInfo::info(*this);\n> > +\n> > +\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> > +}\n> > +\n> >  /**\n> >   * \\brief Assemble and return a string describing the pixel format\n> >   * \\return A string describing the pixel format","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 8A247BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  3 Oct 2024 15:28:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 433B663526;\n\tThu,  3 Oct 2024 17:28:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1AD1762C92\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  3 Oct 2024 17:28:03 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AF6F6593;\n\tThu,  3 Oct 2024 17:26:29 +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=\"rXEwcuqj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727969189;\n\tbh=tfxALYxw9+zJ110nvgJ825mMo3hylPpFfyknObC5NG0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rXEwcuqjZxhFcDGEjHbm3xXkaOrQQTDYqtK1QNt5WwBtirrorLtPIIHT/lZybJyoy\n\tVf6m09cR09sKPCj1OMgagqz8QfimIu7EvvTHhFfyjbJm3Zc6uzF1EhNmBow5pCSl1n\n\tDc9XZSz3VYnQZLP3jKd/C0oeZ7wWpGvD4vfZ+i08=","Date":"Thu, 3 Oct 2024 18:27:59 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Umang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>","Subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","Message-ID":"<20241003152759.GA32537@pendragon.ideasonboard.com>","References":"<20240930152039.72459-1-umang.jain@ideasonboard.com>\n\t<20240930152039.72459-2-umang.jain@ideasonboard.com>\n\t<36v2zdw4iqqzbrajsvgsaca74zldzmz3arg3a6gjy7lmamnkce@k5z4x4vdmcna>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<36v2zdw4iqqzbrajsvgsaca74zldzmz3arg3a6gjy7lmamnkce@k5z4x4vdmcna>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31582,"web_url":"https://patchwork.libcamera.org/comment/31582/","msgid":"<78c7ee0a-5dd4-4326-aa24-641718aad30b@ideasonboard.com>","date":"2024-10-04T04:58:54","subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 03/10/24 8:57 pm, Laurent Pinchart wrote:\n> On Tue, Oct 01, 2024 at 08:41:51PM +0200, Jacopo Mondi wrote:\n>> Hi Umang\n>>\n>> On Mon, Sep 30, 2024 at 08:50:38PM GMT, Umang Jain wrote:\n>>> Add a isRaw() helper to the PixelFormat class, to know whether the\n>>> pixel format has RAW encoding.\n>>>\n>>> This will used by validation and configuration code paths in pipeline\n>>> handlers, to know whether a pixel format is a raw format or not.\n>>>\n>>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n>>> ---\n>>>   include/libcamera/pixel_format.h |  1 +\n>>>   src/libcamera/pixel_format.cpp   | 11 +++++++++++\n>>>   2 files changed, 12 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/pixel_format.h b/include/libcamera/pixel_format.h\n>>> index 1b4d8c7c..aed53ea6 100644\n>>> --- a/include/libcamera/pixel_format.h\n>>> +++ b/include/libcamera/pixel_format.h\n>>> @@ -37,6 +37,7 @@ public:\n>>>   \tconstexpr uint64_t modifier() const { return modifier_; }\n>>>\n>>>   \tstd::string toString() const;\n>>> +\tbool isRaw() const;\n>>>\n>>>   \tstatic PixelFormat fromString(const std::string &name);\n>>>\n>>> diff --git a/src/libcamera/pixel_format.cpp b/src/libcamera/pixel_format.cpp\n>>> index 314179a8..436ef5fb 100644\n>>> --- a/src/libcamera/pixel_format.cpp\n>>> +++ b/src/libcamera/pixel_format.cpp\n>>> @@ -100,6 +100,17 @@ bool PixelFormat::operator<(const PixelFormat &other) const\n>>>    * \\return DRM modifier\n>>>    */\n>>>\n>>> +/**\n>>> + * \\brief Checks if \\a this is a RAW pixel format\n>> Well, I'm not sure I want to go there, as talking about colour spaces\n>> and colour encoding for RAW formats is a bit of a minefield. Indeed we\n>> use the colour encoding of a format to identify it as RAW, so the\n>> \\brief here is somewhat correct.\n> I'm also concerned, sorry :-(\n>\n> \"RAW\" is ill-defined here. It's actually not defined at all in this\n> patch :-) The PixelFormat is a public class, so its API needs to be\n> clearly defined, we can't just implement PixelFormat::isRaw() as \"what\n> libcamera does now internally in most place\". And once you try to define\n> the concept, you'll likely realize that it's not an easy exercize. A\n> good example is the R8 format, which is used for 8-bit greyscale images.\n> An image in that format can be a raw frame captured directly from the\n> snesor, or a processed frame at the output of the ISP.\n\nThanks for the comment, it indeed is a difficult exercise to define 'raw'.\n\nI will drop the series.\n>> However I would mention briefly that we check if a pixel format is RAW\n>> by checking its color encoding.\n>>\n>>\n>> /**\n>>   * \\brief Check if \\this is RAW pixel format\n>>   *\n>>   * Check if a pixel format is RAW by validating that its colour\n>>   * encoding is ColourEncodingRAW.\n>>   *\n>>   * \\return True if \\a this has RAW colour encoding, false otherwise\n>>   */\n>>\n>> With this, if you think it's appropriate:\n>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>>\n>>> + * \\return True if \\a this is a RAW pixel format, false otherwise\n>>> + */\n>>> +bool PixelFormat::isRaw() const\n>>> +{\n>>> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(*this);\n>>> +\n>>> +\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n>>> +}\n>>> +\n>>>   /**\n>>>    * \\brief Assemble and return a string describing the pixel format\n>>>    * \\return A string describing the pixel format","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A052BBD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  4 Oct 2024 04:59:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 485D16352C;\n\tFri,  4 Oct 2024 06:59:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AE5762C8E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  4 Oct 2024 06:58:59 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0329618D;\n\tFri,  4 Oct 2024 06:57:24 +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=\"VpSHF0E8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728017845;\n\tbh=bJX51BJ6pVY1lXFnrEPo9QI/gq1SO6FmYaApfpx87aM=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=VpSHF0E8u2ISzt2Qw5ghhFd82CSpClBz2H25hQvPtPxLcbLoRpjGmOpEvlxKTE24k\n\tFKKguMYT0uoXztX0SvRZWxboH7Dm1EjqqMA1+S6Pd/H3PZYXm9FiXQ3KqDq+x4GR5+\n\tve0oq26zYj41kND6gMspBbdmI+9kEVBOU8Z6ARUA=","Message-ID":"<78c7ee0a-5dd4-4326-aa24-641718aad30b@ideasonboard.com>","Date":"Fri, 4 Oct 2024 10:28:54 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] libcamera: pixel_format: Add isRaw() helper","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>","References":"<20240930152039.72459-1-umang.jain@ideasonboard.com>\n\t<20240930152039.72459-2-umang.jain@ideasonboard.com>\n\t<36v2zdw4iqqzbrajsvgsaca74zldzmz3arg3a6gjy7lmamnkce@k5z4x4vdmcna>\n\t<20241003152759.GA32537@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Umang Jain <umang.jain@ideasonboard.com>","In-Reply-To":"<20241003152759.GA32537@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]