[{"id":27915,"web_url":"https://patchwork.libcamera.org/comment/27915/","msgid":"<20230930095934.GA31829@pendragon.ideasonboard.com>","date":"2023-09-30T09:59:34","subject":"Re: [libcamera-devel] [RFC PATCH v3 4/5] libcamera: converter: add\n\tsoftware converter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Andrey,\n\nThank you for the patch. I haven't reviewed everything in details as\nthere are parts that I expect will change, but here are a few comments\nalready.\n\nOn Thu, Sep 28, 2023 at 09:55:36PM +0300, Andrey Konovalov via libcamera-devel wrote:\n> Use the Converter interface to implement bilinear Bayer demosaic\n> filtering and very simple AWB on CPU.\n> \n> The only output format currently supported is RGB888.\n> \n> Signed-off-by: Andrey Konovalov \n> ---\n> .../internal/converter/converter_softw.h | 100 ++++\n> .../libcamera/internal/converter/meson.build | 1 +\n> src/libcamera/converter/converter_softw.cpp | 445 ++++++++++++++++++\n> src/libcamera/converter/meson.build | 3 +-\n> 4 files changed, 548 insertions(+), 1 deletion(-)\n> create mode 100644 include/libcamera/internal/converter/converter_softw.h\n> create mode 100644 src/libcamera/converter/converter_softw.cpp\n> \n> diff --git a/include/libcamera/internal/converter/converter_softw.h b/include/libcamera/internal/converter/converter_softw.h\n> new file mode 100644\n> index 00000000..daa2d6da\n> --- /dev/null\n> +++ b/include/libcamera/internal/converter/converter_softw.h\n> @@ -0,0 +1,100 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * converter_softw.h - interface of software converter (runs 100% on CPU)\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/converter.h\"\n> +\n> +namespace libcamera {\n> +\n> +class FrameBuffer;\n> +class MediaDevice;\n> +class Size;\n> +class SizeRange;\n> +struct StreamConfiguration;\n> +\n> +class SwConverter : public Converter\n> +{\n> +public:\n> +\tSwConverter(MediaDevice *media);\n> +\n> +\tint loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; }\n> +\tbool isValid() const { return isp_ != nullptr; }\n> +\n> +\tstd::vector formats(PixelFormat input);\n> +\tSizeRange sizes(const Size &input);\n> +\n> +\tstd::tuple\n> +\tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> +\n> +\tint configure(const StreamConfiguration &inputCfg,\n> +\t\t const std::vector> &outputCfg);\n> +\tint exportBuffers(unsigned int ouput, unsigned int count,\n> +\t\t\t std::vector> *buffers);\n> +\n> +\tvoid process(FrameBuffer *input, FrameBuffer *output);\n> +\tint start();\n> +\tvoid stop();\n> +\n> +\tint queueBuffers(FrameBuffer *input,\n> +\t\t\t const std::map &outputs);\n> +\n> +private:\n> +\tclass Isp : public Object\n> +\t{\n> +\tpublic:\n> +\t\tIsp(SwConverter *converter);\n> +\t\t~Isp();\n> +\n> +\t\tint configure(const StreamConfiguration &inputCfg,\n> +\t\t\t const StreamConfiguration &outputCfg);\n> +\t\tint exportBuffers(unsigned int count,\n> +\t\t\t\t std::vector> *buffers);\n> +\t\tvoid process(FrameBuffer *input, FrameBuffer *output);\n> +\t\tint start();\n> +\t\tvoid stop();\n> +\t\tvoid waitForIdle();\n> +\n> +\tprivate:\n> +\t\tvoid debayer(uint8_t *dst, const uint8_t *src);\n> +\n> +\t\tSwConverter *converter_;\n> +\n> +\t\tThread thread_;\n\nI think you can simplify thread management by inheriting from the Thread\nclass instead of the Object class. See PostProcessorWorker in the\nAndroid camera HAL implementation for instance, or\nCameraManager::Private. I would then possibly also name this class\nISPWorker.\n\n> +\n> +\t\tlibcamera::Mutex idleMutex_;\n> +\t\tlibcamera::ConditionVariable idleCV_;\n> +\t\tbool idle_ LIBCAMERA_TSA_GUARDED_BY(idleMutex_);\n> +\n> +\t\tunsigned int width_;\n> +\t\tunsigned int height_;\n> +\t\tunsigned int stride_;\n> +\t\tPoint red_shift_;\n\ns/red_shift_/redShift_/\n\n> +\n> +\t\tunsigned long rNumerat_, rDenomin_; /* red gain for AWB */\n> +\t\tunsigned long bNumerat_, bDenomin_; /* blue gain for AWB */\n> +\t\tunsigned long gNumerat_, gDenomin_; /* green gain for AWB */\n> +\t};\n> +\n> +\tstd::unique_ptr isp_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build\n> index 891e79e7..843a0483 100644\n> --- a/include/libcamera/internal/converter/meson.build\n> +++ b/include/libcamera/internal/converter/meson.build\n> @@ -1,5 +1,6 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_internal_headers += files([\n> + 'converter_softw.h',\n> 'converter_v4l2_m2m.h',\n> ])\n> diff --git a/src/libcamera/converter/converter_softw.cpp b/src/libcamera/converter/converter_softw.cpp\n> new file mode 100644\n> index 00000000..67245715\n> --- /dev/null\n> +++ b/src/libcamera/converter/converter_softw.cpp\n> @@ -0,0 +1,445 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Linaro Ltd\n> + *\n> + * converter_softw.h - interface of software converter (runs 100% on CPU)\n> + */\n> +\n> +#include \"libcamera/internal/converter/converter_softw.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/mapped_framebuffer.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(Converter)\n> +\n> +SwConverter::SwConverter([[maybe_unused]] MediaDevice *media)\n> +\t: Converter()\n> +{\n> +\tisp_ = std::make_unique(this);\n> +}\n> +\n> +std::vector SwConverter::formats(PixelFormat input)\n> +{\n> +\tBayerFormat inputFormat = BayerFormat::fromPixelFormat(input);\n> +\n> +\t/* Only RAW10P is currently supported */\n> +\tif (inputFormat.bitDepth != 10 ||\n> +\t inputFormat.packing != BayerFormat::Packing::CSI2) {\n> +\t\tLOG(Converter, Info)\n> +\t\t\t<< \"Unsupported input format \" << input.toString();\n> +\t\treturn {};\n> +\t}\n> +\n> +\treturn { formats::RGB888 };\n> +}\n> +\n> +SizeRange SwConverter::sizes(const Size &input)\n> +{\n> +\tif (input.width < 2 || input.height < 2) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Input format size too small: \" << input.toString();\n> +\t\treturn {};\n> +\t}\n> +\n> +\treturn SizeRange(Size(input.width - 2, input.height - 2));\n> +}\n> +\n> +std::tuple\n> +SwConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> +\t\t\t\tconst Size &size)\n> +{\n> +\t/* Only RGB888 output is currently supported */\n> +\tif (pixelFormat != formats::RGB888)\n> +\t\treturn {};\n> +\n> +\tunsigned int stride = size.width * 3;\n> +\treturn std::make_tuple(stride, stride * (size.height));\n> +}\n> +\n> +int SwConverter::configure(const StreamConfiguration &inputCfg,\n> +\t\t\t const std::vector> &outputCfgs)\n> +{\n> +\tif (outputCfgs.size() != 1) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Unsupported number of output streams: \"\n> +\t\t\t<< outputCfgs.size();\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn isp_->invokeMethod(&SwConverter::Isp::configure,\n> +\t\t\t\t ConnectionTypeBlocking, inputCfg, outputCfgs[0]);\n> +}\n> +\n> +SwConverter::Isp::Isp(SwConverter *converter)\n> +\t: converter_(converter)\n> +{\n> +\tmoveToThread(&thread_);\n\nNot needed if you inherit from Thread.\n\n> +\tthread_.start();\n\nIs there a reason to start the thread here instead of\nSwConverter::start() ? Same question for stop.\n\nIf you start the thread in SwConverter::start(), you won't need to call\ninvokeMethod() in SwConverter::configure().\n\n> +}\n> +\n> +SwConverter::Isp::~Isp()\n> +{\n> +\tthread_.exit();\n> +\tthread_.wait();\n> +}\n> +\n> +int SwConverter::Isp::configure(const StreamConfiguration &inputCfg,\n> +\t\t\t\tconst StreamConfiguration &outputCfg)\n> +{\n> +\tBayerFormat bayerFormat =\n> +\t\tBayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n> +\twidth_ = inputCfg.size.width;\n> +\theight_ = inputCfg.size.height;\n> +\tstride_ = inputCfg.stride;\n> +\n> +\tif (bayerFormat.bitDepth != 10 ||\n> +\t bayerFormat.packing != BayerFormat::Packing::CSI2 ||\n> +\t width_ < 2 || height_ < 2) {\n> +\t\tLOG(Converter, Error) << \"Input format \"\n> +\t\t\t\t << inputCfg.size << \"-\"\n> +\t\t\t\t << inputCfg.pixelFormat\n> +\t\t\t\t << \"not supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tswitch (bayerFormat.order) {\n> +\tcase BayerFormat::BGGR:\n> +\t\tred_shift_ = Point(0, 0);\n> +\t\tbreak;\n> +\tcase BayerFormat::GBRG:\n> +\t\tred_shift_ = Point(1, 0);\n> +\t\tbreak;\n> +\tcase BayerFormat::GRBG:\n> +\t\tred_shift_ = Point(0, 1);\n> +\t\tbreak;\n> +\tcase BayerFormat::RGGB:\n> +\tdefault:\n> +\t\tred_shift_ = Point(1, 1);\n> +\t\tbreak;\n> +\t}\n> +\n> +\tif (outputCfg.size.width != width_ - 2 ||\n> +\t outputCfg.size.height != height_ - 2 ||\n> +\t outputCfg.stride != (width_ - 2) * 3 ||\n> +\t outputCfg.pixelFormat != formats::RGB888) {\n> +\t\tLOG(Converter, Error)\n> +\t\t\t<< \"Output format not supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tLOG(Converter, Info) << \"SwConverter configuration: \"\n> +\t\t\t << inputCfg.size << \"-\" << inputCfg.pixelFormat\n> +\t\t\t << \" -> \"\n> +\t\t\t << outputCfg.size << \"-\" << outputCfg.pixelFormat;\n> +\n> +\t/* set r/g/b gains to 1.0 until frame data collected */\n> +\trNumerat_ = rDenomin_ = 1;\n> +\tbNumerat_ = bDenomin_ = 1;\n> +\tgNumerat_ = gDenomin_ = 1;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int SwConverter::exportBuffers(unsigned int output, unsigned int count,\n> +\t\t\t std::vector> *buffers)\n> +{\n> +\t/* single output for now */\n> +\tif (output >= 1)\n> +\t\treturn -EINVAL;\n> +\n> +\treturn isp_->invokeMethod(&SwConverter::Isp::exportBuffers,\n> +\t\t\t\t ConnectionTypeBlocking, count, buffers);\n> +}\n> +\n> +int SwConverter::Isp::exportBuffers(unsigned int count,\n> +\t\t\t\t std::vector> *buffers)\n> +{\n> +\t/* V4L2_PIX_FMT_BGR24 aka 'BGR3' for output: */\n> +\tunsigned int bufSize = (height_ - 2) * (width_ - 2) * 3;\n> +\n> +\tfor (unsigned int i = 0; i < count; i++) {\n> +\t\tstd::string name = \"frame-\" + std::to_string(i);\n> +\n> +\t\tconst int ispFd = memfd_create(name.c_str(), 0);\n> +\t\tint ret = ftruncate(ispFd, bufSize);\n> +\t\tif (ret < 0) {\n> +\t\t\tLOG(Converter, Error) << \"ftruncate() for memfd failed \"\n> +\t\t\t\t\t << strerror(-ret);\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tFrameBuffer::Plane outPlane;\n> +\t\toutPlane.fd = SharedFD(std::move(ispFd));\n\nFrameBuffer fds need to be dmabufs, not memfds.\n\n> +\t\toutPlane.offset = 0;\n> +\t\toutPlane.length = bufSize;\n> +\n> +\t\tstd::vector planes{ outPlane };\n> +\t\tbuffers->emplace_back(std::make_unique(std::move(planes)));\n> +\t}\n> +\n> +\treturn count;\n> +}\n> +\n> +int SwConverter::Isp::start()\n> +{\n> +\treturn 0;\n> +}\n> +\n> +void SwConverter::Isp::stop()\n> +{\n> +}\n> +\n> +void SwConverter::Isp::waitForIdle()\n> +{\n> +\tMutexLocker locker(idleMutex_);\n> +\n> +\tidleCV_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(idleMutex_) {\n> +\t\t\t return idle_;\n> +\t\t });\n> +}\n> +\n> +int SwConverter::start()\n> +{\n> +\treturn isp_->invokeMethod(&SwConverter::Isp::start,\n> +\t\t\t\t ConnectionTypeBlocking);\n> +}\n> +\n> +void SwConverter::stop()\n> +{\n> +\tisp_->invokeMethod(&SwConverter::Isp::stop,\n> +\t\t\t ConnectionTypeBlocking);\n> +\tisp_->invokeMethod(&SwConverter::Isp::waitForIdle,\n> +\t\t\t ConnectionTypeDirect);\n> +}\n> +\n> +int SwConverter::queueBuffers(FrameBuffer *input,\n> +\t\t\t const std::map &outputs)\n> +{\n> +\tunsigned int mask = 0;\n> +\n> +\t/*\n> +\t * Validate the outputs as a sanity check: at least one output is\n> +\t * required, all outputs must reference a valid stream and no two\n> +\t * outputs can reference the same stream.\n> +\t */\n> +\tif (outputs.empty())\n> +\t\treturn -EINVAL;\n> +\n> +\tfor (auto [index, buffer] : outputs) {\n> +\t\tif (!buffer)\n> +\t\t\treturn -EINVAL;\n> +\t\tif (index >= 1) /* only single stream atm */\n> +\t\t\treturn -EINVAL;\n> +\t\tif (mask & (1 << index))\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tmask |= 1 << index;\n> +\t}\n> +\n> +\tprocess(input, outputs.at(0));\n> +\n> +\treturn 0;\n> +}\n> +\n> +void SwConverter::process(FrameBuffer *input, FrameBuffer *output)\n> +{\n> +\tisp_->invokeMethod(&SwConverter::Isp::process,\n> +\t\t\t ConnectionTypeQueued, input, output);\n> +}\n> +\n> +void SwConverter::Isp::process(FrameBuffer *input, FrameBuffer *output)\n> +{\n> +\t{\n> +\t\tMutexLocker locker(idleMutex_);\n> +\t\tidle_ = false;\n> +\t}\n> +\n> +\t/* Copy metadata from the input buffer */\n> +\tFrameMetadata &metadata = output->_d()->metadata();\n> +\tmetadata.status = input->metadata().status;\n> +\tmetadata.sequence = input->metadata().sequence;\n> +\tmetadata.timestamp = input->metadata().timestamp;\n> +\n> +\tMappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);\n> +\tMappedFrameBuffer out(output, MappedFrameBuffer::MapFlag::Write);\n> +\tif (!in.isValid() || !out.isValid()) {\n> +\t\tLOG(Converter, Error) << \"mmap-ing buffer(s) failed\";\n> +\t\tmetadata.status = FrameMetadata::FrameError;\n> +\t\tconverter_->outputBufferReady.emit(output);\n> +\t\tconverter_->inputBufferReady.emit(input);\n> +\t\treturn;\n> +\t}\n> +\n> +\tdebayer(out.planes()[0].data(), in.planes()[0].data());\n> +\tmetadata.planes()[0].bytesused = out.planes()[0].size();\n> +\n> +\tconverter_->outputBufferReady.emit(output);\n> +\tconverter_->inputBufferReady.emit(input);\n> +\n> +\t{\n> +\t\tMutexLocker locker(idleMutex_);\n> +\t\tidle_ = true;\n> +\t}\n> +\tidleCV_.notify_all();\n> +}\n> +\n> +void SwConverter::Isp::debayer(uint8_t *dst, const uint8_t *src)\n> +{\n> +\t/* RAW10P input format is assumed */\n> +\n> +\t/* output buffer is in BGR24 format and is of (width-2)*(height-2) */\n> +\n> +\tint w_out = width_ - 2;\n> +\tint h_out = height_ - 2;\n> +\n> +\tunsigned long sumR = 0;\n> +\tunsigned long sumB = 0;\n> +\tunsigned long sumG = 0;\n> +\n> +\tfor (int y = 0; y < h_out; y++) {\n> +\t\tconst uint8_t *pin_base = src + (y + 1) * stride_;\n> +\t\tuint8_t *pout = dst + y * w_out * 3;\n> +\t\tint phase_y = (y + red_shift_.y) % 2;\n> +\n> +\t\tfor (int x = 0; x < w_out; x++) {\n> +\t\t\tint phase_x = (x + red_shift_.x) % 2;\n> +\t\t\tint phase = 2 * phase_y + phase_x;\n> +\n> +\t\t\t/* x part of the offset in the input buffer: */\n> +\t\t\tint x_m1 = x + x / 4;\t\t/* offset for (x-1) */\n> +\t\t\tint x_0 = x + 1 + (x + 1) / 4;\t/* offset for x */\n> +\t\t\tint x_p1 = x + 2 + (x + 2) / 4;\t/* offset for (x+1) */\n> +\t\t\t/* the colour component value to write to the output */\n> +\t\t\tunsigned val;\n> +\n> +\t\t\tswitch (phase) {\n> +\t\t\tcase 0: /* at R pixel */\n> +\t\t\t\t/* blue: ((-1,-1)+(1,-1)+(-1,1)+(1,1)) / 4 */\n> +\t\t\t\tval = ( *(pin_base + x_m1 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_p1 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_m1 + stride_)\n> +\t\t\t\t\t+ *(pin_base + x_p1 + stride_) ) >> 2;\n> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* green: ((0,-1)+(-1,0)+(1,0)+(0,1)) / 4 */\n> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_p1)\n> +\t\t\t\t\t+ *(pin_base + x_m1)\n> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 2;\n> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* red: (0,0) */\n> +\t\t\t\tval = *(pin_base + x_0);\n> +\t\t\t\tsumR += val;\n> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\tbreak;\n> +\t\t\tcase 1: /* at Gr pixel */\n> +\t\t\t\t/* blue: ((0,-1) + (0,1)) / 2 */\n> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 1;\n> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* green: (0,0) */\n> +\t\t\t\tval = *(pin_base + x_0);\n> +\t\t\t\tsumG += val;\n> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* red: ((-1,0) + (1,0)) / 2 */\n> +\t\t\t\tval = ( *(pin_base + x_m1)\n> +\t\t\t\t\t+ *(pin_base + x_p1) ) >> 1;\n> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\tbreak;\n> +\t\t\tcase 2: /* at Gb pixel */\n> +\t\t\t\t/* blue: ((-1,0) + (1,0)) / 2 */\n> +\t\t\t\tval = ( *(pin_base + x_m1)\n> +\t\t\t\t\t+ *(pin_base + x_p1) ) >> 1;\n> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* green: (0,0) */\n> +\t\t\t\tval = *(pin_base + x_0);\n> +\t\t\t\tsumG += val;\n> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* red: ((0,-1) + (0,1)) / 2 */\n> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 1;\n> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\tbreak;\n> +\t\t\tdefault: /* at B pixel */\n> +\t\t\t\t/* blue: (0,0) */\n> +\t\t\t\tval = *(pin_base + x_0);\n> +\t\t\t\tsumB += val;\n> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* green: ((0,-1)+(-1,0)+(1,0)+(0,1)) / 4 */\n> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_p1)\n> +\t\t\t\t\t+ *(pin_base + x_m1)\n> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 2;\n> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t\t/* red: ((-1,-1)+(1,-1)+(-1,1)+(1,1)) / 4 */\n> +\t\t\t\tval = ( *(pin_base + x_m1 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_p1 - stride_)\n> +\t\t\t\t\t+ *(pin_base + x_m1 + stride_)\n> +\t\t\t\t\t+ *(pin_base + x_p1 + stride_) ) >> 2;\n> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\t/* calculate red and blue gains for simple AWB */\n> +\tLOG(Converter, Debug) << \"sumR = \" << sumR\n> +\t\t\t << \", sumB = \" << sumB << \", sumG = \" << sumG;\n> +\n> +\tsumG /= 2; /* the number of G pixels is twice as big vs R and B ones */\n> +\n> +\t/* normalize red, blue, and green sums to fit into 22-bit value */\n> +\tunsigned long fRed = sumR / 0x400000;\n> +\tunsigned long fBlue = sumB / 0x400000;\n> +\tunsigned long fGreen = sumG / 0x400000;\n> +\tunsigned long fNorm = std::max({ 1UL, fRed, fBlue, fGreen });\n> +\tsumR /= fNorm;\n> +\tsumB /= fNorm;\n> +\tsumG /= fNorm;\n> +\n> +\tLOG(Converter, Debug) << \"fNorm = \" << fNorm;\n> +\tLOG(Converter, Debug) << \"Normalized: sumR = \" << sumR\n> +\t\t\t << \", sumB= \" << sumB << \", sumG = \" << sumG;\n> +\n> +\t/* make sure red/blue gains never exceed approximately 256 */\n> +\tunsigned long minDenom;\n> +\trNumerat_ = (sumR + sumB + sumG) / 3;\n> +\tminDenom = rNumerat_ / 0x100;\n> +\trDenomin_ = std::max(minDenom, sumR);\n> +\tbNumerat_ = rNumerat_;\n> +\tbDenomin_ = std::max(minDenom, sumB);\n> +\tgNumerat_ = rNumerat_;\n> +\tgDenomin_ = std::max(minDenom, sumG);\n> +\n> +\tLOG(Converter, Debug) << \"rGain = [ \"\n> +\t\t\t << rNumerat_ << \" / \" << rDenomin_\n> +\t\t\t << \" ], bGain = [ \" << bNumerat_ << \" / \" << bDenomin_\n> +\t\t\t << \" ], gGain = [ \" << gNumerat_ << \" / \" << gDenomin_\n> +\t\t\t << \" (minDenom = \" << minDenom << \")\";\n> +}\n> +\n> +static std::initializer_list compatibles = {};\n> +\n> +REGISTER_CONVERTER(\"linaro-sw-converter\", SwConverter, compatibles)\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build\n> index 2aa72fe4..1f1e0ea4 100644\n> --- a/src/libcamera/converter/meson.build\n> +++ b/src/libcamera/converter/meson.build\n> @@ -1,5 +1,6 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_sources += files([\n> - 'converter_v4l2_m2m.cpp'\n> + 'converter_softw.cpp',\n> + 'converter_v4l2_m2m.cpp',\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 061B5BD808\n\tfor ;\n\tSat, 30 Sep 2023 09:59:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7935F62963;\n\tSat, 30 Sep 2023 11:59:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A85C06295E\n\tfor ;\n\tSat, 30 Sep 2023 11:59:23 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(lfbn-idf1-1-343-200.w86-195.abo.wanadoo.fr [86.195.61.200])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 16189DD9;\n\tSat, 30 Sep 2023 11:57:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1696067965;\n\tbh=eurINuqjdIjuZkS1/cHTlkb/VTMeFX3W2XCuIJJAxNs=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GS8tw/a00CC946g37AskkLdLvWnkHAZIMJXsVruQsrTtV5ETXNvwDg4EOjPtYQMlH\n\t6z3o426HnPQoKTuNOOEjYaNf3ra4JItJjD9vciG27kz3ld3Hh31RZt4S0LZssalOva\n\tx54ALWD3ddDLtQDvwg+OewBwwn38kedmWoaaw+KVmJ7PqEqSu+eqB9tv4CfogCUoQf\n\tI5ygETqNzGApWTvWbbhLxKb+95HUffiYFmW8UwktwttySzIcy/C7zHskiqcsp/SrUp\n\thMUZ3mPpeydmeol07QR3dHdwMJztDT6JB6HBw/FC/mawNFPxIj9AfYUvaMFBI2pdIy\n\tmZL5ifXRj4GNg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1696067860;\n\tbh=eurINuqjdIjuZkS1/cHTlkb/VTMeFX3W2XCuIJJAxNs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RuFEMb7zIho28FIQ9vxctEp8e8HvCKBW9eTWCKsXVO4Dr3i61w72hwA59AyrwDX/A\n\tbT8FPjGHILR50X1p1s50KXyG6aAJiqajZ0htmxAKZNd6yzqJhRcml1Hrn0sgKK6res\n\tBNHXT2V231nPQ66+k0rS/VzvxHadO71VeOPTHb9A="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RuFEMb7z\"; dkim-atps=neutral","Date":"Sat, 30 Sep 2023 12:59:34 +0300","To":"Andrey Konovalov ","Message-ID":"<20230930095934.GA31829@pendragon.ideasonboard.com>","References":"<20230928185537.20178-1-andrey.konovalov@linaro.org>\n\t<20230928185537.20178-5-andrey.konovalov@linaro.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230928185537.20178-5-andrey.konovalov@linaro.org>","Subject":"Re: [libcamera-devel] [RFC PATCH v3 4/5] libcamera: converter: add\n\tsoftware converter","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"jacopo.mondi@ideasonboard.com, bryan.odonoghue@linaro.org,\n\tlibcamera-devel@lists.libcamera.org, srinivas.kandagatla@linaro.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27916,"web_url":"https://patchwork.libcamera.org/comment/27916/","msgid":"<290f948c-e007-0dea-64f6-63c5ed014431@linaro.org>","date":"2023-09-30T10:11:09","subject":"Re: [libcamera-devel] [RFC PATCH v3 4/5] libcamera: converter: add\n\tsoftware converter","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi Laurent,\n\nOn 30.09.2023 12:59, Laurent Pinchart wrote:\n> Hi Andrey,\n> \n> Thank you for the patch. I haven't reviewed everything in details as\n> there are parts that I expect will change, but here are a few comments\n> already.\n> \n> On Thu, Sep 28, 2023 at 09:55:36PM +0300, Andrey Konovalov via libcamera-devel wrote:\n>> Use the Converter interface to implement bilinear Bayer demosaic\n>> filtering and very simple AWB on CPU.\n>>\n>> The only output format currently supported is RGB888.\n>>\n>> Signed-off-by: Andrey Konovalov \n>> ---\n>> .../internal/converter/converter_softw.h | 100 ++++\n>> .../libcamera/internal/converter/meson.build | 1 +\n>> src/libcamera/converter/converter_softw.cpp | 445 ++++++++++++++++++\n>> src/libcamera/converter/meson.build | 3 +-\n>> 4 files changed, 548 insertions(+), 1 deletion(-)\n>> create mode 100644 include/libcamera/internal/converter/converter_softw.h\n>> create mode 100644 src/libcamera/converter/converter_softw.cpp\n>>\n>> diff --git a/include/libcamera/internal/converter/converter_softw.h b/include/libcamera/internal/converter/converter_softw.h\n>> new file mode 100644\n>> index 00000000..daa2d6da\n>> --- /dev/null\n>> +++ b/include/libcamera/internal/converter/converter_softw.h\n>> @@ -0,0 +1,100 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2023, Linaro Ltd\n>> + *\n>> + * converter_softw.h - interface of software converter (runs 100% on CPU)\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/converter.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +class FrameBuffer;\n>> +class MediaDevice;\n>> +class Size;\n>> +class SizeRange;\n>> +struct StreamConfiguration;\n>> +\n>> +class SwConverter : public Converter\n>> +{\n>> +public:\n>> +\tSwConverter(MediaDevice *media);\n>> +\n>> +\tint loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; }\n>> +\tbool isValid() const { return isp_ != nullptr; }\n>> +\n>> +\tstd::vector formats(PixelFormat input);\n>> +\tSizeRange sizes(const Size &input);\n>> +\n>> +\tstd::tuple\n>> +\tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n>> +\n>> +\tint configure(const StreamConfiguration &inputCfg,\n>> +\t\t const std::vector> &outputCfg);\n>> +\tint exportBuffers(unsigned int ouput, unsigned int count,\n>> +\t\t\t std::vector> *buffers);\n>> +\n>> +\tvoid process(FrameBuffer *input, FrameBuffer *output);\n>> +\tint start();\n>> +\tvoid stop();\n>> +\n>> +\tint queueBuffers(FrameBuffer *input,\n>> +\t\t\t const std::map &outputs);\n>> +\n>> +private:\n>> +\tclass Isp : public Object\n>> +\t{\n>> +\tpublic:\n>> +\t\tIsp(SwConverter *converter);\n>> +\t\t~Isp();\n>> +\n>> +\t\tint configure(const StreamConfiguration &inputCfg,\n>> +\t\t\t const StreamConfiguration &outputCfg);\n>> +\t\tint exportBuffers(unsigned int count,\n>> +\t\t\t\t std::vector> *buffers);\n>> +\t\tvoid process(FrameBuffer *input, FrameBuffer *output);\n>> +\t\tint start();\n>> +\t\tvoid stop();\n>> +\t\tvoid waitForIdle();\n>> +\n>> +\tprivate:\n>> +\t\tvoid debayer(uint8_t *dst, const uint8_t *src);\n>> +\n>> +\t\tSwConverter *converter_;\n>> +\n>> +\t\tThread thread_;\n> \n> I think you can simplify thread management by inheriting from the Thread\n> class instead of the Object class. See PostProcessorWorker in the\n> Android camera HAL implementation for instance, or\n> CameraManager::Private. I would then possibly also name this class\n> ISPWorker.\n\nYes, the thread management part of this patch needs a clean up.\n\n>> +\n>> +\t\tlibcamera::Mutex idleMutex_;\n>> +\t\tlibcamera::ConditionVariable idleCV_;\n>> +\t\tbool idle_ LIBCAMERA_TSA_GUARDED_BY(idleMutex_);\n>> +\n>> +\t\tunsigned int width_;\n>> +\t\tunsigned int height_;\n>> +\t\tunsigned int stride_;\n>> +\t\tPoint red_shift_;\n> \n> s/red_shift_/redShift_/\n\nOK\n\n>> +\n>> +\t\tunsigned long rNumerat_, rDenomin_; /* red gain for AWB */\n>> +\t\tunsigned long bNumerat_, bDenomin_; /* blue gain for AWB */\n>> +\t\tunsigned long gNumerat_, gDenomin_; /* green gain for AWB */\n>> +\t};\n>> +\n>> +\tstd::unique_ptr isp_;\n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build\n>> index 891e79e7..843a0483 100644\n>> --- a/include/libcamera/internal/converter/meson.build\n>> +++ b/include/libcamera/internal/converter/meson.build\n>> @@ -1,5 +1,6 @@\n>> # SPDX-License-Identifier: CC0-1.0\n>> \n>> libcamera_internal_headers += files([\n>> + 'converter_softw.h',\n>> 'converter_v4l2_m2m.h',\n>> ])\n>> diff --git a/src/libcamera/converter/converter_softw.cpp b/src/libcamera/converter/converter_softw.cpp\n>> new file mode 100644\n>> index 00000000..67245715\n>> --- /dev/null\n>> +++ b/src/libcamera/converter/converter_softw.cpp\n>> @@ -0,0 +1,445 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2023, Linaro Ltd\n>> + *\n>> + * converter_softw.h - interface of software converter (runs 100% on CPU)\n>> + */\n>> +\n>> +#include \"libcamera/internal/converter/converter_softw.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/mapped_framebuffer.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DECLARE_CATEGORY(Converter)\n>> +\n>> +SwConverter::SwConverter([[maybe_unused]] MediaDevice *media)\n>> +\t: Converter()\n>> +{\n>> +\tisp_ = std::make_unique(this);\n>> +}\n>> +\n>> +std::vector SwConverter::formats(PixelFormat input)\n>> +{\n>> +\tBayerFormat inputFormat = BayerFormat::fromPixelFormat(input);\n>> +\n>> +\t/* Only RAW10P is currently supported */\n>> +\tif (inputFormat.bitDepth != 10 ||\n>> +\t inputFormat.packing != BayerFormat::Packing::CSI2) {\n>> +\t\tLOG(Converter, Info)\n>> +\t\t\t<< \"Unsupported input format \" << input.toString();\n>> +\t\treturn {};\n>> +\t}\n>> +\n>> +\treturn { formats::RGB888 };\n>> +}\n>> +\n>> +SizeRange SwConverter::sizes(const Size &input)\n>> +{\n>> +\tif (input.width < 2 || input.height < 2) {\n>> +\t\tLOG(Converter, Error)\n>> +\t\t\t<< \"Input format size too small: \" << input.toString();\n>> +\t\treturn {};\n>> +\t}\n>> +\n>> +\treturn SizeRange(Size(input.width - 2, input.height - 2));\n>> +}\n>> +\n>> +std::tuple\n>> +SwConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n>> +\t\t\t\tconst Size &size)\n>> +{\n>> +\t/* Only RGB888 output is currently supported */\n>> +\tif (pixelFormat != formats::RGB888)\n>> +\t\treturn {};\n>> +\n>> +\tunsigned int stride = size.width * 3;\n>> +\treturn std::make_tuple(stride, stride * (size.height));\n>> +}\n>> +\n>> +int SwConverter::configure(const StreamConfiguration &inputCfg,\n>> +\t\t\t const std::vector> &outputCfgs)\n>> +{\n>> +\tif (outputCfgs.size() != 1) {\n>> +\t\tLOG(Converter, Error)\n>> +\t\t\t<< \"Unsupported number of output streams: \"\n>> +\t\t\t<< outputCfgs.size();\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\treturn isp_->invokeMethod(&SwConverter::Isp::configure,\n>> +\t\t\t\t ConnectionTypeBlocking, inputCfg, outputCfgs[0]);\n>> +}\n>> +\n>> +SwConverter::Isp::Isp(SwConverter *converter)\n>> +\t: converter_(converter)\n>> +{\n>> +\tmoveToThread(&thread_);\n> \n> Not needed if you inherit from Thread.\n> \n>> +\tthread_.start();\n> \n> Is there a reason to start the thread here instead of\n> SwConverter::start() ? Same question for stop.\n\nNo good reason. I jwas just trying different options.\nWhile listening to your talk at Embedded Recipes yesterday I've got to that\nfollowing the IPA module pattern - start/stop the thread in SwConverter::start()/stop() -\nis the more natural way.\n\n> If you start the thread in SwConverter::start(), you won't need to call\n> invokeMethod() in SwConverter::configure().\n> \n>> +}\n>> +\n>> +SwConverter::Isp::~Isp()\n>> +{\n>> +\tthread_.exit();\n>> +\tthread_.wait();\n>> +}\n>> +\n>> +int SwConverter::Isp::configure(const StreamConfiguration &inputCfg,\n>> +\t\t\t\tconst StreamConfiguration &outputCfg)\n>> +{\n>> +\tBayerFormat bayerFormat =\n>> +\t\tBayerFormat::fromPixelFormat(inputCfg.pixelFormat);\n>> +\twidth_ = inputCfg.size.width;\n>> +\theight_ = inputCfg.size.height;\n>> +\tstride_ = inputCfg.stride;\n>> +\n>> +\tif (bayerFormat.bitDepth != 10 ||\n>> +\t bayerFormat.packing != BayerFormat::Packing::CSI2 ||\n>> +\t width_ < 2 || height_ < 2) {\n>> +\t\tLOG(Converter, Error) << \"Input format \"\n>> +\t\t\t\t << inputCfg.size << \"-\"\n>> +\t\t\t\t << inputCfg.pixelFormat\n>> +\t\t\t\t << \"not supported\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tswitch (bayerFormat.order) {\n>> +\tcase BayerFormat::BGGR:\n>> +\t\tred_shift_ = Point(0, 0);\n>> +\t\tbreak;\n>> +\tcase BayerFormat::GBRG:\n>> +\t\tred_shift_ = Point(1, 0);\n>> +\t\tbreak;\n>> +\tcase BayerFormat::GRBG:\n>> +\t\tred_shift_ = Point(0, 1);\n>> +\t\tbreak;\n>> +\tcase BayerFormat::RGGB:\n>> +\tdefault:\n>> +\t\tred_shift_ = Point(1, 1);\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\tif (outputCfg.size.width != width_ - 2 ||\n>> +\t outputCfg.size.height != height_ - 2 ||\n>> +\t outputCfg.stride != (width_ - 2) * 3 ||\n>> +\t outputCfg.pixelFormat != formats::RGB888) {\n>> +\t\tLOG(Converter, Error)\n>> +\t\t\t<< \"Output format not supported\";\n>> +\t\treturn -EINVAL;\n>> +\t}\n>> +\n>> +\tLOG(Converter, Info) << \"SwConverter configuration: \"\n>> +\t\t\t << inputCfg.size << \"-\" << inputCfg.pixelFormat\n>> +\t\t\t << \" -> \"\n>> +\t\t\t << outputCfg.size << \"-\" << outputCfg.pixelFormat;\n>> +\n>> +\t/* set r/g/b gains to 1.0 until frame data collected */\n>> +\trNumerat_ = rDenomin_ = 1;\n>> +\tbNumerat_ = bDenomin_ = 1;\n>> +\tgNumerat_ = gDenomin_ = 1;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +int SwConverter::exportBuffers(unsigned int output, unsigned int count,\n>> +\t\t\t std::vector> *buffers)\n>> +{\n>> +\t/* single output for now */\n>> +\tif (output >= 1)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\treturn isp_->invokeMethod(&SwConverter::Isp::exportBuffers,\n>> +\t\t\t\t ConnectionTypeBlocking, count, buffers);\n>> +}\n>> +\n>> +int SwConverter::Isp::exportBuffers(unsigned int count,\n>> +\t\t\t\t std::vector> *buffers)\n>> +{\n>> +\t/* V4L2_PIX_FMT_BGR24 aka 'BGR3' for output: */\n>> +\tunsigned int bufSize = (height_ - 2) * (width_ - 2) * 3;\n>> +\n>> +\tfor (unsigned int i = 0; i < count; i++) {\n>> +\t\tstd::string name = \"frame-\" + std::to_string(i);\n>> +\n>> +\t\tconst int ispFd = memfd_create(name.c_str(), 0);\n>> +\t\tint ret = ftruncate(ispFd, bufSize);\n>> +\t\tif (ret < 0) {\n>> +\t\t\tLOG(Converter, Error) << \"ftruncate() for memfd failed \"\n>> +\t\t\t\t\t << strerror(-ret);\n>> +\t\t\treturn ret;\n>> +\t\t}\n>> +\n>> +\t\tFrameBuffer::Plane outPlane;\n>> +\t\toutPlane.fd = SharedFD(std::move(ispFd));\n> \n> FrameBuffer fds need to be dmabufs, not memfds.\n\nWill fix that.\n\nThanks,\nAndrey\n\n>> +\t\toutPlane.offset = 0;\n>> +\t\toutPlane.length = bufSize;\n>> +\n>> +\t\tstd::vector planes{ outPlane };\n>> +\t\tbuffers->emplace_back(std::make_unique(std::move(planes)));\n>> +\t}\n>> +\n>> +\treturn count;\n>> +}\n>> +\n>> +int SwConverter::Isp::start()\n>> +{\n>> +\treturn 0;\n>> +}\n>> +\n>> +void SwConverter::Isp::stop()\n>> +{\n>> +}\n>> +\n>> +void SwConverter::Isp::waitForIdle()\n>> +{\n>> +\tMutexLocker locker(idleMutex_);\n>> +\n>> +\tidleCV_.wait(locker, [&]() LIBCAMERA_TSA_REQUIRES(idleMutex_) {\n>> +\t\t\t return idle_;\n>> +\t\t });\n>> +}\n>> +\n>> +int SwConverter::start()\n>> +{\n>> +\treturn isp_->invokeMethod(&SwConverter::Isp::start,\n>> +\t\t\t\t ConnectionTypeBlocking);\n>> +}\n>> +\n>> +void SwConverter::stop()\n>> +{\n>> +\tisp_->invokeMethod(&SwConverter::Isp::stop,\n>> +\t\t\t ConnectionTypeBlocking);\n>> +\tisp_->invokeMethod(&SwConverter::Isp::waitForIdle,\n>> +\t\t\t ConnectionTypeDirect);\n>> +}\n>> +\n>> +int SwConverter::queueBuffers(FrameBuffer *input,\n>> +\t\t\t const std::map &outputs)\n>> +{\n>> +\tunsigned int mask = 0;\n>> +\n>> +\t/*\n>> +\t * Validate the outputs as a sanity check: at least one output is\n>> +\t * required, all outputs must reference a valid stream and no two\n>> +\t * outputs can reference the same stream.\n>> +\t */\n>> +\tif (outputs.empty())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tfor (auto [index, buffer] : outputs) {\n>> +\t\tif (!buffer)\n>> +\t\t\treturn -EINVAL;\n>> +\t\tif (index >= 1) /* only single stream atm */\n>> +\t\t\treturn -EINVAL;\n>> +\t\tif (mask & (1 << index))\n>> +\t\t\treturn -EINVAL;\n>> +\n>> +\t\tmask |= 1 << index;\n>> +\t}\n>> +\n>> +\tprocess(input, outputs.at(0));\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +void SwConverter::process(FrameBuffer *input, FrameBuffer *output)\n>> +{\n>> +\tisp_->invokeMethod(&SwConverter::Isp::process,\n>> +\t\t\t ConnectionTypeQueued, input, output);\n>> +}\n>> +\n>> +void SwConverter::Isp::process(FrameBuffer *input, FrameBuffer *output)\n>> +{\n>> +\t{\n>> +\t\tMutexLocker locker(idleMutex_);\n>> +\t\tidle_ = false;\n>> +\t}\n>> +\n>> +\t/* Copy metadata from the input buffer */\n>> +\tFrameMetadata &metadata = output->_d()->metadata();\n>> +\tmetadata.status = input->metadata().status;\n>> +\tmetadata.sequence = input->metadata().sequence;\n>> +\tmetadata.timestamp = input->metadata().timestamp;\n>> +\n>> +\tMappedFrameBuffer in(input, MappedFrameBuffer::MapFlag::Read);\n>> +\tMappedFrameBuffer out(output, MappedFrameBuffer::MapFlag::Write);\n>> +\tif (!in.isValid() || !out.isValid()) {\n>> +\t\tLOG(Converter, Error) << \"mmap-ing buffer(s) failed\";\n>> +\t\tmetadata.status = FrameMetadata::FrameError;\n>> +\t\tconverter_->outputBufferReady.emit(output);\n>> +\t\tconverter_->inputBufferReady.emit(input);\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\tdebayer(out.planes()[0].data(), in.planes()[0].data());\n>> +\tmetadata.planes()[0].bytesused = out.planes()[0].size();\n>> +\n>> +\tconverter_->outputBufferReady.emit(output);\n>> +\tconverter_->inputBufferReady.emit(input);\n>> +\n>> +\t{\n>> +\t\tMutexLocker locker(idleMutex_);\n>> +\t\tidle_ = true;\n>> +\t}\n>> +\tidleCV_.notify_all();\n>> +}\n>> +\n>> +void SwConverter::Isp::debayer(uint8_t *dst, const uint8_t *src)\n>> +{\n>> +\t/* RAW10P input format is assumed */\n>> +\n>> +\t/* output buffer is in BGR24 format and is of (width-2)*(height-2) */\n>> +\n>> +\tint w_out = width_ - 2;\n>> +\tint h_out = height_ - 2;\n>> +\n>> +\tunsigned long sumR = 0;\n>> +\tunsigned long sumB = 0;\n>> +\tunsigned long sumG = 0;\n>> +\n>> +\tfor (int y = 0; y < h_out; y++) {\n>> +\t\tconst uint8_t *pin_base = src + (y + 1) * stride_;\n>> +\t\tuint8_t *pout = dst + y * w_out * 3;\n>> +\t\tint phase_y = (y + red_shift_.y) % 2;\n>> +\n>> +\t\tfor (int x = 0; x < w_out; x++) {\n>> +\t\t\tint phase_x = (x + red_shift_.x) % 2;\n>> +\t\t\tint phase = 2 * phase_y + phase_x;\n>> +\n>> +\t\t\t/* x part of the offset in the input buffer: */\n>> +\t\t\tint x_m1 = x + x / 4;\t\t/* offset for (x-1) */\n>> +\t\t\tint x_0 = x + 1 + (x + 1) / 4;\t/* offset for x */\n>> +\t\t\tint x_p1 = x + 2 + (x + 2) / 4;\t/* offset for (x+1) */\n>> +\t\t\t/* the colour component value to write to the output */\n>> +\t\t\tunsigned val;\n>> +\n>> +\t\t\tswitch (phase) {\n>> +\t\t\tcase 0: /* at R pixel */\n>> +\t\t\t\t/* blue: ((-1,-1)+(1,-1)+(-1,1)+(1,1)) / 4 */\n>> +\t\t\t\tval = ( *(pin_base + x_m1 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_p1 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_m1 + stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_p1 + stride_) ) >> 2;\n>> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* green: ((0,-1)+(-1,0)+(1,0)+(0,1)) / 4 */\n>> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_p1)\n>> +\t\t\t\t\t+ *(pin_base + x_m1)\n>> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 2;\n>> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* red: (0,0) */\n>> +\t\t\t\tval = *(pin_base + x_0);\n>> +\t\t\t\tsumR += val;\n>> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\tbreak;\n>> +\t\t\tcase 1: /* at Gr pixel */\n>> +\t\t\t\t/* blue: ((0,-1) + (0,1)) / 2 */\n>> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 1;\n>> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* green: (0,0) */\n>> +\t\t\t\tval = *(pin_base + x_0);\n>> +\t\t\t\tsumG += val;\n>> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* red: ((-1,0) + (1,0)) / 2 */\n>> +\t\t\t\tval = ( *(pin_base + x_m1)\n>> +\t\t\t\t\t+ *(pin_base + x_p1) ) >> 1;\n>> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\tbreak;\n>> +\t\t\tcase 2: /* at Gb pixel */\n>> +\t\t\t\t/* blue: ((-1,0) + (1,0)) / 2 */\n>> +\t\t\t\tval = ( *(pin_base + x_m1)\n>> +\t\t\t\t\t+ *(pin_base + x_p1) ) >> 1;\n>> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* green: (0,0) */\n>> +\t\t\t\tval = *(pin_base + x_0);\n>> +\t\t\t\tsumG += val;\n>> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* red: ((0,-1) + (0,1)) / 2 */\n>> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 1;\n>> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\tbreak;\n>> +\t\t\tdefault: /* at B pixel */\n>> +\t\t\t\t/* blue: (0,0) */\n>> +\t\t\t\tval = *(pin_base + x_0);\n>> +\t\t\t\tsumB += val;\n>> +\t\t\t\tval = val * bNumerat_ / bDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* green: ((0,-1)+(-1,0)+(1,0)+(0,1)) / 4 */\n>> +\t\t\t\tval = ( *(pin_base + x_0 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_p1)\n>> +\t\t\t\t\t+ *(pin_base + x_m1)\n>> +\t\t\t\t\t+ *(pin_base + x_0 + stride_) ) >> 2;\n>> +\t\t\t\tval = val * gNumerat_ / gDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t\t/* red: ((-1,-1)+(1,-1)+(-1,1)+(1,1)) / 4 */\n>> +\t\t\t\tval = ( *(pin_base + x_m1 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_p1 - stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_m1 + stride_)\n>> +\t\t\t\t\t+ *(pin_base + x_p1 + stride_) ) >> 2;\n>> +\t\t\t\tval = val * rNumerat_ / rDenomin_;\n>> +\t\t\t\t*pout++ = (uint8_t)std::min(val, 0xffU);\n>> +\t\t\t}\n>> +\t\t}\n>> +\t}\n>> +\n>> +\t/* calculate red and blue gains for simple AWB */\n>> +\tLOG(Converter, Debug) << \"sumR = \" << sumR\n>> +\t\t\t << \", sumB = \" << sumB << \", sumG = \" << sumG;\n>> +\n>> +\tsumG /= 2; /* the number of G pixels is twice as big vs R and B ones */\n>> +\n>> +\t/* normalize red, blue, and green sums to fit into 22-bit value */\n>> +\tunsigned long fRed = sumR / 0x400000;\n>> +\tunsigned long fBlue = sumB / 0x400000;\n>> +\tunsigned long fGreen = sumG / 0x400000;\n>> +\tunsigned long fNorm = std::max({ 1UL, fRed, fBlue, fGreen });\n>> +\tsumR /= fNorm;\n>> +\tsumB /= fNorm;\n>> +\tsumG /= fNorm;\n>> +\n>> +\tLOG(Converter, Debug) << \"fNorm = \" << fNorm;\n>> +\tLOG(Converter, Debug) << \"Normalized: sumR = \" << sumR\n>> +\t\t\t << \", sumB= \" << sumB << \", sumG = \" << sumG;\n>> +\n>> +\t/* make sure red/blue gains never exceed approximately 256 */\n>> +\tunsigned long minDenom;\n>> +\trNumerat_ = (sumR + sumB + sumG) / 3;\n>> +\tminDenom = rNumerat_ / 0x100;\n>> +\trDenomin_ = std::max(minDenom, sumR);\n>> +\tbNumerat_ = rNumerat_;\n>> +\tbDenomin_ = std::max(minDenom, sumB);\n>> +\tgNumerat_ = rNumerat_;\n>> +\tgDenomin_ = std::max(minDenom, sumG);\n>> +\n>> +\tLOG(Converter, Debug) << \"rGain = [ \"\n>> +\t\t\t << rNumerat_ << \" / \" << rDenomin_\n>> +\t\t\t << \" ], bGain = [ \" << bNumerat_ << \" / \" << bDenomin_\n>> +\t\t\t << \" ], gGain = [ \" << gNumerat_ << \" / \" << gDenomin_\n>> +\t\t\t << \" (minDenom = \" << minDenom << \")\";\n>> +}\n>> +\n>> +static std::initializer_list compatibles = {};\n>> +\n>> +REGISTER_CONVERTER(\"linaro-sw-converter\", SwConverter, compatibles)\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build\n>> index 2aa72fe4..1f1e0ea4 100644\n>> --- a/src/libcamera/converter/meson.build\n>> +++ b/src/libcamera/converter/meson.build\n>> @@ -1,5 +1,6 @@\n>> # SPDX-License-Identifier: CC0-1.0\n>> \n>> libcamera_sources += files([\n>> - 'converter_v4l2_m2m.cpp'\n>> + 'converter_softw.cpp',\n>> + 'converter_v4l2_m2m.cpp',\n>> ])\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 21427C0F2A\n\tfor ;\n\tSat, 30 Sep 2023 10:11:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6D63561DE1;\n\tSat, 30 Sep 2023 12:11:13 +0200 (CEST)","from mail-wm1-x330.google.com (mail-wm1-x330.google.com\n\t[IPv6:2a00:1450:4864:20::330])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A244361DE1\n\tfor ;\n\tSat, 30 Sep 2023 12:11:11 +0200 (CEST)","by mail-wm1-x330.google.com with SMTP id\n\t5b1f17b1804b1-4054f790190so140503315e9.2\n\tfor ;\n\tSat, 30 Sep 2023 03:11:11 -0700 (PDT)","from [192.168.118.20] ([87.116.160.132])\n\tby smtp.gmail.com with ESMTPSA id\n\tn16-20020a7bcbd0000000b0040531f5c51asm3098009wmi.5.2023.09.30.03.11.10\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tSat, 30 Sep 2023 03:11:10 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1696068673;\n\tbh=I1LxjKzoCfCGS56BXomSO2qM7veJqQVjmMBCfLRLL4o=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=EVhcBu4vQXMRxHQ5kGNYBElbeRXJK37z8U3N2cOVMWVpu/+38r1qhpde5rohdxrDC\n\tYtR7U6UMmIOyxqnMTzMAXTHuzMp1scNmwGmHiQzKvaH8NufLv0ku1PdYuePxBHiKTE\n\tJKY4zEi7EZkQvULahFKC3iS7tFJv7r3dRpbtSKyjy9UGRvKTfZC8V1kzCYhSRmSfSU\n\tuaYKOAZdGHia0OFZrbEyTi5c3bUyo3nJIUR5Tvyvxx7ne68sL0LnmncN3ZvtMc1zXZ\n\tT9sObANDeA0L7RC5z+FpCVD8DmWcwgnCbyLD7xYCn01/JpsULBoLqqOG9qhEx3F0+Y\n\t2wbZWXaa0fogQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1696068671; x=1696673471;\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=w1qpHv8BvoIr/l2YTSw+zjodY4mJTjiQISE3UidYexw=;\n\tb=E+p6vbRPpdyWYKmo5E+Z84AN27e/ywMcutR6S/TpknPb4GayEtBBtAAIwoWu7g2ph7\n\t6JFzOJrQqcuAYSWXFcrLfO0iDcYMlSpQtMYk/VnnSl51Vkvs1dFkcGsFAIy+6EeVy0+0\n\tmD6xwpfqU2Bzr7jSqH2Dax7D4wYwSqE9JKSkiIqQW5G/ZA0yqMUpDg37bB4CydUK5IDS\n\tzw/7cC1ll/DkQViMrxjBzSCuoWjStTeQdgtMtmuv/r8gWMscz8THHpOkf/ZC0hv2PYeq\n\t3WfGlWw4oNDTnyNYd93eQHLy/fKXMXcgLcGELB/ym89cZM4F1RmkhJ4XxWIP8/1Dohip\n\t8QrA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=linaro.org\n\theader.i=@linaro.org header.b=\"E+p6vbRP\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1696068671; x=1696673471;\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=w1qpHv8BvoIr/l2YTSw+zjodY4mJTjiQISE3UidYexw=;\n\tb=V0HY3M16hHzIHvDJdMVnflcNUACN7fSyJwO/9TZogcSDOvfOVFM8occtSRglM7aVt2\n\t0ub5+32zL5CCROx6OrdK84avUntY/Gnx4PJb/nsWwTQ0/zFnVP73nVGYR4qwRh7OXEm0\n\tICyBRZTuViT/W0ybTMsXPeOZ41e0l0Ifyu5jLDVXYks/bgWS16i3AsuG3ctXYq8T07zg\n\t+WS0Vv3XZuZv+5wKR5ZJZfmU9oPT49ew66MdhVLfFrq78lw4xH886NVZvh/MU+7+mxeL\n\tv4B9b0acviv+dAgCnoSeqHmJPz6TfKSWYZFWm9+MZQRurJ3b7AFv6HhG/5+YteOpTCqv\n\tvaHA==","X-Gm-Message-State":"AOJu0Yxb0Z9qheujosi/0jenBF2MJb0T42qgdldnAKSVNIi1KAHyI8QN\n\t+vkOf4BrzeTCmBuz3rIs5RWA7Q==","X-Google-Smtp-Source":"AGHT+IFnb6hk2/fHGSJb/5cKwYwxeknWTwwsiCHb1kjuoEVSe1kohLIwG4b1GxOZKcjFTIJtzzFxzA==","X-Received":"by 2002:a05:600c:21c5:b0:406:45c1:a657 with SMTP id\n\tx5-20020a05600c21c500b0040645c1a657mr6182105wmj.6.1696068671098; \n\tSat, 30 Sep 2023 03:11:11 -0700 (PDT)","Message-ID":"<290f948c-e007-0dea-64f6-63c5ed014431@linaro.org>","Date":"Sat, 30 Sep 2023 13:11:09 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.15.1","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20230928185537.20178-1-andrey.konovalov@linaro.org>\n\t<20230928185537.20178-5-andrey.konovalov@linaro.org>\n\t<20230930095934.GA31829@pendragon.ideasonboard.com>","In-Reply-To":"<20230930095934.GA31829@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC PATCH v3 4/5] libcamera: converter: add\n\tsoftware converter","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":"Andrey Konovalov via libcamera-devel\n\t","Reply-To":"Andrey Konovalov ","Cc":"jacopo.mondi@ideasonboard.com, bryan.odonoghue@linaro.org,\n\tlibcamera-devel@lists.libcamera.org, srinivas.kandagatla@linaro.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]