[{"id":14801,"web_url":"https://patchwork.libcamera.org/comment/14801/","msgid":"<YBCguBovvjlIES3J@pendragon.ideasonboard.com>","date":"2021-01-26T23:07:36","subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sebastian,\n\nThank you for the patch.\n\nOn Tue, Jan 26, 2021 at 07:48:53PM +0100, Sebastian Fricke wrote:\n> To transpose a BayerFormat means to flip it over its main-diagonal.\n\ns/main-diagonal/main diagonal/\n\n> \n> For example:\n> G B    G R\n>     ->\n> R G    B G\n> \n> The main-diagonal goes from the top left to the bottom right.\n\nSame here.\n\n> This means, that the only two orders affected by a transpose\n> are GBRG & GRBG. When a transpose is used in combination with horizontal\n> and/or vertical flips it is performed with the lowest priority.\n> Therefore add the functionality by switching GBRG (index 1) with GRBG\n> (index 2), after the flips have been applied.\n> \n> Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> ---\n>  src/libcamera/bayer_format.cpp | 9 +++++----\n>  1 file changed, 5 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> index d4e7f142..e06cd6e8 100644\n> --- a/src/libcamera/bayer_format.cpp\n> +++ b/src/libcamera/bayer_format.cpp\n> @@ -272,10 +272,6 @@ BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n>   * The transformed image would have a GRBG order. The bit depth and modifiers\n>   * are not affected.\n>   *\n> - * Note that transpositions are ignored as the order of a transpose with\n> - * respect to the flips would have to be defined, and sensors are not expected\n> - * to support transposition.\n> - *\n\nThis needs to now document the order in which the flips and transpose\nare applied. Maybe something along the lines of\n\n  * Horizontal and vertical flips are applied before transpose.\n\nWith this change,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nbut I'd like to get David's feedback on this.\n\n>   * \\return The transformed Bayer format\n>   */\n>  BayerFormat BayerFormat::transform(Transform t) const\n> @@ -292,6 +288,11 @@ BayerFormat BayerFormat::transform(Transform t) const\n>  \tif (!!(t & Transform::VFlip))\n>  \t\tresult.order = static_cast<Order>(result.order ^ 2);\n>  \n> +\tif (!!(t & Transform::Transpose) && result.order == 1)\n> +\t\tresult.order = static_cast<Order>(2);\n> +\telse if (!!(t & Transform::Transpose) && result.order == 2)\n> +\t\tresult.order = static_cast<Order>(1);\n> +\n>  \treturn result;\n>  }\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 DFF59BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Jan 2021 23:07:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 73A0468335;\n\tWed, 27 Jan 2021 00:07:57 +0100 (CET)","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 DA3F9682D2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Jan 2021 00:07:56 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 40E9C2C1;\n\tWed, 27 Jan 2021 00:07:56 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Vo6Fl3kb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611702476;\n\tbh=hGdcQGr9FNslbcMfzlAwWt9sSxZCGN7En0VhZZbFpi0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Vo6Fl3kbgP1gt/gPVP3hHlabeD3lBpvzozaFP4xH14tiqd2glkMBB4TQykg8pvbK5\n\tXwZy1rdEot/0UGgirsSQQwzK5+7h3wzp4O+moV8woGOr6lNT5gz2BWN0XJi8IpJpZk\n\taN0OCKglaXWjR0tY5HFrm54G+N0qQAERO2yn/gTQ=","Date":"Wed, 27 Jan 2021 01:07:36 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Sebastian Fricke <sebastian.fricke@posteo.net>","Message-ID":"<YBCguBovvjlIES3J@pendragon.ideasonboard.com>","References":"<20210126184854.46156-1-sebastian.fricke@posteo.net>\n\t<20210126184854.46156-5-sebastian.fricke@posteo.net>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210126184854.46156-5-sebastian.fricke@posteo.net>","Subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14975,"web_url":"https://patchwork.libcamera.org/comment/14975/","msgid":"<YBwmq+oONnE/L8Yp@pendragon.ideasonboard.com>","date":"2021-02-04T16:54:03","subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nWould you have a bit of time to review this ?\n\nOn Wed, Jan 27, 2021 at 01:07:38AM +0200, Laurent Pinchart wrote:\n> Hi Sebastian,\n> \n> Thank you for the patch.\n> \n> On Tue, Jan 26, 2021 at 07:48:53PM +0100, Sebastian Fricke wrote:\n> > To transpose a BayerFormat means to flip it over its main-diagonal.\n> \n> s/main-diagonal/main diagonal/\n> \n> > \n> > For example:\n> > G B    G R\n> >     ->\n> > R G    B G\n> > \n> > The main-diagonal goes from the top left to the bottom right.\n> \n> Same here.\n> \n> > This means, that the only two orders affected by a transpose\n> > are GBRG & GRBG. When a transpose is used in combination with horizontal\n> > and/or vertical flips it is performed with the lowest priority.\n> > Therefore add the functionality by switching GBRG (index 1) with GRBG\n> > (index 2), after the flips have been applied.\n> > \n> > Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> > ---\n> >  src/libcamera/bayer_format.cpp | 9 +++++----\n> >  1 file changed, 5 insertions(+), 4 deletions(-)\n> > \n> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > index d4e7f142..e06cd6e8 100644\n> > --- a/src/libcamera/bayer_format.cpp\n> > +++ b/src/libcamera/bayer_format.cpp\n> > @@ -272,10 +272,6 @@ BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> >   * The transformed image would have a GRBG order. The bit depth and modifiers\n> >   * are not affected.\n> >   *\n> > - * Note that transpositions are ignored as the order of a transpose with\n> > - * respect to the flips would have to be defined, and sensors are not expected\n> > - * to support transposition.\n> > - *\n> \n> This needs to now document the order in which the flips and transpose\n> are applied. Maybe something along the lines of\n> \n>   * Horizontal and vertical flips are applied before transpose.\n> \n> With this change,\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> but I'd like to get David's feedback on this.\n> \n> >   * \\return The transformed Bayer format\n> >   */\n> >  BayerFormat BayerFormat::transform(Transform t) const\n> > @@ -292,6 +288,11 @@ BayerFormat BayerFormat::transform(Transform t) const\n> >  \tif (!!(t & Transform::VFlip))\n> >  \t\tresult.order = static_cast<Order>(result.order ^ 2);\n> >  \n> > +\tif (!!(t & Transform::Transpose) && result.order == 1)\n> > +\t\tresult.order = static_cast<Order>(2);\n> > +\telse if (!!(t & Transform::Transpose) && result.order == 2)\n> > +\t\tresult.order = static_cast<Order>(1);\n> > +\n> >  \treturn result;\n> >  }\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 7240CBD162\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Feb 2021 16:54:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0610B6144D;\n\tThu,  4 Feb 2021 17:54:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 314C361430\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Feb 2021 17:54:26 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9275C510;\n\tThu,  4 Feb 2021 17:54:25 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cpW2Sn8n\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612457665;\n\tbh=2BEW6dgwLbArFIN6lkjiQDoJB3disg36K6hLnqStUQg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cpW2Sn8nATGiNrNpExjvvDwUfQiw8X2uDWWwYAJmewcbQNTbdgFeptbVL/9jkX475\n\t2BPIibEGgMe7HhTs9wjwjthKq4I5/xFzhuqVLZGv+UmV/knBmYtS4RTympHhOLtTRl\n\tMD1ghjiqDA7OhM1M1t8YA/YqZYITwhd5RxvnAnAE=","Date":"Thu, 4 Feb 2021 18:54:03 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"david.plowman@raspberrypi.com","Message-ID":"<YBwmq+oONnE/L8Yp@pendragon.ideasonboard.com>","References":"<20210126184854.46156-1-sebastian.fricke@posteo.net>\n\t<20210126184854.46156-5-sebastian.fricke@posteo.net>\n\t<YBCguBovvjlIES3J@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YBCguBovvjlIES3J@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14980,"web_url":"https://patchwork.libcamera.org/comment/14980/","msgid":"<CAHW6GYL+Xrhjy=0DQ=HB_uvuTtg-0Hhk=q6jzXnM+_FA6XHxiQ@mail.gmail.com>","date":"2021-02-04T18:15:28","subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Sebastian, Laurent,\n\nYes, when I first saw this patch - back when we didn't handle\ntranspose - I thought \"why don't we do transpose?\", rather forgetting\nthat it was me who chickened out of doing it in the first place!\n\nBut I think it's actually good to handle it in the same manner as we\ndo elsewhere, first the flips and then the transpose after. It would\nseem like an omission if we didn't, and you never know, someone might\nwant to apply an arbitrary transform to a Bayer pattern one day.\n\nOf course, users have to be aware of the order, but this is\nimplemented the same as everywhere else and so is consistent. I\nsuppose if you really wanted the other order, it's not difficult to\naccomplish.\n\nOn Tue, 26 Jan 2021 at 23:07, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Sebastian,\n>\n> Thank you for the patch.\n>\n> On Tue, Jan 26, 2021 at 07:48:53PM +0100, Sebastian Fricke wrote:\n> > To transpose a BayerFormat means to flip it over its main-diagonal.\n>\n> s/main-diagonal/main diagonal/\n>\n> >\n> > For example:\n> > G B    G R\n> >     ->\n> > R G    B G\n> >\n> > The main-diagonal goes from the top left to the bottom right.\n>\n> Same here.\n>\n> > This means, that the only two orders affected by a transpose\n> > are GBRG & GRBG. When a transpose is used in combination with horizontal\n> > and/or vertical flips it is performed with the lowest priority.\n\nJust a tiny nit-pick here. \"with the lowest priority\" sounds a bit\nvague to me, I think \"after the flips\" is clearer.\n\n> > Therefore add the functionality by switching GBRG (index 1) with GRBG\n> > (index 2), after the flips have been applied.\n> >\n> > Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> > ---\n> >  src/libcamera/bayer_format.cpp | 9 +++++----\n> >  1 file changed, 5 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > index d4e7f142..e06cd6e8 100644\n> > --- a/src/libcamera/bayer_format.cpp\n> > +++ b/src/libcamera/bayer_format.cpp\n> > @@ -272,10 +272,6 @@ BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> >   * The transformed image would have a GRBG order. The bit depth and modifiers\n> >   * are not affected.\n> >   *\n> > - * Note that transpositions are ignored as the order of a transpose with\n> > - * respect to the flips would have to be defined, and sensors are not expected\n> > - * to support transposition.\n> > - *\n>\n> This needs to now document the order in which the flips and transpose\n> are applied. Maybe something along the lines of\n>\n>   * Horizontal and vertical flips are applied before transpose.\n>\n> With this change,\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nYes, I agree. So with this change, also:\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks very much for all this work!\nDavid\n\n>\n> but I'd like to get David's feedback on this.\n>\n> >   * \\return The transformed Bayer format\n> >   */\n> >  BayerFormat BayerFormat::transform(Transform t) const\n> > @@ -292,6 +288,11 @@ BayerFormat BayerFormat::transform(Transform t) const\n> >       if (!!(t & Transform::VFlip))\n> >               result.order = static_cast<Order>(result.order ^ 2);\n> >\n> > +     if (!!(t & Transform::Transpose) && result.order == 1)\n> > +             result.order = static_cast<Order>(2);\n> > +     else if (!!(t & Transform::Transpose) && result.order == 2)\n> > +             result.order = static_cast<Order>(1);\n> > +\n> >       return result;\n> >  }\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 318A8BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Feb 2021 18:15:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B54D561455;\n\tThu,  4 Feb 2021 19:15:41 +0100 (CET)","from mail-ot1-x32b.google.com (mail-ot1-x32b.google.com\n\t[IPv6:2607:f8b0:4864:20::32b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E8A561430\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Feb 2021 19:15:40 +0100 (CET)","by mail-ot1-x32b.google.com with SMTP id f6so4312553ots.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 04 Feb 2021 10:15:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"SXQGmltn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=/IEk1yr8vUQyid24mTafqsdeYStJbtDK/X0BFjJNp4s=;\n\tb=SXQGmltnxUMU1TpeaBPZeQ4YKkKKXLI6OO4WsRW7I/qQFRV5NWZge7Y0rIptSP0cX0\n\tjEsQ1QRX4HQj7/Osf7B6v0EvrNogrMr3chq0OKStnLPHj+kNEb62dOJ68ejD/469WDQr\n\tMurEk8NnjOoq8bMh+vfnCe2rZJqr4VC53P+y59ttmh/JISvfuJANF4ttbM0ayWcDlvhI\n\t0+ysvaGrPeraejP+rqp1FA9D66weN7B2baySDND88teKvVOnqyQSLMmvx22Ia06l+VlI\n\tjy+7Aka4lKuhA7yPZRcMPGqFcy477nC5G/P2GlpFuAwJeVOgglXtubOSfBcxreGgAzZ0\n\tuq4w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=/IEk1yr8vUQyid24mTafqsdeYStJbtDK/X0BFjJNp4s=;\n\tb=GxoNF3DplUx2xvF0pWZtfQHbJqIcJjPRiRmpMllWZsLhq/1/3u0hs3kg2D0lI/e53U\n\t58vMgzHgaAwnW6mAyKUPjJhOYNN4baCmH1ZO0AG+Yp0H1ccinBq47qfpWoA3lJVUTW0C\n\tA+7HYjNrcPwUd0xmScKtPDtdcGozu44OZAcLp7gfGy10roXQaDHb7JknsvwnghNAwUkK\n\tQ5kP5VqiuWsbV5P4DdcuJ75lZIX85TJx6aP9UC40HCCrtPA2nOFOo8SqSC+C2P414D6+\n\tdtoymKOqJ2Ox4HmI8UtqN5wKEYeaF5pQ4MOkC/BTHa8y584aelu8Jtf0/OvMIbD09B6m\n\tIkxg==","X-Gm-Message-State":"AOAM531NmczcX78eA9fDl/+TbxCkuQrM3h1D/XCgRgQhYf6bM/6TbDDW\n\t5Bt25w9EUKUaeX5PkwsTZ+YKTZJu7IoSQvIhLt1b+lI/vQKSCQ==","X-Google-Smtp-Source":"ABdhPJzNuhjPW/leIyceU5uuQoiSxda9C9hsY6HP5cVHmQX+geej+fcV3HftaEDj2+D3E5oGwzABVJIQymcMCaLsNc8=","X-Received":"by 2002:a9d:3d87:: with SMTP id l7mr440363otc.317.1612462538860; \n\tThu, 04 Feb 2021 10:15:38 -0800 (PST)","MIME-Version":"1.0","References":"<20210126184854.46156-1-sebastian.fricke@posteo.net>\n\t<20210126184854.46156-5-sebastian.fricke@posteo.net>\n\t<YBCguBovvjlIES3J@pendragon.ideasonboard.com>","In-Reply-To":"<YBCguBovvjlIES3J@pendragon.ideasonboard.com>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Thu, 4 Feb 2021 18:15:28 +0000","Message-ID":"<CAHW6GYL+Xrhjy=0DQ=HB_uvuTtg-0Hhk=q6jzXnM+_FA6XHxiQ@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14983,"web_url":"https://patchwork.libcamera.org/comment/14983/","msgid":"<YBxOo/LTF4tnOFCX@pendragon.ideasonboard.com>","date":"2021-02-04T19:44:35","subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Thu, Feb 04, 2021 at 06:15:28PM +0000, David Plowman wrote:\n> Hi Sebastian, Laurent,\n> \n> Yes, when I first saw this patch - back when we didn't handle\n> transpose - I thought \"why don't we do transpose?\", rather forgetting\n> that it was me who chickened out of doing it in the first place!\n> \n> But I think it's actually good to handle it in the same manner as we\n> do elsewhere, first the flips and then the transpose after. It would\n> seem like an omission if we didn't, and you never know, someone might\n> want to apply an arbitrary transform to a Bayer pattern one day.\n> \n> Of course, users have to be aware of the order, but this is\n> implemented the same as everywhere else and so is consistent. I\n> suppose if you really wanted the other order, it's not difficult to\n> accomplish.\n\nThanks for the review David. I've pushed the series after applying the\nsmall fixes listed below.\n\n> On Tue, 26 Jan 2021 at 23:07, Laurent Pinchart wrote:\n> > On Tue, Jan 26, 2021 at 07:48:53PM +0100, Sebastian Fricke wrote:\n> > > To transpose a BayerFormat means to flip it over its main-diagonal.\n> >\n> > s/main-diagonal/main diagonal/\n> >\n> > > For example:\n> > > G B    G R\n> > >     ->\n> > > R G    B G\n> > >\n> > > The main-diagonal goes from the top left to the bottom right.\n> >\n> > Same here.\n> >\n> > > This means, that the only two orders affected by a transpose\n> > > are GBRG & GRBG. When a transpose is used in combination with horizontal\n> > > and/or vertical flips it is performed with the lowest priority.\n> \n> Just a tiny nit-pick here. \"with the lowest priority\" sounds a bit\n> vague to me, I think \"after the flips\" is clearer.\n> \n> > > Therefore add the functionality by switching GBRG (index 1) with GRBG\n> > > (index 2), after the flips have been applied.\n> > >\n> > > Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>\n> > > ---\n> > >  src/libcamera/bayer_format.cpp | 9 +++++----\n> > >  1 file changed, 5 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > > index d4e7f142..e06cd6e8 100644\n> > > --- a/src/libcamera/bayer_format.cpp\n> > > +++ b/src/libcamera/bayer_format.cpp\n> > > @@ -272,10 +272,6 @@ BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> > >   * The transformed image would have a GRBG order. The bit depth and modifiers\n> > >   * are not affected.\n> > >   *\n> > > - * Note that transpositions are ignored as the order of a transpose with\n> > > - * respect to the flips would have to be defined, and sensors are not expected\n> > > - * to support transposition.\n> > > - *\n> >\n> > This needs to now document the order in which the flips and transpose\n> > are applied. Maybe something along the lines of\n> >\n> >   * Horizontal and vertical flips are applied before transpose.\n> >\n> > With this change,\n> >\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> Yes, I agree. So with this change, also:\n> \n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> \n> Thanks very much for all this work!\n> David\n> \n> >\n> > but I'd like to get David's feedback on this.\n> >\n> > >   * \\return The transformed Bayer format\n> > >   */\n> > >  BayerFormat BayerFormat::transform(Transform t) const\n> > > @@ -292,6 +288,11 @@ BayerFormat BayerFormat::transform(Transform t) const\n> > >       if (!!(t & Transform::VFlip))\n> > >               result.order = static_cast<Order>(result.order ^ 2);\n> > >\n> > > +     if (!!(t & Transform::Transpose) && result.order == 1)\n> > > +             result.order = static_cast<Order>(2);\n> > > +     else if (!!(t & Transform::Transpose) && result.order == 2)\n> > > +             result.order = static_cast<Order>(1);\n> > > +\n> > >       return result;\n> > >  }\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 EB4D9BD162\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Feb 2021 19:45:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 86C146145B;\n\tThu,  4 Feb 2021 20:45:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1022461430\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Feb 2021 20:45:00 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7A68A45D;\n\tThu,  4 Feb 2021 20:44:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VVmZqU7s\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612467899;\n\tbh=abmA4jL5uQVEtnJyFCuAicINXiJ5qg9ubFrXQ6j2D/E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VVmZqU7sjb+xfvQ4cEsjeHu0NvjA+5vSZJStUXmVNH5CMWSSd5vkqrHaj6tItzczY\n\tjvxrth9vAJze9BFMg+ewvubaLFb66Xswibw4JSuHAfQwkyJaw1oKDWPgO2+t9aWJYL\n\tDWhujD026Iqkgin58qzFa9o5gr/v//aeITb8Zk3g=","Date":"Thu, 4 Feb 2021 21:44:35 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<YBxOo/LTF4tnOFCX@pendragon.ideasonboard.com>","References":"<20210126184854.46156-1-sebastian.fricke@posteo.net>\n\t<20210126184854.46156-5-sebastian.fricke@posteo.net>\n\t<YBCguBovvjlIES3J@pendragon.ideasonboard.com>\n\t<CAHW6GYL+Xrhjy=0DQ=HB_uvuTtg-0Hhk=q6jzXnM+_FA6XHxiQ@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAHW6GYL+Xrhjy=0DQ=HB_uvuTtg-0Hhk=q6jzXnM+_FA6XHxiQ@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/5] libcamera: Add the transpose\n\ttransformation","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]