[{"id":33201,"web_url":"https://patchwork.libcamera.org/comment/33201/","msgid":"<173797495528.1773152.7001833831622468947@ping.linuxembedded.co.uk>","date":"2025-01-27T10:49:15","subject":"Re: [PATCH] libcamera: matrix: Add read-only accessor to internal\n\tdata","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2025-01-27 06:47:45)\n> Add a data() function to the Matrix class to access the internal data.\n> This is useful for code that needs to use the matrix contents as a\n> linear array, as shows by the RkISP1::Ccm::process() function that needs\n\ns/shows/shown/\n\n> to copy the matrix data to a local variable. Simplify that function by\n> using the new accessor.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/internal/matrix.h | 2 ++\n>  src/ipa/rkisp1/algorithms/ccm.cpp   | 7 +------\n>  src/libcamera/matrix.cpp            | 6 ++++++\n>  3 files changed, 9 insertions(+), 6 deletions(-)\n> \n> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h\n> index 7a71028c473a..a055e6926c94 100644\n> --- a/include/libcamera/internal/matrix.h\n> +++ b/include/libcamera/internal/matrix.h\n> @@ -66,6 +66,8 @@ public:\n>                 return out.str();\n>         }\n>  \n> +       Span<const T, Rows * Cols> data() const { return data_; }\n> +\n>         Span<const T, Cols> operator[](size_t i) const\n>         {\n>                 return Span<const T, Cols>{ &data_.data()[i * Cols], Cols };\n> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> index e2b5cf4d313e..eb8ca39e56a8 100644\n> --- a/src/ipa/rkisp1/algorithms/ccm.cpp\n> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> @@ -120,12 +120,7 @@ void Ccm::process([[maybe_unused]] IPAContext &context,\n>                   [[maybe_unused]] const rkisp1_stat_buffer *stats,\n>                   ControlList &metadata)\n>  {\n> -       float m[9];\n> -       for (unsigned int i = 0; i < 3; i++) {\n> -               for (unsigned int j = 0; j < 3; j++)\n> -                       m[i * 3 + j] = frameContext.ccm.ccm[i][j];\n> -       }\n> -       metadata.set(controls::ColourCorrectionMatrix, m);\n> +       metadata.set(controls::ColourCorrectionMatrix, frameContext.ccm.ccm.data());\n\nThat's better than iterating 3x3.\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n>  }\n>  \n>  REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n> diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp\n> index 4d95a19bfbb9..d9d34867b0a3 100644\n> --- a/src/libcamera/matrix.cpp\n> +++ b/src/libcamera/matrix.cpp\n> @@ -52,6 +52,12 @@ LOG_DEFINE_CATEGORY(Matrix)\n>   * \\return A string describing the matrix\n>   */\n>  \n> +/**\n> + * \\fn Matrix::data()\n> + * \\brief Access the internal data as a linear array\n> + * \\return A span referencing the internal data as a linear array\n> + */\n> +\n>  /**\n>   * \\fn Span<const T, Cols> Matrix::operator[](size_t i) const\n>   * \\brief Index to a row in the matrix\n> \n> base-commit: 8aef7b4dfb12f2f9bf0513625231b85a5b8f5440\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 14EC9BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Jan 2025 10:49:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 230FB68557;\n\tMon, 27 Jan 2025 11:49:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC7C06851B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jan 2025 11:49:18 +0100 (CET)","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 28D1B352;\n\tMon, 27 Jan 2025 11:48:12 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Nuq8i8CM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737974892;\n\tbh=IMaHSP5dSSNsmC684qJTlbKVvexQWvHnMhdmK+FgSvY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Nuq8i8CMHQ2Do1J0jFj6lNq0bkHbNiryhIqMmIEmhirfHEag89WPivKYrX4bXQMNe\n\t1+Hh1XoQg+jP6CUgFKrJrVMnOicHQqLelRRjv9MVpEm2paqYxvlNlezjcVJszmb9ue\n\tANflLrRJz1WrkZ0XfI9mwjd2Ng6ttZ3uveNVJ72w=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250127064745.5740-1-laurent.pinchart@ideasonboard.com>","References":"<20250127064745.5740-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH] libcamera: matrix: Add read-only accessor to internal\n\tdata","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 27 Jan 2025 10:49:15 +0000","Message-ID":"<173797495528.1773152.7001833831622468947@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":"<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":33209,"web_url":"https://patchwork.libcamera.org/comment/33209/","msgid":"<85wmegmo0s.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","date":"2025-01-27T12:36:51","subject":"Re: [PATCH] libcamera: matrix: Add read-only accessor to internal\n\tdata","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for the patch.\n\nLaurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n\n> Add a data() function to the Matrix class to access the internal data.\n> This is useful for code that needs to use the matrix contents as a\n> linear array, as shows by the RkISP1::Ccm::process() function that needs\n> to copy the matrix data to a local variable. Simplify that function by\n> using the new accessor.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/internal/matrix.h | 2 ++\n>  src/ipa/rkisp1/algorithms/ccm.cpp   | 7 +------\n>  src/libcamera/matrix.cpp            | 6 ++++++\n>  3 files changed, 9 insertions(+), 6 deletions(-)\n>\n> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h\n> index 7a71028c473a..a055e6926c94 100644\n> --- a/include/libcamera/internal/matrix.h\n> +++ b/include/libcamera/internal/matrix.h\n> @@ -66,6 +66,8 @@ public:\n>  \t\treturn out.str();\n>  \t}\n>  \n> +\tSpan<const T, Rows * Cols> data() const { return data_; }\n> +\n>  \tSpan<const T, Cols> operator[](size_t i) const\n>  \t{\n>  \t\treturn Span<const T, Cols>{ &data_.data()[i * Cols], Cols };\n> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> index e2b5cf4d313e..eb8ca39e56a8 100644\n> --- a/src/ipa/rkisp1/algorithms/ccm.cpp\n> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> @@ -120,12 +120,7 @@ void Ccm::process([[maybe_unused]] IPAContext &context,\n>  \t\t  [[maybe_unused]] const rkisp1_stat_buffer *stats,\n>  \t\t  ControlList &metadata)\n>  {\n> -\tfloat m[9];\n> -\tfor (unsigned int i = 0; i < 3; i++) {\n> -\t\tfor (unsigned int j = 0; j < 3; j++)\n> -\t\t\tm[i * 3 + j] = frameContext.ccm.ccm[i][j];\n> -\t}\n> -\tmetadata.set(controls::ColourCorrectionMatrix, m);\n> +\tmetadata.set(controls::ColourCorrectionMatrix, frameContext.ccm.ccm.data());\n>  }\n>  \n>  REGISTER_IPA_ALGORITHM(Ccm, \"Ccm\")\n> diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp\n> index 4d95a19bfbb9..d9d34867b0a3 100644\n> --- a/src/libcamera/matrix.cpp\n> +++ b/src/libcamera/matrix.cpp\n> @@ -52,6 +52,12 @@ LOG_DEFINE_CATEGORY(Matrix)\n>   * \\return A string describing the matrix\n>   */\n>  \n> +/**\n> + * \\fn Matrix::data()\n> + * \\brief Access the internal data as a linear array\n> + * \\return A span referencing the internal data as a linear array\n\n\"Internal data\" can be anything and if it is supposed to be used in\nplaces like metadata or for any \"real\" purpose, it should be better\nspecified.  Something like the documentation of `data' argument in the\nconstructor?\n\n> + */\n> +\n>  /**\n>   * \\fn Span<const T, Cols> Matrix::operator[](size_t i) const\n>   * \\brief Index to a row in the matrix\n>\n> base-commit: 8aef7b4dfb12f2f9bf0513625231b85a5b8f5440","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 0C81EBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Jan 2025 12:37:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D3CE6855D;\n\tMon, 27 Jan 2025 13:37:00 +0100 (CET)","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 DFD2168549\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jan 2025 13:36:57 +0100 (CET)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-196-8rLpX9G_PBOxH0TZODYg5A-1; Mon, 27 Jan 2025 07:36:55 -0500","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-ab68f6abdc6so154303966b.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jan 2025 04:36:55 -0800 (PST)","from mzamazal-thinkpadp1gen3.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-ab675e1254esm569909266b.20.2025.01.27.04.36.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 27 Jan 2025 04:36:52 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"YHtszLs2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1737981416;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=5ImUnKXTbvuBVD/ymf7r7K550qrQKxRLQrIZAj+MW/I=;\n\tb=YHtszLs21MDtWy+5sMKQgGTZl+p/PlJxoszQ9lVcdGCxTW7ilidBQNVk4rXwtXIPAUvmTt\n\tveM1TNnCtWX8UOO9OS3/NnjeYa2PSvDh5rb00v1J2PsH11Qz4mc27i5xXnYMlPD6NNHq4i\n\t8GX9L6PdFb1EyfwfIimIf80+7qcefb0=","X-MC-Unique":"8rLpX9G_PBOxH0TZODYg5A-1","X-Mimecast-MFC-AGG-ID":"8rLpX9G_PBOxH0TZODYg5A","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1737981414; x=1738586214;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=5ImUnKXTbvuBVD/ymf7r7K550qrQKxRLQrIZAj+MW/I=;\n\tb=lW8DwnMFJZIaHnxtar5EPSEl+zwlVilGPbmk0SV1LRH7XjUogTWBoSuzFRFVwMl+Si\n\tHdsxI58E1MwZesGHIk3pVRlyzBnnstDBhMyE7TF8TXvGTGYD03mUnCu95TISvf7ibpiF\n\tFMYhFJK73f4ZgOCiMz4ERo+6qlO2ZM7Zi7I8cUNIzbf64l6Sq3bsth2M8s8UL6RSdPfN\n\tj/YVQ8/Ep597oN8C/vG3VZ3cUeBe6VRXYQjSre+on6f0/0MFMb5de0dxhmryJmxp0mWg\n\tjVe7O6RVVXZZhPO06BtNu6KkP4Rm52w6DpEOW90+o9K0XTuGolVTfTM9i8J20IYGHHLE\n\tfaaA==","X-Gm-Message-State":"AOJu0YwAHJvfTcPqbbJYi7FXikdpWQvakchK04s9kmZLvrPnC4Sl7fwO\n\tevcHcYYkC0isyBKUwUvpXgelrTzkUZDCJ4hKnjONQuejlRqZlOOgha6xerQVPWLpBgygsQ8kjE9\n\tJYYkxz/7ky3xQF256Enf9sKiZSFiuf3QF9ZShW/CY7eWDENEHlD+lP9wZs0fXSK1c1xAVAi87Zv\n\tozGLYpzpwSKHzxsnEJ7yiqU37xGVt+Xq4FRk/yuWUdFTR3uU49zXMoH+0=","X-Gm-Gg":"ASbGncsWZAr4CkyaYH0grVCvefQE2zx5siipcryRWhpo0YmOqxHgkMeWvbPdHdp+LC6\n\tWFSGs5oQ25ItK0hxX9VPJPbo8w3LjFRFFCgpHrGx2akHCCwvo9YdRge+sugQ3GaxJCSKltjowcc\n\tDcY6P3HH/zNuLaPwuKopgYrg+rpZZCPRbhdVVQ1G3QdfqlUfOX0Fb4GvJjhD+ZVnZy1ZLFvel1V\n\tMk2oxushzbJ/P7NBZip5NCr3vgTACG4XS3SBJAIhJjX9AWRcs0rSMUoO3yoeF91u6vECs/xAXN6\n\tktt6SrgZW3MIrh25hJi8bmPi4NHD5+bXz4t2QHqgvECxzxf4V6OPi7xxGA==","X-Received":["by 2002:a05:6402:4416:b0:5d0:d3eb:a78f with SMTP id\n\t4fb4d7f45d1cf-5db7d0eca61mr102032014a12.0.1737981413691; \n\tMon, 27 Jan 2025 04:36:53 -0800 (PST)","by 2002:a05:6402:4416:b0:5d0:d3eb:a78f with SMTP id\n\t4fb4d7f45d1cf-5db7d0eca61mr102031954a12.0.1737981413284; \n\tMon, 27 Jan 2025 04:36:53 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHlrSY1+N0UFgW3Uz6s6PP6MQw1J5ijIzA/KYGvvUafgzbH6SMQdLPaIYvz3fGv+VlE556TFg==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: matrix: Add read-only accessor to internal\n\tdata","In-Reply-To":"<20250127064745.5740-1-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Mon, 27 Jan 2025 08:47:45 +0200\")","References":"<20250127064745.5740-1-laurent.pinchart@ideasonboard.com>","Date":"Mon, 27 Jan 2025 13:36:51 +0100","Message-ID":"<85wmegmo0s.fsf@mzamazal-thinkpadp1gen3.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"CKSsmBcomjbFIQqUAKOxFJSiul-hp_xDT-GR-MKmJtQ_1737981414","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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>"}}]