[{"id":28593,"web_url":"https://patchwork.libcamera.org/comment/28593/","msgid":"<87fryo9nby.fsf@redhat.com>","date":"2024-01-23T13:44:01","subject":"Re: [PATCH v2 06/18] libcamera: introduce SoftwareIsp class","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hans de Goede writes:\n\n> From: Andrey Konovalov \n>\n> Doxygen documentation by Dennis Bonke.\n>\n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Signed-off-by: Andrey Konovalov \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n\nI don't feel like being able to say anything intelligent to this, sorry, just a\ncouple of typos:\n\n> ---\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/internal/software_isp.h | 231 ++++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/software_isp.cpp | 62 ++++++\n> 4 files changed, 295 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp.h\n> create mode 100644 src/libcamera/software_isp.cpp\n>\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 5807dfd9..1325941d 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -40,6 +40,7 @@ libcamera_internal_headers = files([\n> 'pub_key.h',\n> 'request.h',\n> 'shared_mem_object.h',\n> + 'software_isp.h',\n> 'source_paths.h',\n> 'sysfs.h',\n> 'v4l2_device.h',\n> diff --git a/include/libcamera/internal/software_isp.h b/include/libcamera/internal/software_isp.h\n> new file mode 100644\n> index 00000000..42ff48ec\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp.h\n> @@ -0,0 +1,231 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * software_isp.h - Interface for a software implementation of an ISP\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +\n> +namespace libcamera {\n> +\n> +class FrameBuffer;\n> +class PixelFormat;\n> +struct StreamConfiguration;\n> +\n> +LOG_DECLARE_CATEGORY(SoftwareIsp)\n> +\n> +/**\n> + * \\brief Base class for the Software ISP.\n> + *\n> + * Base class of the SoftwareIsp interface.\n> + */\n> +class SoftwareIsp\n> +{\n> +public:\n> +\t/**\n> +\t * \\brief Constructor for the SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tSoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls);\n> +\tvirtual ~SoftwareIsp();\n> +\n> +\t/**\n> +\t * \\brief Load a configuration from a file.\n> +\t * \\param[in] filename The file to load from.\n> +\t *\n> +\t * \\return 0 on success.\n> +\t */\n> +\tvirtual int loadConfiguration(const std::string &filename) = 0;\n> +\n> +\t/**\n> +\t * \\brief Gets if there is a valid debayer object.\n> +\t *\n> +\t * \\returns true if there is, false otherwise.\n> +\t */\n> +\tvirtual bool isValid() const = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the supported output formats.\n> +\t * \\param[in] input The input format.\n> +\t *\n> +\t * \\return all supported output formats or an empty vector if there are none.\n> +\t */\n> +\tvirtual std::vector formats(PixelFormat input) = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the supported output sizes for the given input format and size.\n> +\t * \\param[in] inputFormat The input format.\n> +\t * \\param[in] inputSize The input size.\n> +\t *\n> +\t * \\return The valid size ranges or an empty range if there are none.\n> +\t */\n> +\tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the stride and the frame size.\n> +\t * \\param[in] pixelFormat The output format.\n> +\t * \\param[in] size The output size.\n> +\t *\n> +\t * \\return a tuple of the stride and the frame size, or a tuple with 0,0 if there is no valid output config.\n> +\t */\n> +\tvirtual std::tuple\n> +\tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> +\n> +\t/**\n> +\t * \\brief Configure the SwIspSimple object according to the passed in parameters.\n> +\t * \\param[in] inputCfg The input configuration.\n> +\t * \\param[in] outputCfgs The output configurations.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return 0 on success, a negative errno on failure.\n> +\t */\n> +\tvirtual int configure(const StreamConfiguration &inputCfg,\n> +\t\t\t const std::vector> &outputCfgs,\n> +\t\t\t const ControlInfoMap &sensorControls) = 0;\n> +\n> +\t/**\n> +\t * \\brief Exports the buffers for use in processing.\n> +\t * \\param[in] output The number of outputs requested.\n> +\t * \\param[in] count The number of planes.\n> +\t * \\param[out] buffers The exported buffers.\n> +\t *\n> +\t * \\return count when successful, a negative return value if an error occurred.\n> +\t */\n> +\tvirtual int exportBuffers(unsigned int output, unsigned int count,\n> +\t\t\t\t std::vector> *buffers) = 0;\n> +\n> +\t/**\n> +\t * \\brief Starts the Software ISP worker.\n> +\t *\n> +\t * \\return 0 on success, any other value indicates an error.\n> +\t */\n> +\tvirtual int start() = 0;\n> +\n> +\t/**\n> +\t * \\brief Stops the Software ISP worker.\n> +\t */\n> +\tvirtual void stop() = 0;\n> +\n> +\t/**\n> +\t * \\brief Queues buffers for processing.\n> +\t * \\param[in] input The input framebuffer.\n> +\t * \\param[in] outputs The output framebuffers.\n> +\t *\n> +\t * \\return 0 on success, a negative errno on failure\n> +\t */\n> +\tvirtual int queueBuffers(FrameBuffer *input,\n> +\t\t\t\t const std::map &outputs) = 0;\n> +\n> +\t/**\n> +\t * \\brief Process the statistics gathered.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tvirtual void processStats(const ControlList &sensorControls) = 0; // rather merge with queueBuffers()?\n> +\n> +\t/**\n> +\t * \\brief Get the signal for when the sensor controls are set.\n> +\t *\n> +\t * \\return The control list of the sensor controls.\n> +\t */\n> +\tvirtual Signal &getSignalSetSensorControls() = 0;\n> +\n> +\t/**\n> +\t * \\brief Signals that the input buffer is ready.\n> +\t */\n> +\tSignal inputBufferReady;\n> +\t/**\n> +\t * \\brief Signals that the output buffer is ready.\n> +\t */\n> +\tSignal outputBufferReady;\n> +\n> +\t/**\n> +\t * \\brief Signals that the ISP stats are ready.\n> +\t *\n> +\t * The int parameter isn't actually used.\n> +\t */\n> +\tSignal ispStatsReady;\n> +};\n> +\n> +/**\n> + * \\brief Base class for the Software ISP Factory.\n> + *\n> + * Base class of the SoftwareIsp Factory.\n\nIs it necessary to repeat the brief section (with a slightly different wording)\nor can this sentence be omitted?\n\n> + */\n> +class SoftwareIspFactoryBase\n> +{\n> +public:\n> +\tSoftwareIspFactoryBase();\n> +\tvirtual ~SoftwareIspFactoryBase() = default;\n> +\n> +\t/**\n> +\t * \\brief Creates a SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return An unique pointer to the created SoftwareIsp object.\n\nA unique\n\n> +\t */\n> +\tstatic std::unique_ptr create(PipelineHandler *pipe,\n> +\t\t\t\t\t\t const ControlInfoMap &sensorControls);\n> +\t/**\n> +\t * \\brief Gives back a pointer to the factory.\n> +\t *\n> +\t * \\return A static pointer to the factory instance.\n> +\t */\n> +\tstatic SoftwareIspFactoryBase *&factory();\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY_AND_MOVE(SoftwareIspFactoryBase)\n> +\n> +\tstatic void registerType(SoftwareIspFactoryBase *factory);\n> +\tvirtual std::unique_ptr createInstance(PipelineHandler *pipe,\n> +\t\t\t\t\t\t\t const ControlInfoMap &sensorControls) const = 0;\n> +};\n> +\n> +/**\n> + * \\brief Implementation for the Software ISP Factory.\n> + */\n> +template\n> +class SoftwareIspFactory : public SoftwareIspFactoryBase\n> +{\n> +public:\n> +\tSoftwareIspFactory()\n> +\t\t: SoftwareIspFactoryBase()\n> +\t{\n> +\t}\n> +\n> +\t/**\n> +\t * \\brief Creates an instance of a SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return An unique pointer to the created SoftwareIsp object.\n\nA unique\n\n> +\t */\n> +\tstd::unique_ptr createInstance(PipelineHandler *pipe,\n> +\t\t\t\t\t\t const ControlInfoMap &sensorControls) const override\n> +\t{\n> +\t\treturn std::make_unique<_SoftwareIsp>(pipe, sensorControls);\n> +\t}\n> +};\n> +\n> +#define REGISTER_SOFTWAREISP(softwareIsp) \\\n> +\tstatic SoftwareIspFactory global_##softwareIsp##Factory;\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 3c5e43df..86494663 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -41,6 +41,7 @@ libcamera_sources = files([\n> 'process.cpp',\n> 'pub_key.cpp',\n> 'request.cpp',\n> + 'software_isp.cpp',\n> 'source_paths.cpp',\n> 'stream.cpp',\n> 'sysfs.cpp',\n> diff --git a/src/libcamera/software_isp.cpp b/src/libcamera/software_isp.cpp\n> new file mode 100644\n> index 00000000..2ff97d70\n> --- /dev/null\n> +++ b/src/libcamera/software_isp.cpp\n> @@ -0,0 +1,62 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * software_isp.cpp - Interface for a software implementation of an ISP\n> + */\n> +\n> +#include \"libcamera/internal/software_isp.h\"\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(SoftwareIsp)\n> +\n> +SoftwareIsp::SoftwareIsp([[maybe_unused]] PipelineHandler *pipe,\n> +\t\t\t [[maybe_unused]] const ControlInfoMap &sensorControls)\n> +{\n> +}\n> +\n> +SoftwareIsp::~SoftwareIsp()\n> +{\n> +}\n> +\n> +/* SoftwareIspFactoryBase */\n> +\n> +SoftwareIspFactoryBase::SoftwareIspFactoryBase()\n> +{\n> +\tregisterType(this);\n> +}\n> +\n> +void SoftwareIspFactoryBase::registerType(SoftwareIspFactoryBase *factory)\n> +{\n> +\tSoftwareIspFactoryBase *®istered =\n> +\t\tSoftwareIspFactoryBase::factory();\n> +\n> +\tASSERT(!registered && factory);\n> +\tregistered = factory;\n> +}\n> +\n> +SoftwareIspFactoryBase *&SoftwareIspFactoryBase::factory()\n> +{\n> +\tstatic SoftwareIspFactoryBase *factory;\n> +\treturn factory;\n> +}\n> +\n> +std::unique_ptr\n> +SoftwareIspFactoryBase::create(PipelineHandler *pipe,\n> +\t\t\t const ControlInfoMap &sensorControls)\n> +{\n> +\tSoftwareIspFactoryBase *factory = SoftwareIspFactoryBase::factory();\n> +\tif (!factory)\n> +\t\treturn nullptr;\n> +\n> +\tstd::unique_ptr swIsp = factory->createInstance(pipe, sensorControls);\n> +\tif (swIsp->isValid())\n> +\t\treturn swIsp;\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +} /* namespace libcamera */","headers":{"Return-Path":"","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 C58C2BDC71\n\tfor ;\n\tTue, 23 Jan 2024 13:44:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 24BB062944;\n\tTue, 23 Jan 2024 14:44:08 +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 8ADCD628E9\n\tfor ;\n\tTue, 23 Jan 2024 14:44:06 +0100 (CET)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-44-JCm7SBObPfKHl92Txf8u-g-1; Tue, 23 Jan 2024 08:44:04 -0500","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-40e5f548313so37981735e9.1\n\tfor ;\n\tTue, 23 Jan 2024 05:44:03 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tu13-20020a05600c19cd00b0040e451fd602sm46250027wmq.33.2024.01.23.05.44.01\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Jan 2024 05:44:02 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"EWwcCMBJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1706017445;\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=Ji4d1JNhlZo7T6oIKxLQOddp23Vv6TPUCiTD9J6hKVo=;\n\tb=EWwcCMBJlhG/h4m6M99TOeMrryPxL+mI8kz06sWa/kfHbZawifJDIuuxTxeSzOFK3Sw/MS\n\t4SpxMl865wexwqxbrMhazfQeDCUkGCNI9SS2hhWhk8hJdMo6J7CPG7vr8DZKhKGoT2AGbl\n\tXukku3y3H4fAgTmEyPCLcHMyi+9zUwk=","X-MC-Unique":"JCm7SBObPfKHl92Txf8u-g-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706017443; x=1706622243;\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=Ji4d1JNhlZo7T6oIKxLQOddp23Vv6TPUCiTD9J6hKVo=;\n\tb=Hh6JTmW5UHRjjDXCNkSSAHpnaleq3FL24/0nIBVC6z055Kqu4FUSCKVFRcuyRV26EP\n\tUKg1Aol0KRkp+dgm6vPrG8WbZhstLeJUrZulDS/An0iQfcQJHFu4xBZlN+msGCqjilPr\n\tyJPR3C22/bcCbqJPOiFPXLSPK6CBTLcqdtHg3AWA5qmJoe68LuV7h1uzxUHk/actrC50\n\tXJrrmafEEKJwF62WHCutyf2Q4rhagrF/JvHXmpmrEqaI3BoIiuzxvg2wCE9S9aE/Eu8C\n\tSNYdnqbsAjt/sDwEfPxmh5JsPcNU2Xi7jf9/pzIKF9pToeXOAhLlJ3QamscOxF8ywk3Y\n\tYEyQ==","X-Gm-Message-State":"AOJu0YzvnyYNvRcm2B8V37wgSkv3PrusEUiLPn/A2MkFRjEZpdK/CHba\n\tmqzi1AiOAbuwY15d8m+dDhMRWDK4ESRJKcQQOlR5Pv50t8W2VHYrtd78T8iBHjPBz3BXLtEON5e\n\tjH0SIk1SRb2vfTZ8GxTfCD7egem0iBQscQmmyMkdZrVLqcgoEarTa6sz4srSThYTzAgutLuQ=","X-Received":["by 2002:a05:600c:3224:b0:40e:7081:4d78 with SMTP id\n\tr36-20020a05600c322400b0040e70814d78mr516161wmp.140.1706017442950; \n\tTue, 23 Jan 2024 05:44:02 -0800 (PST)","by 2002:a05:600c:3224:b0:40e:7081:4d78 with SMTP id\n\tr36-20020a05600c322400b0040e70814d78mr516156wmp.140.1706017442572; \n\tTue, 23 Jan 2024 05:44:02 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IF6d/6zjHYij0aMAvRjhWTq9t0E7L27wULZP+MPiMmbnuf+//MZNCd6RsSPOsFBRg2a6NXRZw==","From":"Milan Zamazal ","To":"Hans de Goede ","Subject":"Re: [PATCH v2 06/18] libcamera: introduce SoftwareIsp class","In-Reply-To":"<20240113142218.28063-7-hdegoede@redhat.com> (Hans de Goede's\n\tmessage of \"Sat, 13 Jan 2024 15:22:06 +0100\")","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-7-hdegoede@redhat.com>","Date":"Tue, 23 Jan 2024 14:44:01 +0100","Message-ID":"<87fryo9nby.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28605,"web_url":"https://patchwork.libcamera.org/comment/28605/","msgid":"<46e9c419-3b9e-4659-b5ba-3a404c4b4b75@gmail.com>","date":"2024-01-23T14:50:39","subject":"Re: [PATCH v2 06/18] libcamera: introduce SoftwareIsp class","submitter":{"id":179,"url":"https://patchwork.libcamera.org/api/people/179/","name":"Andrei Konovalov","email":"andrey.konovalov.ynk@gmail.com"},"content":"Hi All,\n\nAs an early note for the reviewers.\n\nAs part of the rework to use \"-Dpipelines=simple -Dipas=simple\" and to instantiate\nthe Soft ISP if enabled in the SimplePipelineInfo table, in the next version of my\npatches Soft ISP factory will be dropped, IPASoftBase will be merged into IPASoftSimple,\nand SwIspSimple will be merged into SoftwareIsp.\n\nThanks,\nAndrei\n\nOn 13.01.2024 17:22, Hans de Goede wrote:\n> From: Andrey Konovalov \n> \n> Doxygen documentation by Dennis Bonke.\n> \n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Signed-off-by: Andrey Konovalov \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> ---\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/internal/software_isp.h | 231 ++++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/software_isp.cpp | 62 ++++++\n> 4 files changed, 295 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp.h\n> create mode 100644 src/libcamera/software_isp.cpp\n> \n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 5807dfd9..1325941d 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -40,6 +40,7 @@ libcamera_internal_headers = files([\n> 'pub_key.h',\n> 'request.h',\n> 'shared_mem_object.h',\n> + 'software_isp.h',\n> 'source_paths.h',\n> 'sysfs.h',\n> 'v4l2_device.h',\n> diff --git a/include/libcamera/internal/software_isp.h b/include/libcamera/internal/software_isp.h\n> new file mode 100644\n> index 00000000..42ff48ec\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp.h\n> @@ -0,0 +1,231 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * software_isp.h - Interface for a software implementation of an ISP\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +\n> +namespace libcamera {\n> +\n> +class FrameBuffer;\n> +class PixelFormat;\n> +struct StreamConfiguration;\n> +\n> +LOG_DECLARE_CATEGORY(SoftwareIsp)\n> +\n> +/**\n> + * \\brief Base class for the Software ISP.\n> + *\n> + * Base class of the SoftwareIsp interface.\n> + */\n> +class SoftwareIsp\n> +{\n> +public:\n> +\t/**\n> +\t * \\brief Constructor for the SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tSoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls);\n> +\tvirtual ~SoftwareIsp();\n> +\n> +\t/**\n> +\t * \\brief Load a configuration from a file.\n> +\t * \\param[in] filename The file to load from.\n> +\t *\n> +\t * \\return 0 on success.\n> +\t */\n> +\tvirtual int loadConfiguration(const std::string &filename) = 0;\n> +\n> +\t/**\n> +\t * \\brief Gets if there is a valid debayer object.\n> +\t *\n> +\t * \\returns true if there is, false otherwise.\n> +\t */\n> +\tvirtual bool isValid() const = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the supported output formats.\n> +\t * \\param[in] input The input format.\n> +\t *\n> +\t * \\return all supported output formats or an empty vector if there are none.\n> +\t */\n> +\tvirtual std::vector formats(PixelFormat input) = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the supported output sizes for the given input format and size.\n> +\t * \\param[in] inputFormat The input format.\n> +\t * \\param[in] inputSize The input size.\n> +\t *\n> +\t * \\return The valid size ranges or an empty range if there are none.\n> +\t */\n> +\tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the stride and the frame size.\n> +\t * \\param[in] pixelFormat The output format.\n> +\t * \\param[in] size The output size.\n> +\t *\n> +\t * \\return a tuple of the stride and the frame size, or a tuple with 0,0 if there is no valid output config.\n> +\t */\n> +\tvirtual std::tuple\n> +\tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> +\n> +\t/**\n> +\t * \\brief Configure the SwIspSimple object according to the passed in parameters.\n> +\t * \\param[in] inputCfg The input configuration.\n> +\t * \\param[in] outputCfgs The output configurations.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return 0 on success, a negative errno on failure.\n> +\t */\n> +\tvirtual int configure(const StreamConfiguration &inputCfg,\n> +\t\t\t const std::vector> &outputCfgs,\n> +\t\t\t const ControlInfoMap &sensorControls) = 0;\n> +\n> +\t/**\n> +\t * \\brief Exports the buffers for use in processing.\n> +\t * \\param[in] output The number of outputs requested.\n> +\t * \\param[in] count The number of planes.\n> +\t * \\param[out] buffers The exported buffers.\n> +\t *\n> +\t * \\return count when successful, a negative return value if an error occurred.\n> +\t */\n> +\tvirtual int exportBuffers(unsigned int output, unsigned int count,\n> +\t\t\t\t std::vector> *buffers) = 0;\n> +\n> +\t/**\n> +\t * \\brief Starts the Software ISP worker.\n> +\t *\n> +\t * \\return 0 on success, any other value indicates an error.\n> +\t */\n> +\tvirtual int start() = 0;\n> +\n> +\t/**\n> +\t * \\brief Stops the Software ISP worker.\n> +\t */\n> +\tvirtual void stop() = 0;\n> +\n> +\t/**\n> +\t * \\brief Queues buffers for processing.\n> +\t * \\param[in] input The input framebuffer.\n> +\t * \\param[in] outputs The output framebuffers.\n> +\t *\n> +\t * \\return 0 on success, a negative errno on failure\n> +\t */\n> +\tvirtual int queueBuffers(FrameBuffer *input,\n> +\t\t\t\t const std::map &outputs) = 0;\n> +\n> +\t/**\n> +\t * \\brief Process the statistics gathered.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tvirtual void processStats(const ControlList &sensorControls) = 0; // rather merge with queueBuffers()?\n> +\n> +\t/**\n> +\t * \\brief Get the signal for when the sensor controls are set.\n> +\t *\n> +\t * \\return The control list of the sensor controls.\n> +\t */\n> +\tvirtual Signal &getSignalSetSensorControls() = 0;\n> +\n> +\t/**\n> +\t * \\brief Signals that the input buffer is ready.\n> +\t */\n> +\tSignal inputBufferReady;\n> +\t/**\n> +\t * \\brief Signals that the output buffer is ready.\n> +\t */\n> +\tSignal outputBufferReady;\n> +\n> +\t/**\n> +\t * \\brief Signals that the ISP stats are ready.\n> +\t *\n> +\t * The int parameter isn't actually used.\n> +\t */\n> +\tSignal ispStatsReady;\n> +};\n> +\n> +/**\n> + * \\brief Base class for the Software ISP Factory.\n> + *\n> + * Base class of the SoftwareIsp Factory.\n> + */\n> +class SoftwareIspFactoryBase\n> +{\n> +public:\n> +\tSoftwareIspFactoryBase();\n> +\tvirtual ~SoftwareIspFactoryBase() = default;\n> +\n> +\t/**\n> +\t * \\brief Creates a SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return An unique pointer to the created SoftwareIsp object.\n> +\t */\n> +\tstatic std::unique_ptr create(PipelineHandler *pipe,\n> +\t\t\t\t\t\t const ControlInfoMap &sensorControls);\n> +\t/**\n> +\t * \\brief Gives back a pointer to the factory.\n> +\t *\n> +\t * \\return A static pointer to the factory instance.\n> +\t */\n> +\tstatic SoftwareIspFactoryBase *&factory();\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY_AND_MOVE(SoftwareIspFactoryBase)\n> +\n> +\tstatic void registerType(SoftwareIspFactoryBase *factory);\n> +\tvirtual std::unique_ptr createInstance(PipelineHandler *pipe,\n> +\t\t\t\t\t\t\t const ControlInfoMap &sensorControls) const = 0;\n> +};\n> +\n> +/**\n> + * \\brief Implementation for the Software ISP Factory.\n> + */\n> +template\n> +class SoftwareIspFactory : public SoftwareIspFactoryBase\n> +{\n> +public:\n> +\tSoftwareIspFactory()\n> +\t\t: SoftwareIspFactoryBase()\n> +\t{\n> +\t}\n> +\n> +\t/**\n> +\t * \\brief Creates an instance of a SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return An unique pointer to the created SoftwareIsp object.\n> +\t */\n> +\tstd::unique_ptr createInstance(PipelineHandler *pipe,\n> +\t\t\t\t\t\t const ControlInfoMap &sensorControls) const override\n> +\t{\n> +\t\treturn std::make_unique<_SoftwareIsp>(pipe, sensorControls);\n> +\t}\n> +};\n> +\n> +#define REGISTER_SOFTWAREISP(softwareIsp) \\\n> +\tstatic SoftwareIspFactory global_##softwareIsp##Factory;\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 3c5e43df..86494663 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -41,6 +41,7 @@ libcamera_sources = files([\n> 'process.cpp',\n> 'pub_key.cpp',\n> 'request.cpp',\n> + 'software_isp.cpp',\n> 'source_paths.cpp',\n> 'stream.cpp',\n> 'sysfs.cpp',\n> diff --git a/src/libcamera/software_isp.cpp b/src/libcamera/software_isp.cpp\n> new file mode 100644\n> index 00000000..2ff97d70\n> --- /dev/null\n> +++ b/src/libcamera/software_isp.cpp\n> @@ -0,0 +1,62 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * software_isp.cpp - Interface for a software implementation of an ISP\n> + */\n> +\n> +#include \"libcamera/internal/software_isp.h\"\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(SoftwareIsp)\n> +\n> +SoftwareIsp::SoftwareIsp([[maybe_unused]] PipelineHandler *pipe,\n> +\t\t\t [[maybe_unused]] const ControlInfoMap &sensorControls)\n> +{\n> +}\n> +\n> +SoftwareIsp::~SoftwareIsp()\n> +{\n> +}\n> +\n> +/* SoftwareIspFactoryBase */\n> +\n> +SoftwareIspFactoryBase::SoftwareIspFactoryBase()\n> +{\n> +\tregisterType(this);\n> +}\n> +\n> +void SoftwareIspFactoryBase::registerType(SoftwareIspFactoryBase *factory)\n> +{\n> +\tSoftwareIspFactoryBase *®istered =\n> +\t\tSoftwareIspFactoryBase::factory();\n> +\n> +\tASSERT(!registered && factory);\n> +\tregistered = factory;\n> +}\n> +\n> +SoftwareIspFactoryBase *&SoftwareIspFactoryBase::factory()\n> +{\n> +\tstatic SoftwareIspFactoryBase *factory;\n> +\treturn factory;\n> +}\n> +\n> +std::unique_ptr\n> +SoftwareIspFactoryBase::create(PipelineHandler *pipe,\n> +\t\t\t const ControlInfoMap &sensorControls)\n> +{\n> +\tSoftwareIspFactoryBase *factory = SoftwareIspFactoryBase::factory();\n> +\tif (!factory)\n> +\t\treturn nullptr;\n> +\n> +\tstd::unique_ptr swIsp = factory->createInstance(pipe, sensorControls);\n> +\tif (swIsp->isValid())\n> +\t\treturn swIsp;\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +} /* namespace libcamera */","headers":{"Return-Path":"","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 33B5BC323E\n\tfor ;\n\tTue, 23 Jan 2024 14:50:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DCDEE62949;\n\tTue, 23 Jan 2024 15:50:42 +0100 (CET)","from mail-wr1-x429.google.com (mail-wr1-x429.google.com\n\t[IPv6:2a00:1450:4864:20::429])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1FE462916\n\tfor ;\n\tTue, 23 Jan 2024 15:50:41 +0100 (CET)","by mail-wr1-x429.google.com with SMTP id\n\tffacd0b85a97d-338aca547d9so3515033f8f.0\n\tfor ;\n\tTue, 23 Jan 2024 06:50:41 -0800 (PST)","from [192.168.118.26] ([87.116.160.165])\n\tby smtp.gmail.com with ESMTPSA id\n\tx7-20020a5d6507000000b003392940f749sm8907461wru.77.2024.01.23.06.50.40\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 23 Jan 2024 06:50:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"epwmRu36\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1706021441; x=1706626241;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:references:cc:to\n\t:content-language:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=FgS2tCZ8QmVKmkx7BRMkRHaGYobJDDjPbOJRiNPqUqQ=;\n\tb=epwmRu36Z2/sqxOXbxq/feOVZTHWPLsM1pNZtdIopGlqFFmW11ZB1ZNLxM1zhTal98\n\tHw4jlEfTe7NshUNOeVf7m3oQqENQ5ZSZXGmrl6LzWhRbR0XD/0hmKIrxUMZR4NBjodx9\n\tA1zGle9UiASIBdkLQ4x0V1aSejkvrOFcSnVRsOHoKlNRDxyqjqmpc1i7vIOsB1mOM47o\n\tXcOt0azxgZlw05wkrEc0j87I+DLyeX4XeM+9+kaVz0s0cy4L2qB8Xw2LCeZdYBqf3evg\n\tSgO2lqdMZI4226TH1gceg4j7J/bLCoAfH9WP+nhAe6f1HCFPt6gTYOVwElxo1VKInm4O\n\tHLdA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706021441; x=1706626241;\n\th=content-transfer-encoding:in-reply-to:from:references:cc:to\n\t:content-language:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=FgS2tCZ8QmVKmkx7BRMkRHaGYobJDDjPbOJRiNPqUqQ=;\n\tb=MFGLT7YDjXBY3PpX+6b/Lr6KFGx4SppI76sGmUe6Bp0WGVBGQoefw+4YhDbNyP+/Fv\n\tKZhxEx+kIyQmk8hN4sHJu4II34TRbNMi7iHN07SanbDkHjTfDyuzvY+JufXuDcGX3kgw\n\tKFHsaU9ZbV4ZkLcowFRXN2ESbEWig+3w6s4rcq8qZve/jb/CCOkdVpIXkg0FR93nGJwj\n\tp4mU4TJxOkLEBNzv6qvcOk1QdL8O21sVlBIVjQhbquYAg+93Gu7IE0+nCUWrk+VgE2p9\n\tXZoA0/DNmhGazAV3ffafp7Lbvw3GF0mZ8Vd7CJVIRSMPUx8Q21D1qXPRuBXo+WqIVUky\n\tD4cw==","X-Gm-Message-State":"AOJu0Yyj+h1OiRUOofovAq2AMB7vDwAkHD/thENSvALvJ+uR6R16jmx6\n\tUeR0K+rwyHuciEO8RlclqQ97OCDvnJcF7Ygl9PEUSeHHPZ0QxR1j","X-Google-Smtp-Source":"AGHT+IGHJHUFTbhSE6Pa3UkybRvj41ZTg7ljlNpcyyHKqHKIiyWi76XU3W8yy4cMkwsHvV1r48j1nA==","X-Received":"by 2002:a5d:524a:0:b0:337:c25b:66c2 with SMTP id\n\tk10-20020a5d524a000000b00337c25b66c2mr1703282wrc.143.1706021441164; \n\tTue, 23 Jan 2024 06:50:41 -0800 (PST)","Message-ID":"<46e9c419-3b9e-4659-b5ba-3a404c4b4b75@gmail.com>","Date":"Tue, 23 Jan 2024 17:50:39 +0300","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 06/18] libcamera: introduce SoftwareIsp class","Content-Language":"en-US","To":"Hans de Goede , libcamera-devel@lists.libcamera.org","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-7-hdegoede@redhat.com>","From":"Andrei Konovalov ","In-Reply-To":"<20240113142218.28063-7-hdegoede@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, srinivas.kandagatla@linaro.org,\n\tPavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28606,"web_url":"https://patchwork.libcamera.org/comment/28606/","msgid":"<20240123145731.GI10679@pendragon.ideasonboard.com>","date":"2024-01-23T14:57:31","subject":"Re: [PATCH v2 06/18] libcamera: introduce SoftwareIsp class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Tue, Jan 23, 2024 at 02:44:01PM +0100, Milan Zamazal wrote:\n> Hans de Goede writes:\n> \n> > From: Andrey Konovalov \n> >\n> > Doxygen documentation by Dennis Bonke.\n> >\n> > Co-authored-by: Dennis Bonke \n> > Signed-off-by: Dennis Bonke \n> > Signed-off-by: Andrey Konovalov \n> > Signed-off-by: Hans de Goede \n> > Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> > Tested-by: Pavel Machek \n> \n> I don't feel like being able to say anything intelligent to this, sorry, just a\n> couple of typos:\n> \n> > ---\n> > include/libcamera/internal/meson.build | 1 +\n> > include/libcamera/internal/software_isp.h | 231 ++++++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > src/libcamera/software_isp.cpp | 62 ++++++\n> > 4 files changed, 295 insertions(+)\n> > create mode 100644 include/libcamera/internal/software_isp.h\n> > create mode 100644 src/libcamera/software_isp.cpp\n> >\n> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> > index 5807dfd9..1325941d 100644\n> > --- a/include/libcamera/internal/meson.build\n> > +++ b/include/libcamera/internal/meson.build\n> > @@ -40,6 +40,7 @@ libcamera_internal_headers = files([\n> > 'pub_key.h',\n> > 'request.h',\n> > 'shared_mem_object.h',\n> > + 'software_isp.h',\n> > 'source_paths.h',\n> > 'sysfs.h',\n> > 'v4l2_device.h',\n> > diff --git a/include/libcamera/internal/software_isp.h b/include/libcamera/internal/software_isp.h\n> > new file mode 100644\n> > index 00000000..42ff48ec\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/software_isp.h\n> > @@ -0,0 +1,231 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Linaro Ltd\n> > + *\n> > + * software_isp.h - Interface for a software implementation of an ISP\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/pipeline_handler.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +class FrameBuffer;\n> > +class PixelFormat;\n> > +struct StreamConfiguration;\n> > +\n> > +LOG_DECLARE_CATEGORY(SoftwareIsp)\n> > +\n> > +/**\n> > + * \\brief Base class for the Software ISP.\n> > + *\n> > + * Base class of the SoftwareIsp interface.\n> > + */\n\nDocumentation goes to .cpp files.\n\n> > +class SoftwareIsp\n> > +{\n> > +public:\n> > +\t/**\n> > +\t * \\brief Constructor for the SoftwareIsp object.\n> > +\t * \\param[in] pipe The pipeline handler in use.\n> > +\t * \\param[in] sensorControls The sensor controls.\n\nPlease see existing documentation and follow the same style. No period\nad the end of lines for the brief and param.\n\n> > +\t */\n> > +\tSoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls);\n> > +\tvirtual ~SoftwareIsp();\n> > +\n> > +\t/**\n> > +\t * \\brief Load a configuration from a file.\n> > +\t * \\param[in] filename The file to load from.\n> > +\t *\n> > +\t * \\return 0 on success.\n> > +\t */\n> > +\tvirtual int loadConfiguration(const std::string &filename) = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Gets if there is a valid debayer object.\n\nReading this patch I don't know what a debayer object is, what it means\nto have a valid one, or how it could be invalid.\n\n> > +\t *\n> > +\t * \\returns true if there is, false otherwise.\n\ns/true/True/\n\nSimilar comments below.\n\nIt's \\return, not \\returns. Have you compiled the documentation and\nfixed all warnings/errors ?\n\n> > +\t */\n> > +\tvirtual bool isValid() const = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Get the supported output formats.\n> > +\t * \\param[in] input The input format.\n> > +\t *\n> > +\t * \\return all supported output formats or an empty vector if there are none.\n> > +\t */\n> > +\tvirtual std::vector formats(PixelFormat input) = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Get the supported output sizes for the given input format and size.\n> > +\t * \\param[in] inputFormat The input format.\n> > +\t * \\param[in] inputSize The input size.\n> > +\t *\n> > +\t * \\return The valid size ranges or an empty range if there are none.\n> > +\t */\n> > +\tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Get the stride and the frame size.\n> > +\t * \\param[in] pixelFormat The output format.\n> > +\t * \\param[in] size The output size.\n> > +\t *\n> > +\t * \\return a tuple of the stride and the frame size, or a tuple with 0,0 if there is no valid output config.\n> > +\t */\n> > +\tvirtual std::tuple\n> > +\tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Configure the SwIspSimple object according to the passed in parameters.\n> > +\t * \\param[in] inputCfg The input configuration.\n> > +\t * \\param[in] outputCfgs The output configurations.\n> > +\t * \\param[in] sensorControls The sensor controls.\n> > +\t *\n> > +\t * \\return 0 on success, a negative errno on failure.\n> > +\t */\n> > +\tvirtual int configure(const StreamConfiguration &inputCfg,\n> > +\t\t\t const std::vector> &outputCfgs,\n> > +\t\t\t const ControlInfoMap &sensorControls) = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Exports the buffers for use in processing.\n> > +\t * \\param[in] output The number of outputs requested.\n> > +\t * \\param[in] count The number of planes.\n> > +\t * \\param[out] buffers The exported buffers.\n> > +\t *\n> > +\t * \\return count when successful, a negative return value if an error occurred.\n> > +\t */\n> > +\tvirtual int exportBuffers(unsigned int output, unsigned int count,\n> > +\t\t\t\t std::vector> *buffers) = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Starts the Software ISP worker.\n> > +\t *\n> > +\t * \\return 0 on success, any other value indicates an error.\n> > +\t */\n> > +\tvirtual int start() = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Stops the Software ISP worker.\n> > +\t */\n> > +\tvirtual void stop() = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Queues buffers for processing.\n> > +\t * \\param[in] input The input framebuffer.\n> > +\t * \\param[in] outputs The output framebuffers.\n> > +\t *\n> > +\t * \\return 0 on success, a negative errno on failure\n> > +\t */\n> > +\tvirtual int queueBuffers(FrameBuffer *input,\n> > +\t\t\t\t const std::map &outputs) = 0;\n> > +\n> > +\t/**\n> > +\t * \\brief Process the statistics gathered.\n> > +\t * \\param[in] sensorControls The sensor controls.\n> > +\t */\n> > +\tvirtual void processStats(const ControlList &sensorControls) = 0; // rather merge with queueBuffers()?\n\nThis comment needs to be addressed or dropped.\n\n> > +\n> > +\t/**\n> > +\t * \\brief Get the signal for when the sensor controls are set.\n> > +\t *\n> > +\t * \\return The control list of the sensor controls.\n> > +\t */\n> > +\tvirtual Signal &getSignalSetSensorControls() = 0;\n\nSignals should be named according to the event they represent. For\ninstance, this may be\n\n\tvirtual Signal &ensorControlsReady() = 0;\n\nBut why does it have to be a function, when the next three signals are\nplain Signal instances ?\n\n> > +\n> > +\t/**\n> > +\t * \\brief Signals that the input buffer is ready.\n> > +\t */\n> > +\tSignal inputBufferReady;\n> > +\t/**\n> > +\t * \\brief Signals that the output buffer is ready.\n> > +\t */\n> > +\tSignal outputBufferReady;\n> > +\n> > +\t/**\n> > +\t * \\brief Signals that the ISP stats are ready.\n> > +\t *\n> > +\t * The int parameter isn't actually used.\n> > +\t */\n> > +\tSignal ispStatsReady;\n> > +};\n> > +\n> > +/**\n> > + * \\brief Base class for the Software ISP Factory.\n> > + *\n> > + * Base class of the SoftwareIsp Factory.\n> \n> Is it necessary to repeat the brief section (with a slightly different wording)\n> or can this sentence be omitted?\n\nIt doesn't have to be repeated, and could be omitted. Expect that it\nshould instead be expanded to explain how the class works :-)\nDocumentation is overall too terse in this file.\n\n> > + */\n> > +class SoftwareIspFactoryBase\n> > +{\n> > +public:\n> > +\tSoftwareIspFactoryBase();\n> > +\tvirtual ~SoftwareIspFactoryBase() = default;\n> > +\n> > +\t/**\n> > +\t * \\brief Creates a SoftwareIsp object.\n> > +\t * \\param[in] pipe The pipeline handler in use.\n> > +\t * \\param[in] sensorControls The sensor controls.\n> > +\t *\n> > +\t * \\return An unique pointer to the created SoftwareIsp object.\n> \n> A unique\n> \n> > +\t */\n> > +\tstatic std::unique_ptr create(PipelineHandler *pipe,\n> > +\t\t\t\t\t\t const ControlInfoMap &sensorControls);\n> > +\t/**\n> > +\t * \\brief Gives back a pointer to the factory.\n> > +\t *\n> > +\t * \\return A static pointer to the factory instance.\n> > +\t */\n> > +\tstatic SoftwareIspFactoryBase *&factory();\n> > +\n> > +private:\n> > +\tLIBCAMERA_DISABLE_COPY_AND_MOVE(SoftwareIspFactoryBase)\n> > +\n> > +\tstatic void registerType(SoftwareIspFactoryBase *factory);\n> > +\tvirtual std::unique_ptr createInstance(PipelineHandler *pipe,\n> > +\t\t\t\t\t\t\t const ControlInfoMap &sensorControls) const = 0;\n> > +};\n> > +\n> > +/**\n> > + * \\brief Implementation for the Software ISP Factory.\n> > + */\n> > +template\n> > +class SoftwareIspFactory : public SoftwareIspFactoryBase\n> > +{\n> > +public:\n> > +\tSoftwareIspFactory()\n> > +\t\t: SoftwareIspFactoryBase()\n> > +\t{\n> > +\t}\n> > +\n> > +\t/**\n> > +\t * \\brief Creates an instance of a SoftwareIsp object.\n> > +\t * \\param[in] pipe The pipeline handler in use.\n> > +\t * \\param[in] sensorControls The sensor controls.\n> > +\t *\n> > +\t * \\return An unique pointer to the created SoftwareIsp object.\n> \n> A unique\n> \n> > +\t */\n> > +\tstd::unique_ptr createInstance(PipelineHandler *pipe,\n> > +\t\t\t\t\t\t const ControlInfoMap &sensorControls) const override\n> > +\t{\n> > +\t\treturn std::make_unique<_SoftwareIsp>(pipe, sensorControls);\n> > +\t}\n> > +};\n> > +\n> > +#define REGISTER_SOFTWAREISP(softwareIsp) \\\n> > +\tstatic SoftwareIspFactory global_##softwareIsp##Factory;\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 3c5e43df..86494663 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -41,6 +41,7 @@ libcamera_sources = files([\n> > 'process.cpp',\n> > 'pub_key.cpp',\n> > 'request.cpp',\n> > + 'software_isp.cpp',\n> > 'source_paths.cpp',\n> > 'stream.cpp',\n> > 'sysfs.cpp',\n> > diff --git a/src/libcamera/software_isp.cpp b/src/libcamera/software_isp.cpp\n> > new file mode 100644\n> > index 00000000..2ff97d70\n> > --- /dev/null\n> > +++ b/src/libcamera/software_isp.cpp\n> > @@ -0,0 +1,62 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Linaro Ltd\n> > + *\n> > + * software_isp.cpp - Interface for a software implementation of an ISP\n> > + */\n> > +\n> > +#include \"libcamera/internal/software_isp.h\"\n> > +\n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(SoftwareIsp)\n> > +\n> > +SoftwareIsp::SoftwareIsp([[maybe_unused]] PipelineHandler *pipe,\n> > +\t\t\t [[maybe_unused]] const ControlInfoMap &sensorControls)\n> > +{\n> > +}\n> > +\n> > +SoftwareIsp::~SoftwareIsp()\n> > +{\n> > +}\n> > +\n> > +/* SoftwareIspFactoryBase */\n> > +\n> > +SoftwareIspFactoryBase::SoftwareIspFactoryBase()\n> > +{\n> > +\tregisterType(this);\n> > +}\n> > +\n> > +void SoftwareIspFactoryBase::registerType(SoftwareIspFactoryBase *factory)\n> > +{\n> > +\tSoftwareIspFactoryBase *®istered =\n> > +\t\tSoftwareIspFactoryBase::factory();\n> > +\n> > +\tASSERT(!registered && factory);\n> > +\tregistered = factory;\n> > +}\n> > +\n> > +SoftwareIspFactoryBase *&SoftwareIspFactoryBase::factory()\n> > +{\n> > +\tstatic SoftwareIspFactoryBase *factory;\n> > +\treturn factory;\n> > +}\n> > +\n> > +std::unique_ptr\n> > +SoftwareIspFactoryBase::create(PipelineHandler *pipe,\n> > +\t\t\t const ControlInfoMap &sensorControls)\n> > +{\n> > +\tSoftwareIspFactoryBase *factory = SoftwareIspFactoryBase::factory();\n> > +\tif (!factory)\n> > +\t\treturn nullptr;\n> > +\n> > +\tstd::unique_ptr swIsp = factory->createInstance(pipe, sensorControls);\n> > +\tif (swIsp->isValid())\n> > +\t\treturn swIsp;\n> > +\n> > +\treturn nullptr;\n> > +}\n> > +\n> > +} /* namespace libcamera */","headers":{"Return-Path":"","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 A8790BDC71\n\tfor ;\n\tTue, 23 Jan 2024 14:57:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF29262945;\n\tTue, 23 Jan 2024 15:57:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 347AA62916\n\tfor ;\n\tTue, 23 Jan 2024 15:57:33 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 423DF1890;\n\tTue, 23 Jan 2024 15:56:19 +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=\"IczArv3F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706021779;\n\tbh=tevMq7gpMe96nLWTjngE4VI0TMd8OsUWiItV5t/zQL0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IczArv3FclHzT/EHnnCuJQHaTNbD2JpL6C5gpEO2v8CP1i6KOg9D5jQGbOb6zJE7e\n\t7GFnSeuEoogPmGi1Qj++vKdtIcXdBSHjCCs1N6+naGIrBz6uBzTNOL5uivctHpdqrF\n\tNmOJ1sqMGFiQwDX769lqrose8wAADuejnD2VpMio=","Date":"Tue, 23 Jan 2024 16:57:31 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Subject":"Re: [PATCH v2 06/18] libcamera: introduce SoftwareIsp class","Message-ID":"<20240123145731.GI10679@pendragon.ideasonboard.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-7-hdegoede@redhat.com>\n\t<87fryo9nby.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87fryo9nby.fsf@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28626,"web_url":"https://patchwork.libcamera.org/comment/28626/","msgid":"","date":"2024-01-31T11:56:41","subject":"Re: [libcamera-devel] [PATCH v2 06/18] libcamera: introduce\n\tSoftwareIsp class","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Andrey, a few drive-by comments\n\nOn Sat, Jan 13, 2024 at 03:22:06PM +0100, Hans de Goede via libcamera-devel wrote:\n> From: Andrey Konovalov \n>\n> Doxygen documentation by Dennis Bonke.\n>\n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Signed-off-by: Andrey Konovalov \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> ---\n> include/libcamera/internal/meson.build | 1 +\n> include/libcamera/internal/software_isp.h | 231 ++++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/software_isp.cpp | 62 ++++++\n> 4 files changed, 295 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp.h\n> create mode 100644 src/libcamera/software_isp.cpp\n>\n> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\n> index 5807dfd9..1325941d 100644\n> --- a/include/libcamera/internal/meson.build\n> +++ b/include/libcamera/internal/meson.build\n> @@ -40,6 +40,7 @@ libcamera_internal_headers = files([\n> 'pub_key.h',\n> 'request.h',\n> 'shared_mem_object.h',\n> + 'software_isp.h',\n> 'source_paths.h',\n> 'sysfs.h',\n> 'v4l2_device.h',\n> diff --git a/include/libcamera/internal/software_isp.h b/include/libcamera/internal/software_isp.h\n> new file mode 100644\n> index 00000000..42ff48ec\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp.h\n> @@ -0,0 +1,231 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * software_isp.h - Interface for a software implementation of an ISP\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +\n> +namespace libcamera {\n> +\n> +class FrameBuffer;\n> +class PixelFormat;\n> +struct StreamConfiguration;\n> +\n> +LOG_DECLARE_CATEGORY(SoftwareIsp)\n\nDo you need it here ? (this applies to the log.h inclusion)\n\n> +\n> +/**\n> + * \\brief Base class for the Software ISP.\n> + *\n> + * Base class of the SoftwareIsp interface.\n> + */\n> +class SoftwareIsp\n> +{\n> +public:\n> +\t/**\n> +\t * \\brief Constructor for the SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tSoftwareIsp(PipelineHandler *pipe, const ControlInfoMap &sensorControls);\n> +\tvirtual ~SoftwareIsp();\n> +\n> +\t/**\n> +\t * \\brief Load a configuration from a file.\n> +\t * \\param[in] filename The file to load from.\n> +\t *\n> +\t * \\return 0 on success.\n> +\t */\n> +\tvirtual int loadConfiguration(const std::string &filename) = 0;\n> +\n> +\t/**\n> +\t * \\brief Gets if there is a valid debayer object.\n> +\t *\n> +\t * \\returns true if there is, false otherwise.\n> +\t */\n> +\tvirtual bool isValid() const = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the supported output formats.\n> +\t * \\param[in] input The input format.\n> +\t *\n> +\t * \\return all supported output formats or an empty vector if there are none.\n> +\t */\n> +\tvirtual std::vector formats(PixelFormat input) = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the supported output sizes for the given input format and size.\n> +\t * \\param[in] inputFormat The input format.\n> +\t * \\param[in] inputSize The input size.\n> +\t *\n> +\t * \\return The valid size ranges or an empty range if there are none.\n> +\t */\n> +\tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n> +\n> +\t/**\n> +\t * \\brief Get the stride and the frame size.\n> +\t * \\param[in] pixelFormat The output format.\n> +\t * \\param[in] size The output size.\n> +\t *\n> +\t * \\return a tuple of the stride and the frame size, or a tuple with 0,0 if there is no valid output config.\n> +\t */\n> +\tvirtual std::tuple\n> +\tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;\n> +\n> +\t/**\n> +\t * \\brief Configure the SwIspSimple object according to the passed in parameters.\n> +\t * \\param[in] inputCfg The input configuration.\n> +\t * \\param[in] outputCfgs The output configurations.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return 0 on success, a negative errno on failure.\n> +\t */\n> +\tvirtual int configure(const StreamConfiguration &inputCfg,\n> +\t\t\t const std::vector> &outputCfgs,\n> +\t\t\t const ControlInfoMap &sensorControls) = 0;\n\nControlInfoMap should be forward declared ? (as well as ControlList\nbelow, so you could possibily include controls.h)\n\n> +\n> +\t/**\n> +\t * \\brief Exports the buffers for use in processing.\n> +\t * \\param[in] output The number of outputs requested.\n> +\t * \\param[in] count The number of planes.\n> +\t * \\param[out] buffers The exported buffers.\n> +\t *\n> +\t * \\return count when successful, a negative return value if an error occurred.\n> +\t */\n> +\tvirtual int exportBuffers(unsigned int output, unsigned int count,\n> +\t\t\t\t std::vector> *buffers) = 0;\n> +\n> +\t/**\n> +\t * \\brief Starts the Software ISP worker.\n> +\t *\n> +\t * \\return 0 on success, any other value indicates an error.\n> +\t */\n> +\tvirtual int start() = 0;\n> +\n> +\t/**\n> +\t * \\brief Stops the Software ISP worker.\n> +\t */\n> +\tvirtual void stop() = 0;\n> +\n> +\t/**\n> +\t * \\brief Queues buffers for processing.\n> +\t * \\param[in] input The input framebuffer.\n> +\t * \\param[in] outputs The output framebuffers.\n> +\t *\n> +\t * \\return 0 on success, a negative errno on failure\n> +\t */\n> +\tvirtual int queueBuffers(FrameBuffer *input,\n> +\t\t\t\t const std::map &outputs) = 0;\n> +\n> +\t/**\n> +\t * \\brief Process the statistics gathered.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tvirtual void processStats(const ControlList &sensorControls) = 0; // rather merge with queueBuffers()?\n> +\n> +\t/**\n> +\t * \\brief Get the signal for when the sensor controls are set.\n> +\t *\n> +\t * \\return The control list of the sensor controls.\n> +\t */\n> +\tvirtual Signal &getSignalSetSensorControls() = 0;\n> +\n> +\t/**\n> +\t * \\brief Signals that the input buffer is ready.\n> +\t */\n> +\tSignal inputBufferReady;\n> +\t/**\n> +\t * \\brief Signals that the output buffer is ready.\n> +\t */\n> +\tSignal outputBufferReady;\n> +\n> +\t/**\n> +\t * \\brief Signals that the ISP stats are ready.\n> +\t *\n> +\t * The int parameter isn't actually used.\n> +\t */\n> +\tSignal ispStatsReady;\n\nLet's review the interface once seen in ation ;)\n\nSo far it seems it could be realized by extending the existing\nConverter interface, we'll see..\n\n\n> +};\n> +\n> +/**\n> + * \\brief Base class for the Software ISP Factory.\n> + *\n> + * Base class of the SoftwareIsp Factory.\n> + */\n> +class SoftwareIspFactoryBase\n> +{\n> +public:\n> +\tSoftwareIspFactoryBase();\n> +\tvirtual ~SoftwareIspFactoryBase() = default;\n> +\n> +\t/**\n> +\t * \\brief Creates a SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return An unique pointer to the created SoftwareIsp object.\n> +\t */\n> +\tstatic std::unique_ptr create(PipelineHandler *pipe,\n> +\t\t\t\t\t\t const ControlInfoMap &sensorControls);\n> +\t/**\n> +\t * \\brief Gives back a pointer to the factory.\n> +\t *\n> +\t * \\return A static pointer to the factory instance.\n> +\t */\n> +\tstatic SoftwareIspFactoryBase *&factory();\n> +\n> +private:\n> +\tLIBCAMERA_DISABLE_COPY_AND_MOVE(SoftwareIspFactoryBase)\n> +\n> +\tstatic void registerType(SoftwareIspFactoryBase *factory);\n> +\tvirtual std::unique_ptr createInstance(PipelineHandler *pipe,\n> +\t\t\t\t\t\t\t const ControlInfoMap &sensorControls) const = 0;\n> +};\n> +\n> +/**\n> + * \\brief Implementation for the Software ISP Factory.\n> + */\n> +template\n> +class SoftwareIspFactory : public SoftwareIspFactoryBase\n> +{\n> +public:\n> +\tSoftwareIspFactory()\n> +\t\t: SoftwareIspFactoryBase()\n> +\t{\n> +\t}\n> +\n> +\t/**\n> +\t * \\brief Creates an instance of a SoftwareIsp object.\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t *\n> +\t * \\return An unique pointer to the created SoftwareIsp object.\n> +\t */\n> +\tstd::unique_ptr createInstance(PipelineHandler *pipe,\n> +\t\t\t\t\t\t const ControlInfoMap &sensorControls) const override\n> +\t{\n> +\t\treturn std::make_unique<_SoftwareIsp>(pipe, sensorControls);\n> +\t}\n> +};\n> +\n> +#define REGISTER_SOFTWAREISP(softwareIsp) \\\n> +\tstatic SoftwareIspFactory global_##softwareIsp##Factory;\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 3c5e43df..86494663 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -41,6 +41,7 @@ libcamera_sources = files([\n> 'process.cpp',\n> 'pub_key.cpp',\n> 'request.cpp',\n> + 'software_isp.cpp',\n> 'source_paths.cpp',\n> 'stream.cpp',\n> 'sysfs.cpp',\n> diff --git a/src/libcamera/software_isp.cpp b/src/libcamera/software_isp.cpp\n> new file mode 100644\n> index 00000000..2ff97d70\n> --- /dev/null\n> +++ b/src/libcamera/software_isp.cpp\n> @@ -0,0 +1,62 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * software_isp.cpp - Interface for a software implementation of an ISP\n> + */\n> +\n> +#include \"libcamera/internal/software_isp.h\"\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(SoftwareIsp)\n> +\n> +SoftwareIsp::SoftwareIsp([[maybe_unused]] PipelineHandler *pipe,\n> +\t\t\t [[maybe_unused]] const ControlInfoMap &sensorControls)\n> +{\n> +}\n> +\n> +SoftwareIsp::~SoftwareIsp()\n> +{\n> +}\n> +\n> +/* SoftwareIspFactoryBase */\n> +\n> +SoftwareIspFactoryBase::SoftwareIspFactoryBase()\n> +{\n> +\tregisterType(this);\n> +}\n> +\n> +void SoftwareIspFactoryBase::registerType(SoftwareIspFactoryBase *factory)\n> +{\n> +\tSoftwareIspFactoryBase *®istered =\n> +\t\tSoftwareIspFactoryBase::factory();\n> +\n> +\tASSERT(!registered && factory);\n> +\tregistered = factory;\n> +}\n> +\n> +SoftwareIspFactoryBase *&SoftwareIspFactoryBase::factory()\n> +{\n> +\tstatic SoftwareIspFactoryBase *factory;\n> +\treturn factory;\n> +}\n> +\n> +std::unique_ptr\n> +SoftwareIspFactoryBase::create(PipelineHandler *pipe,\n> +\t\t\t const ControlInfoMap &sensorControls)\n> +{\n> +\tSoftwareIspFactoryBase *factory = SoftwareIspFactoryBase::factory();\n> +\tif (!factory)\n> +\t\treturn nullptr;\n> +\n> +\tstd::unique_ptr swIsp = factory->createInstance(pipe, sensorControls);\n> +\tif (swIsp->isValid())\n> +\t\treturn swIsp;\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +} /* namespace libcamera */\n> --\n> 2.43.0\n>","headers":{"Return-Path":"","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 303ECBE173\n\tfor ;\n\tWed, 31 Jan 2024 11:56:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7273362805;\n\tWed, 31 Jan 2024 12:56:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 308EF62800\n\tfor ;\n\tWed, 31 Jan 2024 12:56:45 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4B742B3;\n\tWed, 31 Jan 2024 12:55: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=\"kRGWCA4d\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706702126;\n\tbh=iDrZ/AvkovFzpOzMPmvLPRtLYFxFh+vB4UET4GevSAM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kRGWCA4dwunRJSJ5vWH6pguem6uL1NNDWxOeSFG91c6qqRME/dLcuNX3XsE/MTyMk\n\t9FcCn/bUORP3U2TNFggiS7vu69TrneHYD4R+79wy1mqO9tHQadv7NcjAGyLxDb22NS\n\te/qrbwQF/fnzpnGUeDRduANEvxELtRzGHZ8wS/Cw=","Date":"Wed, 31 Jan 2024 12:56:41 +0100","From":"Jacopo Mondi ","To":"Hans de Goede ","Subject":"Re: [libcamera-devel] [PATCH v2 06/18] libcamera: introduce\n\tSoftwareIsp class","Message-ID":"","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-7-hdegoede@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240113142218.28063-7-hdegoede@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Maxime Ripard , g.martti@gmail.com,\n\tt.langendam@gmail.com, libcamera-devel@lists.libcamera.org,\n\tsrinivas.kandagatla@linaro.org, Pavel Machek ,\n\tBryan O'Donoghue , admin@dennisbonke.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]