[{"id":28457,"web_url":"https://patchwork.libcamera.org/comment/28457/","msgid":"<_bXfGClV8rYY-aIRKu8gahKrAbgat45cwi-rrb1LkvWLLFQ3ETl8ud88nZGgyQFxh9k2yqo0nNAaKmA9-GPeivhhQM_HKozJHQkqYaUI5Pg=@protonmail.com>","date":"2024-01-13T16:30:05","subject":"Re: [libcamera-devel] [PATCH v2 13/18] libcamera: software_isp: add\n\tSimple SoftwareIsp implementation","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. január 13., szombat 15:22 keltezéssel, Hans de Goede via libcamera-devel írta:\n\n> From: Andrey Konovalov \n> \n> The implementation of SoftwareIsp handles creation of Soft IPA\n> and interactions with it, so that the pipeline handler wouldn't\n> need to care about the Soft IPA.\n> \n> Doxygen documentation by Dennis Bonke.\n> \n> Signed-off-by: Andrey Konovalov \n> Co-authored-by: Dennis Bonke \n> Signed-off-by: Dennis Bonke \n> Co-authored-by: Hans de Goede \n> Signed-off-by: Hans de Goede \n> Tested-by: Bryan O'Donoghue # sc8280xp Lenovo x13s\n> Tested-by: Pavel Machek \n> ---\n> .../internal/software_isp/meson.build | 1 +\n> .../internal/software_isp/swisp_simple.h | 163 ++++++++++++\n> src/libcamera/software_isp/meson.build | 19 ++\n> src/libcamera/software_isp/swisp_simple.cpp | 238 ++++++++++++++++++\n> 4 files changed, 421 insertions(+)\n> create mode 100644 include/libcamera/internal/software_isp/swisp_simple.h\n> create mode 100644 src/libcamera/software_isp/swisp_simple.cpp\n> \n> diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build\n> index b5a0d737..cf21ace5 100644\n> --- a/include/libcamera/internal/software_isp/meson.build\n> +++ b/include/libcamera/internal/software_isp/meson.build\n> @@ -4,6 +4,7 @@ libcamera_internal_headers += files([\n> 'debayer.h',\n> 'debayer_cpu.h',\n> 'debayer_params.h',\n> + 'swisp_simple.h',\n> 'swisp_stats.h',\n> 'swstats.h',\n> 'swstats_cpu.h',\n> diff --git a/include/libcamera/internal/software_isp/swisp_simple.h b/include/libcamera/internal/software_isp/swisp_simple.h\n> new file mode 100644\n> index 00000000..87613c23\n> --- /dev/null\n> +++ b/include/libcamera/internal/software_isp/swisp_simple.h\n> @@ -0,0 +1,163 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * swisp_simple.h - Simple software ISP implementation\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/dma_heaps.h\"\n> +#include \"libcamera/internal/software_isp.h\"\n> +#include \"libcamera/internal/software_isp/debayer_cpu.h\"\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\brief Class for the Simple Software ISP.\n> + *\n> + * Implementation of the SoftwareIsp interface.\n> + */\n> +class SwIspSimple : public SoftwareIsp\n> +{\n> +public:\n> +\t/**\n> +\t * \\brief Constructor for the SwIspSimple object.\n> +\t *\n> +\t * \\param[in] pipe The pipeline handler in use.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tSwIspSimple(PipelineHandler *pipe, const ControlInfoMap &sensorControls);\n> +\t~SwIspSimple() {}\n\n~SwIspSimple() = default;\n\n\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> +\tint loadConfiguration([[maybe_unused]] const std::string &filename) { return 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> +\tbool isValid() const;\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> +\tstd::vector formats(PixelFormat input);\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> +\tSizeRange sizes(PixelFormat inputFormat, const Size &inputSize);\n> +\n> +\t/**\n> +\t * \\brief Get the stride and the frame size.\n> +\t * \\param[in] outputFormat 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> +\tstd::tuple\n> +\tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size);\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> +\tint configure(const StreamConfiguration &inputCfg,\n> +\t\t const std::vector> &outputCfgs,\n> +\t\t const ControlInfoMap &sensorControls);\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> +\tint exportBuffers(unsigned int output, unsigned int count,\n> +\t\t\t std::vector> *buffers);\n> +\n> +\t/**\n> +\t * \\brief Process the statistics gathered.\n> +\t * \\param[in] sensorControls The sensor controls.\n> +\t */\n> +\tvoid processStats(const ControlList &sensorControls);\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> +\tint start();\n> +\n> +\t/**\n> +\t * \\brief Stops the Software ISP worker.\n> +\t */\n> +\tvoid stop();\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> +\tint queueBuffers(FrameBuffer *input,\n> +\t\t\t const std::map &outputs);\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> +\tSignal &getSignalSetSensorControls();\n> +\n> +\t/**\n> +\t * \\brief Process the input framebuffer.\n> +\t * \\param[in] input The input framebuffer.\n> +\t * \\param[out] output The output framebuffer.\n> +\t */\n> +\tvoid process(FrameBuffer *input, FrameBuffer *output);\n> +\n> +private:\n> +\tvoid saveIspParams(int dummy);\n> +\tvoid statsReady(int dummy);\n> +\tvoid inputReady(FrameBuffer *input);\n> +\tvoid outputReady(FrameBuffer *output);\n> +\n> +\tstd::unique_ptr debayer_;\n> +\tThread ispWorkerThread_;\n> +\tSharedMemObject sharedParams_;\n> +\tDebayerParams debayerParams_;\n> +\tDmaHeap dmaHeap_;\n> +\n> +\tstd::unique_ptr ipa_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build\n> index 6d7a44d7..9464f2fd 100644\n> --- a/src/libcamera/software_isp/meson.build\n> +++ b/src/libcamera/software_isp/meson.build\n> @@ -6,3 +6,22 @@ libcamera_sources += files([\n> \t'swstats.cpp',\n> \t'swstats_cpu.cpp',\n> ])\n> +\n> +# Software ISP is enabled for 'simple' pipeline handler.\n> +# The 'pipelines' option value should be\n> +# 'simple/' e.g.:\n> +# -Dpipelines=simple/simple\n> +# The source file should be named swisp_.cpp,\n> +# e.g. 'swisp_simple.cpp'.\n> +\n> +foreach pipeline : pipelines\n> + pipeline = pipeline.split('/')\n> + if pipeline.length() == 2 and pipeline[0] == 'simple'\n> + libcamera_sources += files([\n> + 'swisp_' + pipeline[1] + '.cpp',\n> + ])\n> + # the 'break' below can be removed if/when multiple\n> + # Software ISP implementations are allowed in single build\n> + break\n> + endif\n> +endforeach\n> diff --git a/src/libcamera/software_isp/swisp_simple.cpp b/src/libcamera/software_isp/swisp_simple.cpp\n> new file mode 100644\n> index 00000000..0884166e\n> --- /dev/null\n> +++ b/src/libcamera/software_isp/swisp_simple.cpp\n> @@ -0,0 +1,238 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * swisp_simple.cpp - Simple software ISP implementation\n> + */\n> +\n> +#include \"libcamera/internal/software_isp/swisp_simple.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/bayer_format.h\"\n> +#include \"libcamera/internal/framebuffer.h\"\n> +#include \"libcamera/internal/ipa_manager.h\"\n> +#include \"libcamera/internal/mapped_framebuffer.h\"\n> +\n> +namespace libcamera {\n> +\n> +SwIspSimple::SwIspSimple(PipelineHandler *pipe, const ControlInfoMap &sensorControls)\n> +\t: SoftwareIsp(pipe, sensorControls), debayer_(nullptr), debayerParams_{ 256, 256, 256, 0.5f },\n> +\t dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)\n> +{\n> +\tif (!dmaHeap_.isValid()) {\n> +\t\tLOG(SoftwareIsp, Error) << \"Failed to create DmaHeap object\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tsharedParams_ = SharedMemObject(\"softIsp_params\");\n> +\tif (!sharedParams_.fd().isValid()) {\n> +\t\tLOG(SoftwareIsp, Error) << \"Failed to create shared memory for parameters\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tstd::unique_ptr stats;\n> +\n> +\tstats = std::make_unique();\n\nYou can just say\n\n auto stats = std::make_unique();\n\n\n> +\tif (!stats) {\n\nstd::make_unique should never return nullptr.\n\n\n> +\t\tLOG(SoftwareIsp, Error) << \"Failed to create SwStatsCpu object\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tstats->statsReady.connect(this, &SwIspSimple::statsReady);\n> +\n> +\tdebayer_ = std::make_unique(std::move(stats));\n> +\tif (!debayer_) {\n\nSame here.\n\n\n> +\t\tLOG(SoftwareIsp, Error) << \"Failed to create DebayerCpu object\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tdebayer_->inputBufferReady.connect(this, &SwIspSimple::inputReady);\n> +\tdebayer_->outputBufferReady.connect(this, &SwIspSimple::outputReady);\n> +\n> +\tipa_ = IPAManager::createIPA(pipe, 0, 0);\n> +\tif (!ipa_) {\n> +\t\tLOG(SoftwareIsp, Error)\n> +\t\t\t<< \"Creating IPA for software ISP failed\";\n> +\t\tdebayer_.reset();\n> +\t\treturn;\n> +\t}\n> +\n> +\tint ret = ipa_->init(IPASettings{ \"No cfg file\", \"No sensor model\" },\n> +\t\t\t debayer_->getStatsFD(),\n> +\t\t\t sharedParams_.fd(),\n> +\t\t\t sensorControls);\n> +\tif (ret) {\n> +\t\tLOG(SoftwareIsp, Error) << \"IPA init failed\";\n> +\t\tdebayer_.reset();\n> +\t\treturn;\n> +\t}\n> +\n> +\tipa_->setIspParams.connect(this, &SwIspSimple::saveIspParams);\n> +\n> +\tdebayer_->moveToThread(&ispWorkerThread_);\n> +}\n> [...]\n\n\nRegards,\nBarnabás Pőcze","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 ACD43BEFBE\n\tfor ;\n\tSat, 13 Jan 2024 16:30:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D9C2D627E7;\n\tSat, 13 Jan 2024 17:30:13 +0100 (CET)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65DB361D57\n\tfor ;\n\tSat, 13 Jan 2024 17:30:12 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1705163413;\n\tbh=gpg4qM8Mb/jXDUQUjpSkyozvs2B9fUrbucq9JhLBw3A=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=bjGNF1QnqBKnP16KeOH0YG+3uhxm3VAIbdCkVF65Nmmcs+ST90l0M6+BBNpj7/o/i\n\tZbc9Xs0vg7qSZTvjSELV2z6ps7efFaTBon2PtO/rqfyDS6FmuFjaeb12UpxM29Cs0w\n\taojIHi+HTROINaKah3zR0urVwIRtFD2jhbw73MVt/nEBRW22pz2bxYojTsgtDptxPN\n\t/aVN7+dm0g4U29rdvzHPs62l5m0Q7pON5nO4tBnL8ldXP21ot3y5oma1oi3yqpw+ne\n\t+RFQMQGL+QbZr4mNhpF3Or4e3MBb8EDJ1/938VAHC9YeyBDpcD+7X+LIn7EKCIBI5O\n\tLg32ByhAc2eYQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1705163411; x=1705422611;\n\tbh=TXsJP6yLx1VEm+jNlvfWCcgJz4eVXRbmK5jHqDOHibg=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=iENN4fKPABcIyC5oLTlqm3eOfkfBdbv9PcdvqXajoJlZKxYwKnV4jm1Vv2MTkjrsN\n\tFPcTwPpCNxFVDUG0UMMrlJUYBsEIiFk2b5oh5gdSFgxw5Kc6USoXVo1irZBEIPJmQf\n\tQdlvx2cttBQtuCR0ESCaLM9BMO62ku18bcxAyYQh9iuw+NOOdq0KQfAFhEstLjhtWS\n\tkz0OYYmkgwYtOsehRwOdXYILHge/HLtm6NHiri7eNsEpLXwQc+g9U/mcQ88OlPUX1M\n\tCzLPbabTEjjzpSibu7T1cLEtz82EVflrfHOOoD8EJok1jCR/iunWqK1yUeoxWSu3kN\n\tKIeqTRXt7ljNQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"iENN4fKP\"; dkim-atps=neutral","Date":"Sat, 13 Jan 2024 16:30:05 +0000","To":"Hans de Goede ","Message-ID":"<_bXfGClV8rYY-aIRKu8gahKrAbgat45cwi-rrb1LkvWLLFQ3ETl8ud88nZGgyQFxh9k2yqo0nNAaKmA9-GPeivhhQM_HKozJHQkqYaUI5Pg=@protonmail.com>","In-Reply-To":"<20240113142218.28063-14-hdegoede@redhat.com>","References":"<20240113142218.28063-1-hdegoede@redhat.com>\n\t<20240113142218.28063-14-hdegoede@redhat.com>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v2 13/18] libcamera: software_isp: add\n\tSimple SoftwareIsp implementation","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","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","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\" "}}]