[{"id":27984,"web_url":"https://patchwork.libcamera.org/comment/27984/","msgid":"<20231018205416.GJ1512@pendragon.ideasonboard.com>","date":"2023-10-18T20:54:16","subject":"Re: [libcamera-devel] [PATCH v5 08/12] test: Add unit test for\n\tTransform and Orientation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, Sep 01, 2023 at 05:02:11PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> Add a unit test for Transform and Orientation to validate the\n> implementation of the operations between the two types.\n> \n> In particular, test that:\n> \n> \to1 / o2 = t\n> \to2 * t = o1\n> \n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  test/meson.build   |   1 +\n>  test/transform.cpp | 331 +++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 332 insertions(+)\n>  create mode 100644 test/transform.cpp\n> \n> diff --git a/test/meson.build b/test/meson.build\n> index b227be818419..189e1428485a 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -46,6 +46,7 @@ public_tests = [\n>      {'name': 'public-api', 'sources': ['public-api.cpp']},\n>      {'name': 'signal', 'sources': ['signal.cpp']},\n>      {'name': 'span', 'sources': ['span.cpp']},\n> +    {'name': 'transform', 'sources': ['transform.cpp']},\n>  ]\n>  \n>  internal_tests = [\n> diff --git a/test/transform.cpp b/test/transform.cpp\n> new file mode 100644\n> index 000000000000..4a6cf7387f02\n> --- /dev/null\n> +++ b/test/transform.cpp\n> @@ -0,0 +1,331 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas On Board Oy\n> + *\n> + * transform.cpp - Transform and Orientation tests\n> + */\n> +\n> +#include <iostream>\n> +\n> +#include <libcamera/orientation.h>\n> +#include <libcamera/transform.h>\n> +\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class TransformTest : public Test\n> +{\n> +protected:\n> +\tint run();\n> +};\n> +\n> +int TransformTest::run()\n> +{\n> +\t/*\n> +\t * RotationTestEntry collects two Orientation and one Transform that\n> +\t * gets combined to validate that (o1 / o2 = T) and (o1 = o2 * T)\n> +\t *\n> +\t * o1 / o2 = t computes the Transform to apply to o2 to obtain o1\n> +\t * o2 * t = o1 combines o2 with t by applying o2 first then t\n> +\t *\n> +\t * The comments on the (most complex) transform show how applying to\n> +\t * an image with orientation o2 the Transform t allows to obtain o1.\n> +\t *\n> +\t * The image with basic rotation0 is assumed to be:\n> +\t *\n> +\t * \tAB\n> +\t * \tCD\n> +\t *\n> +\t * And the Transform operators are:\n> +\t *\n> +\t * \tV = vertical flip\n> +\t * \tH = horizontal flip\n> +\t * \tT = transpose\n> +\t *\n> +\t * the operator '* (T|V)' applies V first then T.\n> +\t */\n> +\tstruct RotationTestEntry {\n\nstatic const\n\n> +\t\tOrientation o1;\n> +\t\tOrientation o2;\n> +\t\tTransform t;\n> +\t} testEntries[] = {\n> +\t\t/* Test identities transforms first. */\n> +\t\t{\n> +\t\t\tOrientation::rotate0, Orientation::rotate0,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate0Flip, Orientation::rotate0Flip,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate180, Orientation::rotate180,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate180Flip, Orientation::rotate180Flip,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate90, Orientation::rotate90,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate90Flip, Orientation::rotate90Flip,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate270, Orientation::rotate270,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate270Flip, Orientation::rotate270Flip,\n> +\t\t\tTransform::Identity,\n> +\t\t},\n> +\t\t/*\n> +\t\t * Combine 0 and 180 degrees rotation as they're the most common\n> +\t\t * ones.\n> +\t\t */\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tCD  * (H|V) =  \tBA\tAB\n> +\t\t\t *\tBA\t\tCD\tCD\n> +\t\t\t */\n> +\t\t\tOrientation::rotate0, Orientation::rotate180,\n> +\t\t\tTransform::Rot180,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tAB  * (H|V) =  \tCD\tDC\n> +\t\t\t *\tCD\t\tAB\tBA\n> +\t\t\t */\n> +\t\t\tOrientation::rotate180, Orientation::rotate0,\n> +\t\t\tTransform::Rot180\n> +\t\t},\n> +\t\t/* Test that transpositions are handled correctly. */\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tAB  * (T|V) =  \tCD\tCA\n> +\t\t\t *\tCD\t\tAB\tDB\n> +\t\t\t */\n> +\t\t\tOrientation::rotate90, Orientation::rotate0,\n> +\t\t\tTransform::Rot90,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tCA  * (T|H) =  \tAC\tAB\n> +\t\t\t *\tDB\t\tBD\tCD\n> +\t\t\t */\n> +\t\t\tOrientation::rotate0, Orientation::rotate90,\n> +\t\t\tTransform::Rot270,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tAB  * (T|H) =  \tBA\tBD\n> +\t\t\t *\tCD\t\tDC\tAC\n> +\t\t\t */\n> +\t\t\tOrientation::rotate270, Orientation::rotate0,\n> +\t\t\tTransform::Rot270,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tBD  * (T|V) =  \tAC\tAB\n> +\t\t\t *\tAC\t\tBD\tCD\n> +\t\t\t */\n> +\t\t\tOrientation::rotate0, Orientation::rotate270,\n> +\t\t\tTransform::Rot90,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tCD  * (T|H) =  \tDC\tDA\n> +\t\t\t *\tBA\t\tAB\tCB\n> +\t\t\t */\n> +\t\t\tOrientation::rotate90, Orientation::rotate180,\n> +\t\t\tTransform::Rot270,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tDA  * (T|V) =  \tCB\tCD\n> +\t\t\t *\tCB\t\tDA\tBA\n> +\t\t\t */\n> +\t\t\tOrientation::rotate180, Orientation::rotate90,\n> +\t\t\tTransform::Rot90,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tCD  * (T|V) =  \tBA\tBC\n> +\t\t\t *\tBA\t\tCD\tAD\n> +\t\t\t */\n> +\t\t\tOrientation::rotate270, Orientation::rotate180,\n> +\t\t\tTransform::Rot90,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tBC  * (T|H) =  \tCB\tCD\n> +\t\t\t *\tAD\t\tDA\tBA\n> +\t\t\t */\n> +\t\t\tOrientation::rotate180, Orientation::rotate270,\n> +\t\t\tTransform::Rot270,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tDA  * (V|H) =  \tAD\tBC\n> +\t\t\t *\tCB\t\tBC\tAD\n> +\t\t\t */\n> +\t\t\tOrientation::rotate270, Orientation::rotate90,\n> +\t\t\tTransform::Rot180,\n> +\t\t},\n> +\t\t/* Test that mirroring is handled correctly. */\n> +\t\t{\n> +\t\t\tOrientation::rotate0, Orientation::rotate0Flip,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate0Flip, Orientation::rotate0,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate180, Orientation::rotate180Flip,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate180Flip, Orientation::rotate180,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate90, Orientation::rotate90Flip,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate90Flip, Orientation::rotate90,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate270, Orientation::rotate270Flip,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate270Flip, Orientation::rotate270,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t{\n> +\t\t\tOrientation::rotate0, Orientation::rotate0Flip,\n> +\t\t\tTransform::HFlip\n> +\t\t},\n> +\t\t/*\n> +\t\t * More exotic transforms which include Transpositions and\n> +\t\t * mirroring.\n> +\t\t */\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t       o1\n> +\t\t\t *      ------------------\n> +\t\t\t *\tBC  * (V) =  \tAD\n> +\t\t\t *\tAD\t\tBC\n> +\t\t\t */\n> +\t\t\tOrientation::rotate90Flip, Orientation::rotate270,\n> +\t\t\tTransform::VFlip,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t       o1\n> +\t\t\t *      ------------------\n> +\t\t\t *\tCB  * (T) =  \tCD\n> +\t\t\t *\tDA\t\tBA\n> +\t\t\t */\n> +\t\t\tOrientation::rotate180, Orientation::rotate270Flip,\n> +\t\t\tTransform::Transpose,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t       o1\n> +\t\t\t *      ------------------\n> +\t\t\t *\tAD  * (T) =  \tAB\n> +\t\t\t *\tBC\t\tDC\n> +\t\t\t */\n> +\t\t\tOrientation::rotate0, Orientation::rotate90Flip,\n> +\t\t\tTransform::Transpose,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t       o1\n> +\t\t\t *      ------------------\n> +\t\t\t *\tAD  * (V) =  \tBC\n> +\t\t\t *\tBC\t\tAD\n> +\t\t\t */\n> +\t\t\tOrientation::rotate270, Orientation::rotate90Flip,\n> +\t\t\tTransform::VFlip,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t       o1\n> +\t\t\t *      ------------------\n> +\t\t\t *\tDA  * (V) =  \tCB\n> +\t\t\t *\tCB\t\tDA\n> +\t\t\t */\n> +\t\t\tOrientation::rotate270Flip, Orientation::rotate90,\n> +\t\t\tTransform::VFlip,\n> +\t\t},\n> +\t\t{\n> +\t\t\t/*\n> +\t\t\t *      o2      t               o1\n> +\t\t\t *      --------------------------\n> +\t\t\t *\tCB  * (V|H) =\tBC  \tAD\n> +\t\t\t *\tDA\t\tAD\tBC\n> +\t\t\t */\n> +\t\t\tOrientation::rotate90Flip, Orientation::rotate270Flip,\n> +\t\t\tTransform::Rot180,\n> +\t\t},\n> +\t};\n> +\n> +\tfor (const auto &entry : testEntries) {\n> +\t\tTransform transform = entry.o1 / entry.o2;\n> +\t\tcout << \"Testing: \" << entry.o1 << \" / \" << entry.o2\n> +\t\t     << \" = \" << transformToString(transform) << endl;\n\nI would drop this message, it's unnecessarily verbose when everything\nruns fine. The error messages below are enough.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t\tif (transform != entry.t) {\n> +\t\t\tcerr << \"Failed to validate: \" << entry.o1\n> +\t\t\t     << \" / \" << entry.o2\n> +\t\t\t     << \" = \" << transformToString(entry.t) << endl;\n> +\t\t\tcerr << \"Got back: \"\n> +\t\t\t     << transformToString(transform) << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tOrientation adjusted = entry.o2 * entry.t;\n> +\t\tif (adjusted != entry.o1) {\n> +\t\t\tcerr << \"Failed to validate: \" << entry.o2\n> +\t\t\t     << \" * \" << transformToString(entry.t)\n> +\t\t\t     << \" = \" << entry.o1 << endl;\n> +\t\t\tcerr << \"Got back: \" << adjusted << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t}\n> +\n> +\treturn TestPass;\n> +}\n> +\n> +TEST_REGISTER(TransformTest)","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 2504CC3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 18 Oct 2023 20:54:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6602961DD1;\n\tWed, 18 Oct 2023 22:54:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18A9961DD1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 18 Oct 2023 22:54:10 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 36CD8666;\n\tWed, 18 Oct 2023 22:54:02 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697662452;\n\tbh=od8Dxf3pRp2H3QNf775yZqPl8hHyAyfiKGZvreF4enw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=phexTSwy4w3h1SaBYyRqeAOT6JN/ZrakxI/IijkIX/prXxEj9/c+NX/RHpfIj1OYm\n\toC/Zu5324DveLhmHFoJvDkyPlltLFLfeGSVA9s1uFDVAc+Xu31EyqAS+EV15nBqaPr\n\tJLdNly1VNITdNvxqc6PkPcQ67PTVhGCf2/jE5exuq+TFmYz5lSOeKAE6TtJQ5xNwyP\n\tjQ5u9NAlh7gn3BfxLmDfEfbP81ZhbG1EZEcAsLRFCxrS0GUhr+Z7fxA23iUb5i5Alf\n\tyDpZTclXCRsMRZ0gquGkNXfQWKNyvr03sPh85ZRuxFfNQUs5DqLKl+zgx06r9d44er\n\th6fFLUMaHMMqA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697662442;\n\tbh=od8Dxf3pRp2H3QNf775yZqPl8hHyAyfiKGZvreF4enw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OtBvrtuS4rM0EuPRcLt8n9XU89l1pw3A0nEnT8DOjrpr/31jHhqdMX0haSXgODtJU\n\thpJPQ91yk/DgKwS7Mu16iZoQYrMNGUwXunJVnBcefn9OgR9w9yk0ovJyM6nf5QKEYg\n\tay12gK3a1nNHlKDBB8z1ZRml/AMrgfury1HWG530="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OtBvrtuS\"; dkim-atps=neutral","Date":"Wed, 18 Oct 2023 23:54:16 +0300","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Message-ID":"<20231018205416.GJ1512@pendragon.ideasonboard.com>","References":"<20230901150215.11585-1-jacopo.mondi@ideasonboard.com>\n\t<20230901150215.11585-9-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230901150215.11585-9-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 08/12] test: Add unit test for\n\tTransform and Orientation","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]