[{"id":35715,"web_url":"https://patchwork.libcamera.org/comment/35715/","msgid":"<175706898431.1787083.363180465216060487@neptunite.rasen.tech>","date":"2025-09-05T10:43:04","subject":"Re: [PATCH v3 01/19] ipa: rkisp1: Add basic compression algorithm","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-08-15 19:29:21)\n> The i.MX8 M Plus has a compression curve inside the compand block.  This\n> curve is necessary to process HDR stitched data and is useful for other\n> aspects like applying a digital gain to the incoming sensor data.\n> \n> Add a basic algorithm for the compression curve. This algorithm has a\n> hardcoded input width of 20bit and output width of 12bit which matches\n> the imx8mp pipeline. Only a static gain is supported in this version.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> \n> ---\n> \n> Changes in v3:\n> - Removed unused member\n> - Fixed comment referencing copy-paste source\n> - Ensure activeState.compress.supported stays false if unsupported\n> ---\n>  src/ipa/rkisp1/algorithms/compress.cpp | 102 +++++++++++++++++++++++++\n>  src/ipa/rkisp1/algorithms/compress.h   |  30 ++++++++\n>  src/ipa/rkisp1/algorithms/meson.build  |   1 +\n>  src/ipa/rkisp1/ipa_context.h           |   9 +++\n>  4 files changed, 142 insertions(+)\n>  create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp\n>  create mode 100644 src/ipa/rkisp1/algorithms/compress.h\n> \n> diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp\n> new file mode 100644\n> index 000000000000..e076727de4f1\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/compress.cpp\n> @@ -0,0 +1,102 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * RkISP1 Compression curve\n> + */\n> +#include \"compress.h\"\n> +\n> +#include <linux/videodev2.h>\n> +\n> +#include <libcamera/base/log.h>\n> +#include <libcamera/base/utils.h>\n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"linux/rkisp1-config.h\"\n> +\n> +/**\n> + * \\file compress.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Compress\n> + * \\brief RkISP1 Compress curve\n> + *\n> + * This algorithm implements support for the compressions curve in the compand\n\ns/compressions/compression/\n\n> + * block available in the i.MX8 M Plus\n> + *\n> + * In its current version it only supports a static gain. This is useful for\n> + * the agc algorithm to compensate for exposure/gain quantization effects.\n> + *\n> + * This algorithm doesn't have any configuration options. It needs to be\n> + * configured per frame by other algorithms.\n> + *\n> + * Other algorithms can check activeState.compress.supported to see if\n> + * compression is available. If it is available they can configure it per frame\n> + * using frameContext.compress.enable and frameContext.compress.gain.\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Compress)\n> +\n> +constexpr static int kRkISP1CompressInBits = 20;\n> +constexpr static int kRkISP1CompressOutBits = 12;\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::configure\n> + */\n> +int Compress::configure(IPAContext &context,\n> +                       [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> +{\n> +       if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n> +           !context.hw->compand) {\n> +               LOG(RkISP1Compress, Warning)\n> +                       << \"Compression is not supported by the hardware or kernel.\";\n> +               return 0;\n> +       }\n> +\n> +       context.activeState.compress.supported = true;\n\nimo this sounds like something that should be in the session configuration\ninstead of the active state.\n\n> +       return 0;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Compress::prepare([[maybe_unused]] IPAContext &context,\n> +                      [[maybe_unused]] const uint32_t frame,\n> +                      IPAFrameContext &frameContext,\n> +                      RkISP1Params *params)\n> +{\n> +       auto comp = params->block<BlockType::CompandCompress>();\n> +       comp.setEnabled(frameContext.compress.enable);\n\nShould this be not enabled if not frameContext.compress.enable?\n\n> +\n> +       if (!frameContext.compress.enable)\n> +               return;\n> +\n> +       int xmax = (1 << kRkISP1CompressInBits);\n> +       int ymax = (1 << kRkISP1CompressOutBits);\n> +       int inLogStep = std::log2(xmax / 64);\n\nimo s/64/RKISP1_CIF_ISP_COMPAND_NUM_POINTS/ is clearer as to what the 64 means.\n\n\nPaul\n\n> +\n> +       for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) {\n> +               double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS);\n> +               double y = x * frameContext.compress.gain;\n> +\n> +               comp->px[i] = inLogStep;\n> +               comp->x[i] = std::min<int>(x * xmax, xmax - 1);\n> +               comp->y[i] = std::min<int>(y * ymax, ymax - 1);\n> +       }\n> +\n> +       LOG(RkISP1Compress, Debug) << \"Compression: \" << kRkISP1CompressInBits\n> +                                  << \" bits to \" << kRkISP1CompressOutBits\n> +                                  << \" bits gain: \" << frameContext.compress.gain;\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Compress, \"Compress\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h\n> new file mode 100644\n> index 000000000000..87797b8ebcc5\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/compress.h\n> @@ -0,0 +1,30 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board\n> + *\n> + * RkISP1 Compression curve\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class Compress : public Algorithm\n> +{\n> +public:\n> +       Compress() = default;\n> +       ~Compress() = default;\n> +\n> +       int configure(IPAContext &context,\n> +                     const IPACameraSensorInfo &configInfo) override;\n> +       void prepare(IPAContext &context, const uint32_t frame,\n> +                    IPAFrameContext &frameContext,\n> +                    RkISP1Params *params) override;\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index c66b0b70b82f..2e42a80cf99d 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([\n>      'awb.cpp',\n>      'blc.cpp',\n>      'ccm.cpp',\n> +    'compress.cpp',\n>      'cproc.cpp',\n>      'dpcc.cpp',\n>      'dpf.cpp',\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 7ccc7b501aff..37a74215ce19 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -106,6 +106,10 @@ struct IPAActiveState {\n>                 Matrix<float, 3, 3> automatic;\n>         } ccm;\n>  \n> +       struct {\n> +               bool supported;\n> +       } compress;\n> +\n>         struct {\n>                 int8_t brightness;\n>                 uint8_t contrast;\n> @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext {\n>                 bool update;\n>         } cproc;\n>  \n> +       struct {\n> +               bool enable;\n> +               double gain;\n> +       } compress;\n> +\n>         struct {\n>                 bool denoise;\n>                 bool update;\n> -- \n> 2.48.1\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 EEB0BC332F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Sep 2025 10:43:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 103626936E;\n\tFri,  5 Sep 2025 12:43:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E00169337\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Sep 2025 12:43:10 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:dbfb:387c:7405:52bc])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 144B5F09;\n\tFri,  5 Sep 2025 12:41:58 +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=\"BS+YJvgT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1757068919;\n\tbh=XpwanVMOR49qqTaflkDqSZsqmCEux+y3t4KurEoYfi8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=BS+YJvgTYfcWys4zhKz38BdDhJLcrKUe6mIqP4PO6clN61IHrtxd3/N+3imNrKzMp\n\taNow/+RP2g+x7jGeAiaGqGYLd0xeMapzPuWnBwTcxcN3JnJplt2UEKLUYt6UesDNm2\n\tzY/Go2io9bF5Q8hDv45+5z34EQ5TQANsM4d3sTD4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250815102945.1602071-2-stefan.klug@ideasonboard.com>","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-2-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v3 01/19] ipa: rkisp1: Add basic compression algorithm","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 05 Sep 2025 19:43:04 +0900","Message-ID":"<175706898431.1787083.363180465216060487@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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":35891,"web_url":"https://patchwork.libcamera.org/comment/35891/","msgid":"<175820022661.17312.13043571464514356432@localhost>","date":"2025-09-18T12:57:06","subject":"Re: [PATCH v3 01/19] ipa: rkisp1: Add basic compression algorithm","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the review.\n\nQuoting Paul Elder (2025-09-05 12:43:04)\n> Quoting Stefan Klug (2025-08-15 19:29:21)\n> > The i.MX8 M Plus has a compression curve inside the compand block.  This\n> > curve is necessary to process HDR stitched data and is useful for other\n> > aspects like applying a digital gain to the incoming sensor data.\n> > \n> > Add a basic algorithm for the compression curve. This algorithm has a\n> > hardcoded input width of 20bit and output width of 12bit which matches\n> > the imx8mp pipeline. Only a static gain is supported in this version.\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > \n> > ---\n> > \n> > Changes in v3:\n> > - Removed unused member\n> > - Fixed comment referencing copy-paste source\n> > - Ensure activeState.compress.supported stays false if unsupported\n> > ---\n> >  src/ipa/rkisp1/algorithms/compress.cpp | 102 +++++++++++++++++++++++++\n> >  src/ipa/rkisp1/algorithms/compress.h   |  30 ++++++++\n> >  src/ipa/rkisp1/algorithms/meson.build  |   1 +\n> >  src/ipa/rkisp1/ipa_context.h           |   9 +++\n> >  4 files changed, 142 insertions(+)\n> >  create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp\n> >  create mode 100644 src/ipa/rkisp1/algorithms/compress.h\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp\n> > new file mode 100644\n> > index 000000000000..e076727de4f1\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/compress.cpp\n> > @@ -0,0 +1,102 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * RkISP1 Compression curve\n> > + */\n> > +#include \"compress.h\"\n> > +\n> > +#include <linux/videodev2.h>\n> > +\n> > +#include <libcamera/base/log.h>\n> > +#include <libcamera/base/utils.h>\n> > +\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"linux/rkisp1-config.h\"\n> > +\n> > +/**\n> > + * \\file compress.h\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +/**\n> > + * \\class Compress\n> > + * \\brief RkISP1 Compress curve\n> > + *\n> > + * This algorithm implements support for the compressions curve in the compand\n> \n> s/compressions/compression/\n> \n> > + * block available in the i.MX8 M Plus\n> > + *\n> > + * In its current version it only supports a static gain. This is useful for\n> > + * the agc algorithm to compensate for exposure/gain quantization effects.\n> > + *\n> > + * This algorithm doesn't have any configuration options. It needs to be\n> > + * configured per frame by other algorithms.\n> > + *\n> > + * Other algorithms can check activeState.compress.supported to see if\n> > + * compression is available. If it is available they can configure it per frame\n> > + * using frameContext.compress.enable and frameContext.compress.gain.\n> > + */\n> > +\n> > +LOG_DEFINE_CATEGORY(RkISP1Compress)\n> > +\n> > +constexpr static int kRkISP1CompressInBits = 20;\n> > +constexpr static int kRkISP1CompressOutBits = 12;\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::configure\n> > + */\n> > +int Compress::configure(IPAContext &context,\n> > +                       [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> > +{\n> > +       if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n> > +           !context.hw->compand) {\n> > +               LOG(RkISP1Compress, Warning)\n> > +                       << \"Compression is not supported by the hardware or kernel.\";\n> > +               return 0;\n> > +       }\n> > +\n> > +       context.activeState.compress.supported = true;\n> \n> imo this sounds like something that should be in the session configuration\n> instead of the active state.\n\nOoh, you are absolutely right. I'll move that.\n\n> \n> > +       return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::prepare\n> > + */\n> > +void Compress::prepare([[maybe_unused]] IPAContext &context,\n> > +                      [[maybe_unused]] const uint32_t frame,\n> > +                      IPAFrameContext &frameContext,\n> > +                      RkISP1Params *params)\n> > +{\n> > +       auto comp = params->block<BlockType::CompandCompress>();\n> > +       comp.setEnabled(frameContext.compress.enable);\n> \n> Should this be not enabled if not frameContext.compress.enable?\n\nI'd say so, yes. Why do you think it shouldn't?\n\n> \n> > +\n> > +       if (!frameContext.compress.enable)\n> > +               return;\n> > +\n> > +       int xmax = (1 << kRkISP1CompressInBits);\n> > +       int ymax = (1 << kRkISP1CompressOutBits);\n> > +       int inLogStep = std::log2(xmax / 64);\n> \n> imo s/64/RKISP1_CIF_ISP_COMPAND_NUM_POINTS/ is clearer as to what the 64 means.\n\nOh, yes I missed that one.\n\n> \n> \n> Paul\n\nThanks, I'll fix these and post a v4.\n\nBest regards,\nStefan\n\n> \n> > +\n> > +       for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) {\n> > +               double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS);\n> > +               double y = x * frameContext.compress.gain;\n> > +\n> > +               comp->px[i] = inLogStep;\n> > +               comp->x[i] = std::min<int>(x * xmax, xmax - 1);\n> > +               comp->y[i] = std::min<int>(y * ymax, ymax - 1);\n> > +       }\n> > +\n> > +       LOG(RkISP1Compress, Debug) << \"Compression: \" << kRkISP1CompressInBits\n> > +                                  << \" bits to \" << kRkISP1CompressOutBits\n> > +                                  << \" bits gain: \" << frameContext.compress.gain;\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(Compress, \"Compress\")\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h\n> > new file mode 100644\n> > index 000000000000..87797b8ebcc5\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/compress.h\n> > @@ -0,0 +1,30 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board\n> > + *\n> > + * RkISP1 Compression curve\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +class Compress : public Algorithm\n> > +{\n> > +public:\n> > +       Compress() = default;\n> > +       ~Compress() = default;\n> > +\n> > +       int configure(IPAContext &context,\n> > +                     const IPACameraSensorInfo &configInfo) override;\n> > +       void prepare(IPAContext &context, const uint32_t frame,\n> > +                    IPAFrameContext &frameContext,\n> > +                    RkISP1Params *params) override;\n> > +};\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > index c66b0b70b82f..2e42a80cf99d 100644\n> > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([\n> >      'awb.cpp',\n> >      'blc.cpp',\n> >      'ccm.cpp',\n> > +    'compress.cpp',\n> >      'cproc.cpp',\n> >      'dpcc.cpp',\n> >      'dpf.cpp',\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 7ccc7b501aff..37a74215ce19 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -106,6 +106,10 @@ struct IPAActiveState {\n> >                 Matrix<float, 3, 3> automatic;\n> >         } ccm;\n> >  \n> > +       struct {\n> > +               bool supported;\n> > +       } compress;\n> > +\n> >         struct {\n> >                 int8_t brightness;\n> >                 uint8_t contrast;\n> > @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext {\n> >                 bool update;\n> >         } cproc;\n> >  \n> > +       struct {\n> > +               bool enable;\n> > +               double gain;\n> > +       } compress;\n> > +\n> >         struct {\n> >                 bool denoise;\n> >                 bool update;\n> > -- \n> > 2.48.1\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 C09F9BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Sep 2025 12:57:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C09686936A;\n\tThu, 18 Sep 2025 14:57:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5909262C3B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Sep 2025 14:57:10 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:ace3:d2c2:5eff:9cc5])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 7300F842;\n\tThu, 18 Sep 2025 14:55:50 +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=\"Mu4kR/dZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758200150;\n\tbh=TUDAmLEbE2olE2kRYH8M83iu1ZchJIIxwbtNNEda59Q=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Mu4kR/dZ02q1/PXfYgkQFrnWrIu1qtMTByfanOpiQI+lvbEoOFZYXr71MCjr8YE9D\n\t083swyX4s9jCP0/fLvQ/Y7w2uPMQ3Uz4mxzNXCxTfDa99L6JPCzLk2n0R6i2sKGHfY\n\tLG2DyZKPRG3ipU/7ojULVHnFm5pQrG6vr0ypa4k4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175706898431.1787083.363180465216060487@neptunite.rasen.tech>","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-2-stefan.klug@ideasonboard.com>\n\t<175706898431.1787083.363180465216060487@neptunite.rasen.tech>","Subject":"Re: [PATCH v3 01/19] ipa: rkisp1: Add basic compression algorithm","From":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 18 Sep 2025 14:57:06 +0200","Message-ID":"<175820022661.17312.13043571464514356432@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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":35908,"web_url":"https://patchwork.libcamera.org/comment/35908/","msgid":"<175822132753.2724359.17717124577198573600@neptunite.rasen.tech>","date":"2025-09-18T18:48:47","subject":"Re: [PATCH v3 01/19] ipa: rkisp1: Add basic compression algorithm","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-09-18 21:57:06)\n> Hi Paul,\n> \n> Thank you for the review.\n> \n> Quoting Paul Elder (2025-09-05 12:43:04)\n> > Quoting Stefan Klug (2025-08-15 19:29:21)\n> > > The i.MX8 M Plus has a compression curve inside the compand block.  This\n> > > curve is necessary to process HDR stitched data and is useful for other\n> > > aspects like applying a digital gain to the incoming sensor data.\n> > > \n> > > Add a basic algorithm for the compression curve. This algorithm has a\n> > > hardcoded input width of 20bit and output width of 12bit which matches\n> > > the imx8mp pipeline. Only a static gain is supported in this version.\n> > > \n> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > > \n> > > ---\n> > > \n> > > Changes in v3:\n> > > - Removed unused member\n> > > - Fixed comment referencing copy-paste source\n> > > - Ensure activeState.compress.supported stays false if unsupported\n> > > ---\n> > >  src/ipa/rkisp1/algorithms/compress.cpp | 102 +++++++++++++++++++++++++\n> > >  src/ipa/rkisp1/algorithms/compress.h   |  30 ++++++++\n> > >  src/ipa/rkisp1/algorithms/meson.build  |   1 +\n> > >  src/ipa/rkisp1/ipa_context.h           |   9 +++\n> > >  4 files changed, 142 insertions(+)\n> > >  create mode 100644 src/ipa/rkisp1/algorithms/compress.cpp\n> > >  create mode 100644 src/ipa/rkisp1/algorithms/compress.h\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/compress.cpp b/src/ipa/rkisp1/algorithms/compress.cpp\n> > > new file mode 100644\n> > > index 000000000000..e076727de4f1\n> > > --- /dev/null\n> > > +++ b/src/ipa/rkisp1/algorithms/compress.cpp\n> > > @@ -0,0 +1,102 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2025, Ideas On Board\n> > > + *\n> > > + * RkISP1 Compression curve\n> > > + */\n> > > +#include \"compress.h\"\n> > > +\n> > > +#include <linux/videodev2.h>\n> > > +\n> > > +#include <libcamera/base/log.h>\n> > > +#include <libcamera/base/utils.h>\n> > > +\n> > > +#include \"libcamera/internal/yaml_parser.h\"\n> > > +\n> > > +#include \"linux/rkisp1-config.h\"\n> > > +\n> > > +/**\n> > > + * \\file compress.h\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::rkisp1::algorithms {\n> > > +\n> > > +/**\n> > > + * \\class Compress\n> > > + * \\brief RkISP1 Compress curve\n> > > + *\n> > > + * This algorithm implements support for the compressions curve in the compand\n> > \n> > s/compressions/compression/\n> > \n> > > + * block available in the i.MX8 M Plus\n> > > + *\n> > > + * In its current version it only supports a static gain. This is useful for\n> > > + * the agc algorithm to compensate for exposure/gain quantization effects.\n> > > + *\n> > > + * This algorithm doesn't have any configuration options. It needs to be\n> > > + * configured per frame by other algorithms.\n> > > + *\n> > > + * Other algorithms can check activeState.compress.supported to see if\n> > > + * compression is available. If it is available they can configure it per frame\n> > > + * using frameContext.compress.enable and frameContext.compress.gain.\n> > > + */\n> > > +\n> > > +LOG_DEFINE_CATEGORY(RkISP1Compress)\n> > > +\n> > > +constexpr static int kRkISP1CompressInBits = 20;\n> > > +constexpr static int kRkISP1CompressOutBits = 12;\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::configure\n> > > + */\n> > > +int Compress::configure(IPAContext &context,\n> > > +                       [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> > > +{\n> > > +       if (context.configuration.paramFormat != V4L2_META_FMT_RK_ISP1_EXT_PARAMS ||\n> > > +           !context.hw->compand) {\n> > > +               LOG(RkISP1Compress, Warning)\n> > > +                       << \"Compression is not supported by the hardware or kernel.\";\n> > > +               return 0;\n> > > +       }\n> > > +\n> > > +       context.activeState.compress.supported = true;\n> > \n> > imo this sounds like something that should be in the session configuration\n> > instead of the active state.\n> \n> Ooh, you are absolutely right. I'll move that.\n> \n> > \n> > > +       return 0;\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::prepare\n> > > + */\n> > > +void Compress::prepare([[maybe_unused]] IPAContext &context,\n> > > +                      [[maybe_unused]] const uint32_t frame,\n> > > +                      IPAFrameContext &frameContext,\n> > > +                      RkISP1Params *params)\n> > > +{\n> > > +       auto comp = params->block<BlockType::CompandCompress>();\n> > > +       comp.setEnabled(frameContext.compress.enable);\n> > \n> > Should this be not enabled if not frameContext.compress.enable?\n> \n> I'd say so, yes. Why do you think it shouldn't?\n\niirc the confirmation was about disabling the block entirely vs enabling the\nblock but setting a reset curve. I think.\n\nI'm not sure which is the correct answer so I think I was asking just to check.\n\n\nPaul\n\n> \n> > \n> > > +\n> > > +       if (!frameContext.compress.enable)\n> > > +               return;\n> > > +\n> > > +       int xmax = (1 << kRkISP1CompressInBits);\n> > > +       int ymax = (1 << kRkISP1CompressOutBits);\n> > > +       int inLogStep = std::log2(xmax / 64);\n> > \n> > imo s/64/RKISP1_CIF_ISP_COMPAND_NUM_POINTS/ is clearer as to what the 64 means.\n> \n> Oh, yes I missed that one.\n> \n> > \n> > \n> > Paul\n> \n> Thanks, I'll fix these and post a v4.\n> \n> Best regards,\n> Stefan\n> \n> > \n> > > +\n> > > +       for (unsigned int i = 0; i < RKISP1_CIF_ISP_COMPAND_NUM_POINTS; i++) {\n> > > +               double x = (i + 1) * (1.0 / RKISP1_CIF_ISP_COMPAND_NUM_POINTS);\n> > > +               double y = x * frameContext.compress.gain;\n> > > +\n> > > +               comp->px[i] = inLogStep;\n> > > +               comp->x[i] = std::min<int>(x * xmax, xmax - 1);\n> > > +               comp->y[i] = std::min<int>(y * ymax, ymax - 1);\n> > > +       }\n> > > +\n> > > +       LOG(RkISP1Compress, Debug) << \"Compression: \" << kRkISP1CompressInBits\n> > > +                                  << \" bits to \" << kRkISP1CompressOutBits\n> > > +                                  << \" bits gain: \" << frameContext.compress.gain;\n> > > +}\n> > > +\n> > > +REGISTER_IPA_ALGORITHM(Compress, \"Compress\")\n> > > +\n> > > +} /* namespace ipa::rkisp1::algorithms */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/rkisp1/algorithms/compress.h b/src/ipa/rkisp1/algorithms/compress.h\n> > > new file mode 100644\n> > > index 000000000000..87797b8ebcc5\n> > > --- /dev/null\n> > > +++ b/src/ipa/rkisp1/algorithms/compress.h\n> > > @@ -0,0 +1,30 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2025, Ideas On Board\n> > > + *\n> > > + * RkISP1 Compression curve\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include \"algorithm.h\"\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::rkisp1::algorithms {\n> > > +\n> > > +class Compress : public Algorithm\n> > > +{\n> > > +public:\n> > > +       Compress() = default;\n> > > +       ~Compress() = default;\n> > > +\n> > > +       int configure(IPAContext &context,\n> > > +                     const IPACameraSensorInfo &configInfo) override;\n> > > +       void prepare(IPAContext &context, const uint32_t frame,\n> > > +                    IPAFrameContext &frameContext,\n> > > +                    RkISP1Params *params) override;\n> > > +};\n> > > +\n> > > +} /* namespace ipa::rkisp1::algorithms */\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > > index c66b0b70b82f..2e42a80cf99d 100644\n> > > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([\n> > >      'awb.cpp',\n> > >      'blc.cpp',\n> > >      'ccm.cpp',\n> > > +    'compress.cpp',\n> > >      'cproc.cpp',\n> > >      'dpcc.cpp',\n> > >      'dpf.cpp',\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index 7ccc7b501aff..37a74215ce19 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -106,6 +106,10 @@ struct IPAActiveState {\n> > >                 Matrix<float, 3, 3> automatic;\n> > >         } ccm;\n> > >  \n> > > +       struct {\n> > > +               bool supported;\n> > > +       } compress;\n> > > +\n> > >         struct {\n> > >                 int8_t brightness;\n> > >                 uint8_t contrast;\n> > > @@ -158,6 +162,11 @@ struct IPAFrameContext : public FrameContext {\n> > >                 bool update;\n> > >         } cproc;\n> > >  \n> > > +       struct {\n> > > +               bool enable;\n> > > +               double gain;\n> > > +       } compress;\n> > > +\n> > >         struct {\n> > >                 bool denoise;\n> > >                 bool update;\n> > > -- \n> > > 2.48.1\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 25336BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Sep 2025 18:48:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E20E69371;\n\tThu, 18 Sep 2025 20:48:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B2B3B62C3A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Sep 2025 20:48:53 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:ae7e:60b0:e249:fbe5])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 3F7F1C6E;\n\tThu, 18 Sep 2025 20:47: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=\"W7rRPLdC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758221253;\n\tbh=xJQ0xJWy/6qRi9gH2BY2sAaOQFTewSVoEyV973YcFAk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=W7rRPLdCaR3mwUDUomO57x6sj88TqmCg0uJNku0oLEh7z7WfcCAjiQXG/REc+74KO\n\taFMfn/do8UkCNprWGoc16o8l5m7kTV35X/oIccE/9yTPTlTYelbZrNqjfkg1mtu47m\n\trUCq6Y13WtibfnJtp2R0jgBfTLIVngdNzFbRs10Y=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<175820022661.17312.13043571464514356432@localhost>","References":"<20250815102945.1602071-1-stefan.klug@ideasonboard.com>\n\t<20250815102945.1602071-2-stefan.klug@ideasonboard.com>\n\t<175706898431.1787083.363180465216060487@neptunite.rasen.tech>\n\t<175820022661.17312.13043571464514356432@localhost>","Subject":"Re: [PATCH v3 01/19] ipa: rkisp1: Add basic compression algorithm","From":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 19 Sep 2025 03:48:47 +0900","Message-ID":"<175822132753.2724359.17717124577198573600@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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>"}}]