[{"id":31119,"web_url":"https://patchwork.libcamera.org/comment/31119/","msgid":"<4yxkdyi2zup2rqbh5y6rhbnonwavwtakbm23ogqyznvqcuwkax@puc5rxcryra7>","date":"2024-09-09T09:06:33","subject":"Re: [PATCH v11 4/7] libcamera: pipeline: Add test pattern for\n\tVirtualPipelineHandler","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi\n\nOn Sat, Sep 07, 2024 at 02:28:29PM GMT, Harvey Yang wrote:\n> From: Konami Shu \n>\n> Add a test pattern generator class hierarchy for the Virtual\n> pipeline handler.\n>\n> Implement two types of test patterns: color bars and diagonal lines\n> generator and use them in the Virtual pipeline handler.\n>\n> Add a dependency for libyuv to the build system to generate images\n> in NV12 format from the test pattern.\n>\n> Signed-off-by: Konami Shu \n> Co-developed-by: Harvey Yang \n> Co-developed-by: Yunke Cao \n> Co-developed-by: Tomasz Figa \n>\n> libcamera: pipeline: Shift test pattern by 1 pixel left every frame\n>\n> - This write the buffer every frame\n> - Shifting makes the frame rate dropped from about 160 to 40\n>\n> Patchset1->2\n> - Use constant instead of using a magic number\n>\n> Patchset2->3\n> - Make shiftLeft() private\n>\n> Signed-off-by: Konami Shu \n> Co-developed-by: Harvey Yang \n> Co-developed-by: Yunke Cao \n> Co-developed-by: Tomasz Figa \n\nThis is not a well-formed commit message. When you squash two patches,\nmake a single commit message out of them\n\n> ---\n> src/android/meson.build | 19 ---\n> .../pipeline/virtual/frame_generator.h | 29 ++++\n> src/libcamera/pipeline/virtual/meson.build | 3 +\n> .../virtual/test_pattern_generator.cpp | 140 ++++++++++++++++++\n> .../pipeline/virtual/test_pattern_generator.h | 57 +++++++\n> src/libcamera/pipeline/virtual/virtual.cpp | 38 ++++-\n> src/libcamera/pipeline/virtual/virtual.h | 7 +\n> src/meson.build | 19 +++\n> 8 files changed, 290 insertions(+), 22 deletions(-)\n> create mode 100644 src/libcamera/pipeline/virtual/frame_generator.h\n> create mode 100644 src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> create mode 100644 src/libcamera/pipeline/virtual/test_pattern_generator.h\n>\n> diff --git a/src/android/meson.build b/src/android/meson.build\n> index 68646120..6341ee8b 100644\n> --- a/src/android/meson.build\n> +++ b/src/android/meson.build\n> @@ -15,25 +15,6 @@ foreach dep : android_deps\n> endif\n> endforeach\n>\n> -libyuv_dep = dependency('libyuv', required : false)\n> -\n> -# Fallback to a subproject if libyuv isn't found, as it's typically not\n> -# provided by distributions.\n> -if not libyuv_dep.found()\n> - cmake = import('cmake')\n> -\n> - libyuv_vars = cmake.subproject_options()\n> - libyuv_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})\n> - libyuv_vars.set_override_option('cpp_std', 'c++17')\n> - libyuv_vars.append_compile_args('cpp',\n> - '-Wno-sign-compare',\n> - '-Wno-unused-variable',\n> - '-Wno-unused-parameter')\n> - libyuv_vars.append_link_args('-ljpeg')\n> - libyuv = cmake.subproject('libyuv', options : libyuv_vars)\n> - libyuv_dep = libyuv.dependency('yuv')\n> -endif\n> -\n\nI thought moving this to the virtual/ directory would have break\nbuilding Android without building the Virtual pipline. Turns out it\ndoesn't, as iirc, all symbols are global in meson, so\n\n> android_deps += [libyuv_dep]\n\nthis is enough to pull-in libyuv as a dependency for android even if\nthe definition is somewhere else!\n\n>\n> android_hal_sources = files([\n> diff --git a/src/libcamera/pipeline/virtual/frame_generator.h b/src/libcamera/pipeline/virtual/frame_generator.h\n> new file mode 100644\n> index 00000000..d8727b8f\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/frame_generator.h\n> @@ -0,0 +1,29 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Google Inc.\n> + *\n> + * frame_generator.h - Virtual cameras helper to generate frames\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class FrameGenerator\n> +{\n> +public:\n> +\tvirtual ~FrameGenerator() = default;\n> +\n> +\tvirtual void configure(const Size &size) = 0;\n> +\n> +\tvirtual void generateFrame(const Size &size,\n> +\t\t\t\t const FrameBuffer *buffer) = 0;\n> +\n> +protected:\n> +\tFrameGenerator() {}\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build\n> index ada1b335..0c537777 100644\n> --- a/src/libcamera/pipeline/virtual/meson.build\n> +++ b/src/libcamera/pipeline/virtual/meson.build\n> @@ -1,5 +1,8 @@\n> # SPDX-License-Identifier: CC0-1.0\n>\n> libcamera_internal_sources += files([\n> + 'test_pattern_generator.cpp',\n> 'virtual.cpp',\n> ])\n> +\n> +libcamera_deps += [libyuv_dep]\n> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> new file mode 100644\n> index 00000000..7094818e\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> @@ -0,0 +1,140 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Google Inc.\n> + *\n> + * test_pattern_generator.cpp - Derived class of FrameGenerator for\n> + * generating test patterns\n> + */\n> +\n> +#include \"test_pattern_generator.h\"\n> +\n> +#include \n> +\n> +#include \"libcamera/internal/mapped_framebuffer.h\"\n> +\n> +#include \"libyuv/convert_from_argb.h\"\n\nWhy use the \"\" include directive variant ?\n\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(Virtual)\n> +\n> +static const unsigned int kARGBSize = 4;\n> +\n> +void TestPatternGenerator::generateFrame(const Size &size,\n> +\t\t\t\t\t const FrameBuffer *buffer)\n> +{\n> +\tMappedFrameBuffer mappedFrameBuffer(buffer,\n> +\t\t\t\t\t MappedFrameBuffer::MapFlag::Write);\n> +\n> +\tauto planes = mappedFrameBuffer.planes();\n> +\n> +\tshiftLeft(size);\n> +\n> +\t/* Convert the template_ to the frame buffer */\n> +\tint ret = libyuv::ARGBToNV12(\n> +\t\ttemplate_.get(), size.width * kARGBSize,\n> +\t\tplanes[0].begin(), size.width,\n> +\t\tplanes[1].begin(), size.width,\n> +\t\tsize.width, size.height);\n\nYou can easily use a more conventional indendation style\n\n\tint ret = libyuv::ARGBToNV12(template_.get(), size.width * kARGBSize,\n\t\t\t\t planes[0].begin(), size.width,\n\t\t\t\t planes[1].begin(), size.width,\n\t\t\t\t size.width, size.height);\n\n> +\tif (ret != 0)\n> +\t\tLOG(Virtual, Error) << \"ARGBToNV12() failed with \" << ret;\n> +}\n> +\n> +void TestPatternGenerator::shiftLeft(const Size &size)\n> +{\n\nThis function basically copies the whole pattern at every frame.\nAs this is a testing pipeline I think it's fine, but it's certainly\nconsuming.\n\n> +\t/* Store the first column temporarily */\n> +\tauto firstColumn = std::make_unique(size.height * kARGBSize);\n> +\tfor (size_t h = 0; h < size.height; h++) {\n> +\t\tunsigned int index = h * size.width * kARGBSize;\n> +\t\tunsigned int index1 = h * kARGBSize;\n> +\t\tfirstColumn[index1] = template_[index];\n> +\t\tfirstColumn[index1 + 1] = template_[index + 1];\n> +\t\tfirstColumn[index1 + 2] = template_[index + 2];\n> +\t\tfirstColumn[index1 + 3] = 0x00;\n> +\t}\n> +\n> +\t/* Overwrite template_ */\n> +\tuint8_t *buf = template_.get();\n> +\tfor (size_t h = 0; h < size.height; h++) {\n> +\t\tfor (size_t w = 0; w < size.width - 1; w++) {\n> +\t\t\t/* Overwrite with the pixel on the right */\n> +\t\t\tunsigned int index = (h * size.width + w + 1) * kARGBSize;\n> +\t\t\t*buf++ = template_[index]; // B\n> +\t\t\t*buf++ = template_[index + 1]; // G\n> +\t\t\t*buf++ = template_[index + 2]; // R\n> +\t\t\t*buf++ = 0x00; // A\n> +\t\t}\n> +\t\t/* Overwrite the new last column with the original first column */\n> +\t\tunsigned int index1 = h * kARGBSize;\n> +\t\t*buf++ = firstColumn[index1]; // B\n> +\t\t*buf++ = firstColumn[index1 + 1]; // G\n> +\t\t*buf++ = firstColumn[index1 + 2]; // R\n> +\t\t*buf++ = 0x00; // A\n> +\t}\n> +}\n> +\n> +ColorBarsGenerator::ColorBarsGenerator() = default;\n\nDo you need this and the below one ?\n\n> +\n> +void ColorBarsGenerator::configure(const Size &size)\n> +{\n> +\tconstexpr uint8_t kColorBar[8][3] = {\n> +\t\t// R, G, B\n\nCould you please make sure, once and for all, that in your next\npatches there won't be // comments ?\n\n> +\t\t{ 0xff, 0xff, 0xff }, // White\n> +\t\t{ 0xff, 0xff, 0x00 }, // Yellow\n> +\t\t{ 0x00, 0xff, 0xff }, // Cyan\n> +\t\t{ 0x00, 0xff, 0x00 }, // Green\n> +\t\t{ 0xff, 0x00, 0xff }, // Magenta\n> +\t\t{ 0xff, 0x00, 0x00 }, // Red\n> +\t\t{ 0x00, 0x00, 0xff }, // Blue\n> +\t\t{ 0x00, 0x00, 0x00 }, // Black\n> +\t};\n> +\n> +\ttemplate_ = std::make_unique(\n> +\t\tsize.width * size.height * kARGBSize);\n> +\n> +\tunsigned int colorBarWidth = size.width / std::size(kColorBar);\n> +\n> +\tuint8_t *buf = template_.get();\n> +\tfor (size_t h = 0; h < size.height; h++) {\n> +\t\tfor (size_t w = 0; w < size.width; w++) {\n> +\t\t\t/* Repeat when the width is exceed */\n> +\t\t\tunsigned int index = (w / colorBarWidth) % std::size(kColorBar);\n> +\n> +\t\t\t*buf++ = kColorBar[index][2]; /* B */\n> +\t\t\t*buf++ = kColorBar[index][1]; /* G */\n> +\t\t\t*buf++ = kColorBar[index][0]; /* R */\n> +\t\t\t*buf++ = 0x00; /* A */\n> +\t\t}\n> +\t}\n> +}\n> +\n> +DiagonalLinesGenerator::DiagonalLinesGenerator() = default;\n> +\n> +void DiagonalLinesGenerator::configure(const Size &size)\n> +{\n> +\tconstexpr uint8_t kColorBar[8][3] = {\n> +\t\t/* R, G, B */\n> +\t\t{ 0xff, 0xff, 0xff }, /* White */\n> +\t\t{ 0x00, 0x00, 0x00 }, /* Black */\n> +\t};\n> +\n> +\ttemplate_ = std::make_unique(\n> +\t\tsize.width * size.height * kARGBSize);\n> +\n> +\tunsigned int lineWidth = size.width / 10;\n> +\n> +\tuint8_t *buf = template_.get();\n> +\tfor (size_t h = 0; h < size.height; h++) {\n> +\t\tfor (size_t w = 0; w < size.width; w++) {\n> +\t\t\t/* Repeat when the width is exceed */\n> +\t\t\tint index = ((w + h) / lineWidth) % 2;\n> +\n> +\t\t\t*buf++ = kColorBar[index][2]; /* B */\n> +\t\t\t*buf++ = kColorBar[index][1]; /* G */\n> +\t\t\t*buf++ = kColorBar[index][0]; /* R */\n> +\t\t\t*buf++ = 0x00; /* A */\n> +\t\t}\n> +\t}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.h b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> new file mode 100644\n> index 00000000..11bcb5f0\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> @@ -0,0 +1,57 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Google Inc.\n> + *\n> + * test_pattern_generator.h - Derived class of FrameGenerator for\n> + * generating test patterns\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"frame_generator.h\"\n> +\n> +namespace libcamera {\n> +\n> +enum class TestPattern : char {\n> +\tColorBars = 0,\n> +\tDiagonalLines = 1,\n> +};\n> +\n> +class TestPatternGenerator : public FrameGenerator\n> +{\n> +public:\n> +\tvoid generateFrame(const Size &size, const FrameBuffer *buffer) override;\n> +\n> +protected:\n> +\t/* Buffer of test pattern template */\n> +\tstd::unique_ptr template_;\n> +\n> +private:\n> +\t/* Shift the buffer by 1 pixel left each frame */\n> +\tvoid shiftLeft(const Size &size);\n> +};\n> +\n> +class ColorBarsGenerator : public TestPatternGenerator\n> +{\n> +public:\n> +\tColorBarsGenerator();\n\nYou can drop this and rely on the default constructor generated by the\ncompiler\n\n> +\n> +\t/* Generate a template buffer of the color bar test pattern. */\n> +\tvoid configure(const Size &size) override;\n> +};\n> +\n> +class DiagonalLinesGenerator : public TestPatternGenerator\n> +{\n> +public:\n> +\tDiagonalLinesGenerator();\n\nditto\n\n> +\n> +\t/* Generate a template buffer of the diagonal lines test pattern. */\n> +\tvoid configure(const Size &size) override;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> index f85ec3dd..6e64aeee 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> @@ -173,8 +173,11 @@ int PipelineHandlerVirtual::configure(Camera *camera,\n> \t\t\t\t CameraConfiguration *config)\n> {\n> \tVirtualCameraData *data = cameraData(camera);\n> -\tfor (size_t i = 0; i < config->size(); ++i)\n> +\tfor (size_t i = 0; i < config->size(); ++i) {\n> \t\tconfig->at(i).setStream(&data->streamConfigs_[i].stream);\n> +\t\tdata->streamConfigs_[i].frameGenerator->configure(\n> +\t\t\tdata->streamConfigs_[i].stream.configuration().size);\n\nIf you use enumerate() as previously suggested you will be able to\naccess the StreamConfiguration more easily\n\n> +\t}\n>\n> \treturn 0;\n> }\n> @@ -210,9 +213,24 @@ void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera)\n> int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,\n> \t\t\t\t\t Request *request)\n> {\n> +\tVirtualCameraData *data = cameraData(camera);\n> +\n> \t/* \\todo Read from the virtual video if any. */\n\nWhat's this \"virtual video\" ?\n\n> -\tfor (auto it : request->buffers())\n> -\t\tcompleteBuffer(request, it.second);\n> +\tfor (auto const &[stream, buffer] : request->buffers()) {\n> +\t\tbool found = false;\n> +\t\t/* map buffer and fill test patterns */\n> +\t\tfor (auto &streamConfig : data->streamConfigs_) {\n\nNow sure how VirtualCameraData::StreamConfig will grow in the next\npatches, but an std::map indexed by libcamera::Stream * migth be\neasier here\n\n> +\t\t\tif (stream == &streamConfig.stream) {\n> +\t\t\t\tfound = true;\n> +\t\t\t\tstreamConfig.frameGenerator->generateFrame(\n> +\t\t\t\t\tstream->configuration().size, buffer);\n> +\t\t\t\tcompleteBuffer(request, buffer);\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t\tif (!found)\n> +\t\t\tLOG(Virtual, Fatal) << \"Stream not defined\";\n\nThis can' happen, unless there's a big issue that should be spotted\nduring the development phase. You can ASSERT() I presume\n\n\n> +\t}\n>\n> \trequest->metadata().set(controls::SensorTimestamp, currentTimestamp());\n> \tcompleteRequest(request);\n> @@ -257,11 +275,25 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n>\n> \tconst std::string id = \"Virtual0\";\n> \tstd::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> +\n> +\tinitFrameGenerator(camera.get());\n> +\n> \tregisterCamera(std::move(camera));\n>\n> \treturn true;\n> }\n>\n> +void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> +{\n> +\tauto data = cameraData(camera);\n> +\tfor (auto &streamConfig : data->streamConfigs_) {\n> +\t\tif (data->testPattern_ == TestPattern::DiagonalLines)\n> +\t\t\tstreamConfig.frameGenerator = std::make_unique();\n> +\t\telse\n> +\t\t\tstreamConfig.frameGenerator = std::make_unique();\n> +\t}\n> +}\n> +\n> REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, \"virtual\")\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h\n> index fb3dbcad..09d73051 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.h\n> +++ b/src/libcamera/pipeline/virtual/virtual.h\n> @@ -16,6 +16,8 @@\n> #include \"libcamera/internal/dma_buf_allocator.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n>\n> +#include \"test_pattern_generator.h\"\n> +\n> namespace libcamera {\n>\n> class VirtualCameraData : public Camera::Private\n> @@ -29,6 +31,7 @@ public:\n> \t};\n> \tstruct StreamConfig {\n> \t\tStream stream;\n> +\t\tstd::unique_ptr frameGenerator;\n> \t};\n>\n> \tVirtualCameraData(PipelineHandler *pipe,\n> @@ -36,6 +39,8 @@ public:\n>\n> \t~VirtualCameraData() = default;\n>\n> +\tTestPattern testPattern_ = TestPattern::ColorBars;\n> +\n> \tconst std::vector supportedResolutions_;\n> \tSize maxResolutionSize_;\n> \tSize minResolutionSize_;\n> @@ -83,6 +88,8 @@ private:\n> \t\treturn static_cast(camera->_d());\n> \t}\n>\n> +\tvoid initFrameGenerator(Camera *camera);\n> +\n> \tDmaBufAllocator dmaBufAllocator_;\n> };\n>\n> diff --git a/src/meson.build b/src/meson.build\n> index 165a77bb..91bea775 100644\n> --- a/src/meson.build\n> +++ b/src/meson.build\n> @@ -27,6 +27,25 @@ else\n> ipa_sign_module = false\n> endif\n>\n> +libyuv_dep = dependency('libyuv', required : false)\n> +\n> +# Fallback to a subproject if libyuv isn't found, as it's typically not\n> +# provided by distributions.\n> +if not libyuv_dep.found()\n> + cmake = import('cmake')\n> +\n> + libyuv_vars = cmake.subproject_options()\n> + libyuv_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})\n> + libyuv_vars.set_override_option('cpp_std', 'c++17')\n> + libyuv_vars.append_compile_args('cpp',\n> + '-Wno-sign-compare',\n> + '-Wno-unused-variable',\n> + '-Wno-unused-parameter')\n> + libyuv_vars.append_link_args('-ljpeg')\n> + libyuv = cmake.subproject('libyuv', options : libyuv_vars)\n> + libyuv_dep = libyuv.dependency('yuv')\n> +endif\n> +\n\nI think it's better to have this in src/meson.build, yes\n\nMostly minors\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n\n> # libcamera must be built first as a dependency to the other components.\n> subdir('libcamera')\n>\n> --\n> 2.46.0.469.g59c65b2a67-goog\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 0AD57BF415\n\tfor ;\n\tMon, 9 Sep 2024 09:06:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 05BB9634F4;\n\tMon, 9 Sep 2024 11:06:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 421B8634E4\n\tfor ;\n\tMon, 9 Sep 2024 11:06:40 +0200 (CEST)","from ideasonboard.com (unknown [213.208.157.109])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 491C56EC;\n\tMon, 9 Sep 2024 11:05:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iYQPs4b7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725872724;\n\tbh=XNH0kZx6iykf1jvf/j8sudq7Qoa93/Lokp9u+d9/AIU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iYQPs4b7dIUmm34qB3f5Wg6xlIEWwlSSadTF3CG/Gap0qhQxWNyFIsd5WwS6z/s6r\n\tjvgMOrHD7Ku2pQSrMQXW/JkhHVlYxeTSygRLcOIE+YN9RHIA4JfvefAGhB7YXOErne\n\t/cjRRdKufxYDXjaRPH2mtjvmXc7dG3ljW5iMUr3U=","Date":"Mon, 9 Sep 2024 11:06:33 +0200","From":"Jacopo Mondi ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org, Konami Shu , \n\tYunke Cao , Tomasz Figa ","Subject":"Re: [PATCH v11 4/7] libcamera: pipeline: Add test pattern for\n\tVirtualPipelineHandler","Message-ID":"<4yxkdyi2zup2rqbh5y6rhbnonwavwtakbm23ogqyznvqcuwkax@puc5rxcryra7>","References":"<20240907143110.2210711-1-chenghaoyang@google.com>\n\t<20240907143110.2210711-5-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240907143110.2210711-5-chenghaoyang@google.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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31149,"web_url":"https://patchwork.libcamera.org/comment/31149/","msgid":"","date":"2024-09-10T04:50:08","subject":"Re: [PATCH v11 4/7] libcamera: pipeline: Add test pattern for\n\tVirtualPipelineHandler","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Jacopo,\n\nOn Mon, Sep 9, 2024 at 5:06 PM Jacopo Mondi \nwrote:\n\n> Hi\n>\n> On Sat, Sep 07, 2024 at 02:28:29PM GMT, Harvey Yang wrote:\n> > From: Konami Shu \n> >\n> > Add a test pattern generator class hierarchy for the Virtual\n> > pipeline handler.\n> >\n> > Implement two types of test patterns: color bars and diagonal lines\n> > generator and use them in the Virtual pipeline handler.\n> >\n> > Add a dependency for libyuv to the build system to generate images\n> > in NV12 format from the test pattern.\n> >\n> > Signed-off-by: Konami Shu \n> > Co-developed-by: Harvey Yang \n> > Co-developed-by: Yunke Cao \n> > Co-developed-by: Tomasz Figa \n> >\n> > libcamera: pipeline: Shift test pattern by 1 pixel left every frame\n> >\n> > - This write the buffer every frame\n> > - Shifting makes the frame rate dropped from about 160 to 40\n> >\n> > Patchset1->2\n> > - Use constant instead of using a magic number\n> >\n> > Patchset2->3\n> > - Make shiftLeft() private\n> >\n> > Signed-off-by: Konami Shu \n> > Co-developed-by: Harvey Yang \n> > Co-developed-by: Yunke Cao \n> > Co-developed-by: Tomasz Figa \n>\n> This is not a well-formed commit message. When you squash two patches,\n> make a single commit message out of them\n>\n\nRight, sorry. Updated.\n\n\n>\n> > ---\n> > src/android/meson.build | 19 ---\n> > .../pipeline/virtual/frame_generator.h | 29 ++++\n> > src/libcamera/pipeline/virtual/meson.build | 3 +\n> > .../virtual/test_pattern_generator.cpp | 140 ++++++++++++++++++\n> > .../pipeline/virtual/test_pattern_generator.h | 57 +++++++\n> > src/libcamera/pipeline/virtual/virtual.cpp | 38 ++++-\n> > src/libcamera/pipeline/virtual/virtual.h | 7 +\n> > src/meson.build | 19 +++\n> > 8 files changed, 290 insertions(+), 22 deletions(-)\n> > create mode 100644 src/libcamera/pipeline/virtual/frame_generator.h\n> > create mode 100644\n> src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> > create mode 100644\n> src/libcamera/pipeline/virtual/test_pattern_generator.h\n> >\n> > diff --git a/src/android/meson.build b/src/android/meson.build\n> > index 68646120..6341ee8b 100644\n> > --- a/src/android/meson.build\n> > +++ b/src/android/meson.build\n> > @@ -15,25 +15,6 @@ foreach dep : android_deps\n> > endif\n> > endforeach\n> >\n> > -libyuv_dep = dependency('libyuv', required : false)\n> > -\n> > -# Fallback to a subproject if libyuv isn't found, as it's typically not\n> > -# provided by distributions.\n> > -if not libyuv_dep.found()\n> > - cmake = import('cmake')\n> > -\n> > - libyuv_vars = cmake.subproject_options()\n> > - libyuv_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE':\n> 'ON'})\n> > - libyuv_vars.set_override_option('cpp_std', 'c++17')\n> > - libyuv_vars.append_compile_args('cpp',\n> > - '-Wno-sign-compare',\n> > - '-Wno-unused-variable',\n> > - '-Wno-unused-parameter')\n> > - libyuv_vars.append_link_args('-ljpeg')\n> > - libyuv = cmake.subproject('libyuv', options : libyuv_vars)\n> > - libyuv_dep = libyuv.dependency('yuv')\n> > -endif\n> > -\n>\n> I thought moving this to the virtual/ directory would have break\n> building Android without building the Virtual pipline. Turns out it\n> doesn't, as iirc, all symbols are global in meson, so\n>\n\nActually I moved it to `src/meson.build`, instead of to the `virtual/`\ndirectory, so it's expected that `src/android/meson.build` finds\nlibyuv_dep again.\n\n\n>\n> > android_deps += [libyuv_dep]\n>\n> this is enough to pull-in libyuv as a dependency for android even if\n> the definition is somewhere else!\n>\n> >\n> > android_hal_sources = files([\n> > diff --git a/src/libcamera/pipeline/virtual/frame_generator.h\n> b/src/libcamera/pipeline/virtual/frame_generator.h\n> > new file mode 100644\n> > index 00000000..d8727b8f\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/virtual/frame_generator.h\n> > @@ -0,0 +1,29 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Google Inc.\n> > + *\n> > + * frame_generator.h - Virtual cameras helper to generate frames\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class FrameGenerator\n> > +{\n> > +public:\n> > + virtual ~FrameGenerator() = default;\n> > +\n> > + virtual void configure(const Size &size) = 0;\n> > +\n> > + virtual void generateFrame(const Size &size,\n> > + const FrameBuffer *buffer) = 0;\n> > +\n> > +protected:\n> > + FrameGenerator() {}\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/virtual/meson.build\n> b/src/libcamera/pipeline/virtual/meson.build\n> > index ada1b335..0c537777 100644\n> > --- a/src/libcamera/pipeline/virtual/meson.build\n> > +++ b/src/libcamera/pipeline/virtual/meson.build\n> > @@ -1,5 +1,8 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> >\n> > libcamera_internal_sources += files([\n> > + 'test_pattern_generator.cpp',\n> > 'virtual.cpp',\n> > ])\n> > +\n> > +libcamera_deps += [libyuv_dep]\n> > diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> > new file mode 100644\n> > index 00000000..7094818e\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp\n> > @@ -0,0 +1,140 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Google Inc.\n> > + *\n> > + * test_pattern_generator.cpp - Derived class of FrameGenerator for\n> > + * generating test patterns\n> > + */\n> > +\n> > +#include \"test_pattern_generator.h\"\n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/mapped_framebuffer.h\"\n> > +\n> > +#include \"libyuv/convert_from_argb.h\"\n>\n> Why use the \"\" include directive variant ?\n>\n\nUpdated to <> instead.\n\n\n>\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(Virtual)\n> > +\n> > +static const unsigned int kARGBSize = 4;\n> > +\n> > +void TestPatternGenerator::generateFrame(const Size &size,\n> > + const FrameBuffer *buffer)\n> > +{\n> > + MappedFrameBuffer mappedFrameBuffer(buffer,\n> > +\n> MappedFrameBuffer::MapFlag::Write);\n> > +\n> > + auto planes = mappedFrameBuffer.planes();\n> > +\n> > + shiftLeft(size);\n> > +\n> > + /* Convert the template_ to the frame buffer */\n> > + int ret = libyuv::ARGBToNV12(\n> > + template_.get(), size.width * kARGBSize,\n> > + planes[0].begin(), size.width,\n> > + planes[1].begin(), size.width,\n> > + size.width, size.height);\n>\n> You can easily use a more conventional indendation style\n>\n> int ret = libyuv::ARGBToNV12(template_.get(), size.width *\n> kARGBSize,\n> planes[0].begin(), size.width,\n> planes[1].begin(), size.width,\n> size.width, size.height);\n>\n>\nDone.\n\n\n> > + if (ret != 0)\n> > + LOG(Virtual, Error) << \"ARGBToNV12() failed with \" << ret;\n> > +}\n> > +\n> > +void TestPatternGenerator::shiftLeft(const Size &size)\n> > +{\n>\n> This function basically copies the whole pattern at every frame.\n> As this is a testing pipeline I think it's fine, but it's certainly\n> consuming.\n>\n\nYeah... We could probably come up with an approach to swap\nonly one column, while libyuv::ARGBToNV12 seems to require\ncontinuous memory for the source, let's leave it as is for now.\n\n\n>\n> > + /* Store the first column temporarily */\n> > + auto firstColumn = std::make_unique(size.height *\n> kARGBSize);\n> > + for (size_t h = 0; h < size.height; h++) {\n> > + unsigned int index = h * size.width * kARGBSize;\n> > + unsigned int index1 = h * kARGBSize;\n> > + firstColumn[index1] = template_[index];\n> > + firstColumn[index1 + 1] = template_[index + 1];\n> > + firstColumn[index1 + 2] = template_[index + 2];\n> > + firstColumn[index1 + 3] = 0x00;\n> > + }\n> > +\n> > + /* Overwrite template_ */\n> > + uint8_t *buf = template_.get();\n> > + for (size_t h = 0; h < size.height; h++) {\n> > + for (size_t w = 0; w < size.width - 1; w++) {\n> > + /* Overwrite with the pixel on the right */\n> > + unsigned int index = (h * size.width + w + 1) *\n> kARGBSize;\n> > + *buf++ = template_[index]; // B\n> > + *buf++ = template_[index + 1]; // G\n> > + *buf++ = template_[index + 2]; // R\n> > + *buf++ = 0x00; // A\n> > + }\n> > + /* Overwrite the new last column with the original first\n> column */\n> > + unsigned int index1 = h * kARGBSize;\n> > + *buf++ = firstColumn[index1]; // B\n> > + *buf++ = firstColumn[index1 + 1]; // G\n> > + *buf++ = firstColumn[index1 + 2]; // R\n> > + *buf++ = 0x00; // A\n> > + }\n> > +}\n> > +\n> > +ColorBarsGenerator::ColorBarsGenerator() = default;\n>\n> Do you need this and the below one ?\n>\n>\nRemoved the declarations and definitions.\n\n\n> > +\n> > +void ColorBarsGenerator::configure(const Size &size)\n> > +{\n> > + constexpr uint8_t kColorBar[8][3] = {\n> > + // R, G, B\n>\n> Could you please make sure, once and for all, that in your next\n> patches there won't be // comments ?\n>\n>\nYeah sorry. Updated.\n\n\n> > + { 0xff, 0xff, 0xff }, // White\n> > + { 0xff, 0xff, 0x00 }, // Yellow\n> > + { 0x00, 0xff, 0xff }, // Cyan\n> > + { 0x00, 0xff, 0x00 }, // Green\n> > + { 0xff, 0x00, 0xff }, // Magenta\n> > + { 0xff, 0x00, 0x00 }, // Red\n> > + { 0x00, 0x00, 0xff }, // Blue\n> > + { 0x00, 0x00, 0x00 }, // Black\n> > + };\n> > +\n> > + template_ = std::make_unique(\n> > + size.width * size.height * kARGBSize);\n> > +\n> > + unsigned int colorBarWidth = size.width / std::size(kColorBar);\n> > +\n> > + uint8_t *buf = template_.get();\n> > + for (size_t h = 0; h < size.height; h++) {\n> > + for (size_t w = 0; w < size.width; w++) {\n> > + /* Repeat when the width is exceed */\n> > + unsigned int index = (w / colorBarWidth) %\n> std::size(kColorBar);\n> > +\n> > + *buf++ = kColorBar[index][2]; /* B */\n> > + *buf++ = kColorBar[index][1]; /* G */\n> > + *buf++ = kColorBar[index][0]; /* R */\n> > + *buf++ = 0x00; /* A */\n> > + }\n> > + }\n> > +}\n> > +\n> > +DiagonalLinesGenerator::DiagonalLinesGenerator() = default;\n> > +\n> > +void DiagonalLinesGenerator::configure(const Size &size)\n> > +{\n> > + constexpr uint8_t kColorBar[8][3] = {\n> > + /* R, G, B */\n> > + { 0xff, 0xff, 0xff }, /* White */\n> > + { 0x00, 0x00, 0x00 }, /* Black */\n> > + };\n> > +\n> > + template_ = std::make_unique(\n> > + size.width * size.height * kARGBSize);\n> > +\n> > + unsigned int lineWidth = size.width / 10;\n> > +\n> > + uint8_t *buf = template_.get();\n> > + for (size_t h = 0; h < size.height; h++) {\n> > + for (size_t w = 0; w < size.width; w++) {\n> > + /* Repeat when the width is exceed */\n> > + int index = ((w + h) / lineWidth) % 2;\n> > +\n> > + *buf++ = kColorBar[index][2]; /* B */\n> > + *buf++ = kColorBar[index][1]; /* G */\n> > + *buf++ = kColorBar[index][0]; /* R */\n> > + *buf++ = 0x00; /* A */\n> > + }\n> > + }\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> > new file mode 100644\n> > index 00000000..11bcb5f0\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.h\n> > @@ -0,0 +1,57 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Google Inc.\n> > + *\n> > + * test_pattern_generator.h - Derived class of FrameGenerator for\n> > + * generating test patterns\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"frame_generator.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +enum class TestPattern : char {\n> > + ColorBars = 0,\n> > + DiagonalLines = 1,\n> > +};\n> > +\n> > +class TestPatternGenerator : public FrameGenerator\n> > +{\n> > +public:\n> > + void generateFrame(const Size &size, const FrameBuffer *buffer)\n> override;\n> > +\n> > +protected:\n> > + /* Buffer of test pattern template */\n> > + std::unique_ptr template_;\n> > +\n> > +private:\n> > + /* Shift the buffer by 1 pixel left each frame */\n> > + void shiftLeft(const Size &size);\n> > +};\n> > +\n> > +class ColorBarsGenerator : public TestPatternGenerator\n> > +{\n> > +public:\n> > + ColorBarsGenerator();\n>\n> You can drop this and rely on the default constructor generated by the\n> compiler\n>\n>\nDone.\n\n\n> > +\n> > + /* Generate a template buffer of the color bar test pattern. */\n> > + void configure(const Size &size) override;\n> > +};\n> > +\n> > +class DiagonalLinesGenerator : public TestPatternGenerator\n> > +{\n> > +public:\n> > + DiagonalLinesGenerator();\n>\n> ditto\n>\n>\nDone.\n\n\n> > +\n> > + /* Generate a template buffer of the diagonal lines test pattern.\n> */\n> > + void configure(const Size &size) override;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp\n> b/src/libcamera/pipeline/virtual/virtual.cpp\n> > index f85ec3dd..6e64aeee 100644\n> > --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> > +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> > @@ -173,8 +173,11 @@ int PipelineHandlerVirtual::configure(Camera\n> *camera,\n> > CameraConfiguration *config)\n> > {\n> > VirtualCameraData *data = cameraData(camera);\n> > - for (size_t i = 0; i < config->size(); ++i)\n> > + for (size_t i = 0; i < config->size(); ++i) {\n> > config->at(i).setStream(&data->streamConfigs_[i].stream);\n> > + data->streamConfigs_[i].frameGenerator->configure(\n> > +\n> data->streamConfigs_[i].stream.configuration().size);\n>\n> If you use enumerate() as previously suggested you will be able to\n> access the StreamConfiguration more easily\n>\n\nDone, and this fixes a bug that\n`data->streamConfigs_[i].stream.configuration()` hasn't been set yet.\nThanks!\n\n\n>\n> > + }\n> >\n> > return 0;\n> > }\n> > @@ -210,9 +213,24 @@ void\n> PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera)\n> > int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera\n> *camera,\n> > Request *request)\n> > {\n> > + VirtualCameraData *data = cameraData(camera);\n> > +\n> > /* \\todo Read from the virtual video if any. */\n>\n> What's this \"virtual video\" ?\n>\n>\nIt means the potential video support. In the next patches we support\nimage loading, while video loading is nice to have.\n\nMaybe the todo comment is redundant?\n\n\n> > - for (auto it : request->buffers())\n> > - completeBuffer(request, it.second);\n> > + for (auto const &[stream, buffer] : request->buffers()) {\n> > + bool found = false;\n> > + /* map buffer and fill test patterns */\n> > + for (auto &streamConfig : data->streamConfigs_) {\n>\n> Now sure how VirtualCameraData::StreamConfig will grow in the next\n> patches, but an std::map indexed by libcamera::Stream * migth be\n> easier here\n>\n>\nHmm, as we assign the streams to StreamConfigurations based on the\norder of the streams (VirtualCameraData::streamConfigs_), would it be\na bit weird to depend on the order of a map? Just curious :p\n\n\n> > + if (stream == &streamConfig.stream) {\n> > + found = true;\n> > + streamConfig.frameGenerator->generateFrame(\n> > + stream->configuration().size,\n> buffer);\n> > + completeBuffer(request, buffer);\n> > + break;\n> > + }\n> > + }\n> > + if (!found)\n> > + LOG(Virtual, Fatal) << \"Stream not defined\";\n>\n> This can' happen, unless there's a big issue that should be spotted\n> during the development phase. You can ASSERT() I presume\n>\n>\nDone.\n\n\n>\n> > + }\n> >\n> > request->metadata().set(controls::SensorTimestamp,\n> currentTimestamp());\n> > completeRequest(request);\n> > @@ -257,11 +275,25 @@ bool\n> PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n> >\n> > const std::string id = \"Virtual0\";\n> > std::shared_ptr camera = Camera::create(std::move(data),\n> id, streams);\n> > +\n> > + initFrameGenerator(camera.get());\n> > +\n> > registerCamera(std::move(camera));\n> >\n> > return true;\n> > }\n> >\n> > +void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> > +{\n> > + auto data = cameraData(camera);\n> > + for (auto &streamConfig : data->streamConfigs_) {\n> > + if (data->testPattern_ == TestPattern::DiagonalLines)\n> > + streamConfig.frameGenerator =\n> std::make_unique();\n> > + else\n> > + streamConfig.frameGenerator =\n> std::make_unique();\n> > + }\n> > +}\n> > +\n> > REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, \"virtual\")\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/virtual/virtual.h\n> b/src/libcamera/pipeline/virtual/virtual.h\n> > index fb3dbcad..09d73051 100644\n> > --- a/src/libcamera/pipeline/virtual/virtual.h\n> > +++ b/src/libcamera/pipeline/virtual/virtual.h\n> > @@ -16,6 +16,8 @@\n> > #include \"libcamera/internal/dma_buf_allocator.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> >\n> > +#include \"test_pattern_generator.h\"\n> > +\n> > namespace libcamera {\n> >\n> > class VirtualCameraData : public Camera::Private\n> > @@ -29,6 +31,7 @@ public:\n> > };\n> > struct StreamConfig {\n> > Stream stream;\n> > + std::unique_ptr frameGenerator;\n> > };\n> >\n> > VirtualCameraData(PipelineHandler *pipe,\n> > @@ -36,6 +39,8 @@ public:\n> >\n> > ~VirtualCameraData() = default;\n> >\n> > + TestPattern testPattern_ = TestPattern::ColorBars;\n> > +\n> > const std::vector supportedResolutions_;\n> > Size maxResolutionSize_;\n> > Size minResolutionSize_;\n> > @@ -83,6 +88,8 @@ private:\n> > return static_cast(camera->_d());\n> > }\n> >\n> > + void initFrameGenerator(Camera *camera);\n> > +\n> > DmaBufAllocator dmaBufAllocator_;\n> > };\n> >\n> > diff --git a/src/meson.build b/src/meson.build\n> > index 165a77bb..91bea775 100644\n> > --- a/src/meson.build\n> > +++ b/src/meson.build\n> > @@ -27,6 +27,25 @@ else\n> > ipa_sign_module = false\n> > endif\n> >\n> > +libyuv_dep = dependency('libyuv', required : false)\n> > +\n> > +# Fallback to a subproject if libyuv isn't found, as it's typically not\n> > +# provided by distributions.\n> > +if not libyuv_dep.found()\n> > + cmake = import('cmake')\n> > +\n> > + libyuv_vars = cmake.subproject_options()\n> > + libyuv_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE':\n> 'ON'})\n> > + libyuv_vars.set_override_option('cpp_std', 'c++17')\n> > + libyuv_vars.append_compile_args('cpp',\n> > + '-Wno-sign-compare',\n> > + '-Wno-unused-variable',\n> > + '-Wno-unused-parameter')\n> > + libyuv_vars.append_link_args('-ljpeg')\n> > + libyuv = cmake.subproject('libyuv', options : libyuv_vars)\n> > + libyuv_dep = libyuv.dependency('yuv')\n> > +endif\n> > +\n>\n> I think it's better to have this in src/meson.build, yes\n>\n> Mostly minors\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n>\n>\n> > # libcamera must be built first as a dependency to the other components.\n> > subdir('libcamera')\n> >\n> > --\n> > 2.46.0.469.g59c65b2a67-goog\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 CC498C324C\n\tfor ;\n\tTue, 10 Sep 2024 04:50:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71370634FB;\n\tTue, 10 Sep 2024 06:50:22 +0200 (CEST)","from mail-lj1-x230.google.com (mail-lj1-x230.google.com\n\t[IPv6:2a00:1450:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA8C5634F7\n\tfor ;\n\tTue, 10 Sep 2024 06:50:20 +0200 (CEST)","by mail-lj1-x230.google.com with SMTP id\n\t38308e7fff4ca-2f761461150so37241101fa.0\n\tfor ;\n\tMon, 09 Sep 2024 21:50:20 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"WHm60IuH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1725943820; x=1726548620;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=Tx7ULp/lJP7AIhHJk1DgQIdWZuszhobUz3F93M/y3dU=;\n\tb=WHm60IuHgrQDPbp5mrTNDBbPIcW7yZNEYPisKjvM0ah7NTxT3xjbEV3aQngv3Wp3B/\n\tI/r26Yc1j3/t/rfM53EEO9FSbUj65x/3+qru2UyZrwO0HOvx0/H+LdkPsJ/dz/BUb7dT\n\ttHiJAhGX20wtNGenA7zKZrGH6lA3qlRUPMF/I=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1725943820; x=1726548620;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=Tx7ULp/lJP7AIhHJk1DgQIdWZuszhobUz3F93M/y3dU=;\n\tb=FDd8Lo9ZMpIg5zDkf+pq6ItFGxIz+4w6EswS2ktRXI1RDFUwdAhBk8C0K+iy2joXzi\n\tIxNJEiwAtrIix3TA6Z8KJX1R9tW/xE9yrYaupSz4bKbep39pMilUZU20NDozhnh85vfR\n\tgFnLQAdlq9c2rNqdSCwKnmnDHOFAEX4EZcB0toL2MLa1qW1Vu+Ia5T2ahl8iatPYT9XN\n\tu0UROTk7Bww3Ucz9wePx6XaIWh4DmESmsrlNbabxHFRXd8rgI65bhvSNU6UAbxe56Wl8\n\tkARgpti3OtviCeN/xyNmlZrUOW0hu0Uw3yRv0lPzTVtP7EpZF9pkItBjk3oFf7S/arob\n\td4Xw==","X-Gm-Message-State":"AOJu0Yx4x+o9tmjZLkfHvPyRMCPb3+fnzLgMWfSP3350nJOeBv/QhN50\n\thaCEAPmOaI+4wsk3BGx3Rin2JwqZdqbmI3fywIN0UL67QF5pCbbvhvXGPY/GSiFQbLJ7DYr4tuX\n\tMuPxySefyoRt82Bwp8EtRJjgCpA7R8b5fq8/m","X-Google-Smtp-Source":"AGHT+IG8ZJCR5lkFcDeTDJRyuNmml7I0T/ky0LC6IBi5Mgtkjt9hmTlIZEAMf8Xz+af6/KDpg1gApy9BhqlZKVpD6+s=","X-Received":"by 2002:a2e:bc1a:0:b0:2f7:5907:97a6 with SMTP id\n\t38308e7fff4ca-2f75a968c4dmr98657471fa.1.1725943819838;\n\tMon, 09 Sep 2024 21:50:19 -0700 (PDT)","MIME-Version":"1.0","References":"<20240907143110.2210711-1-chenghaoyang@google.com>\n\t<20240907143110.2210711-5-chenghaoyang@google.com>\n\t<4yxkdyi2zup2rqbh5y6rhbnonwavwtakbm23ogqyznvqcuwkax@puc5rxcryra7>","In-Reply-To":"<4yxkdyi2zup2rqbh5y6rhbnonwavwtakbm23ogqyznvqcuwkax@puc5rxcryra7>","From":"Cheng-Hao Yang ","Date":"Tue, 10 Sep 2024 12:50:08 +0800","Message-ID":"","Subject":"Re: [PATCH v11 4/7] libcamera: pipeline: Add test pattern for\n\tVirtualPipelineHandler","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, Konami Shu , \n\tYunke Cao , Tomasz Figa ","Content-Type":"multipart/alternative; boundary=\"00000000000048b8d60621bc9cf6\"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]