<div dir="ltr"><div dir="ltr">Thanks Jacopo,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 27, 2024 at 6:35 PM Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com" target="_blank">jacopo.mondi@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Harvey, Konami<br>
<br>
On Tue, Aug 20, 2024 at 04:23:35PM GMT, Harvey Yang wrote:<br>
> From: Konami Shu <<a href="mailto:konamiz@google.com" target="_blank">konamiz@google.com</a>><br>
><br>
> - There are two test patterns: color bars and diagonal lines<br>
> - Add class for generating test patterns<br>
> - Add libyuv to build dependencies<br>
> - Make VirtualPipelineHandler show the test pattern<br>
> - Format the code<br>
<br>
Could you make a commit message out of this please ? Like:<br>
<br>
Add a test pattern generator class hierarchy for the Virtual<br>
pipeline handler.<br>
<br>
Implement two types of test patterns: color bars and diagonal lines<br>
generator and use them in the Virtual pipeline handler.<br>
<br>
Add a dependency for libyuv to the build system to generate images<br>
in NV12 format from the test pattern.<br>
<br></blockquote><div>Adopted, thanks!</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
><br>
> - Rename test_pattern to frame_generator<br>
> - reflect comment<br>
> - Fix const variable name<br>
> - Use #pragma once<br>
> - Make configure() private<br>
<br>
This list looks like a changelog more than a commit message. Could you<br>
drop it or place it below<br></blockquote><div>Removed.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
---<br>
<br>
So that it doesn't appear in the git history ?<br>
<br>
><br>
> Signed-off-by: Konami Shu <<a href="mailto:konamiz@google.com" target="_blank">konamiz@google.com</a>><br>
> Co-developed-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> Co-developed-by: Yunke Cao <<a href="mailto:yunkec@chromium.org" target="_blank">yunkec@chromium.org</a>><br>
> Co-developed-by: Tomasz Figa <<a href="mailto:tfiga@chromium.org" target="_blank">tfiga@chromium.org</a>><br>
> ---<br>
> .../pipeline/virtual/frame_generator.h | 33 ++++++<br>
> src/libcamera/pipeline/virtual/meson.build | 22 ++++<br>
> .../virtual/test_pattern_generator.cpp | 112 ++++++++++++++++++<br>
> .../pipeline/virtual/test_pattern_generator.h | 58 +++++++++<br>
> src/libcamera/pipeline/virtual/virtual.cpp | 28 ++++-<br>
> src/libcamera/pipeline/virtual/virtual.h | 8 ++<br>
> 6 files changed, 258 insertions(+), 3 deletions(-)<br>
> create mode 100644 src/libcamera/pipeline/virtual/frame_generator.h<br>
> create mode 100644 src/libcamera/pipeline/virtual/test_pattern_generator.cpp<br>
> create mode 100644 src/libcamera/pipeline/virtual/test_pattern_generator.h<br>
><br>
> diff --git a/src/libcamera/pipeline/virtual/frame_generator.h b/src/libcamera/pipeline/virtual/frame_generator.h<br>
> new file mode 100644<br>
> index 000000000..9699af7a4<br>
> --- /dev/null<br>
> +++ b/src/libcamera/pipeline/virtual/frame_generator.h<br>
> @@ -0,0 +1,33 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2023, Google Inc.<br>
> + *<br>
> + * frame_generator.h - Virtual cameras helper to generate frames<br>
> + */<br>
> +<br>
> +#pragma once<br>
> +<br>
> +#include <libcamera/framebuffer.h><br>
> +#include <libcamera/geometry.h><br>
> +<br>
> +namespace libcamera {<br>
> +<br>
> +class FrameGenerator<br>
> +{<br>
> +public:<br>
> + virtual ~FrameGenerator() = default;<br>
> +<br>
> + /* Create buffers for using them in `generateFrame` */<br>
<br>
Usually we don't comment headers, but I don't mind<br>
<br></blockquote><div>Right, I think Konami put it here because there's no .cpp file for this</div><div>virtual class. Removed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + virtual void configure(const Size &size) = 0;<br>
> +<br>
> + /** Fill the output frame buffer.<br>
> + * Use the frame at the frameCount of image frames<br>
> + */<br>
<br>
As long as the right commenting style is used: this is not Doxygen (no<br>
/**) and I don't see references to frameCount anywhere ?<br></blockquote><div>Removed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + virtual void generateFrame(const Size &size,<br>
> + const FrameBuffer *buffer) = 0;<br>
> +<br>
> +protected:<br>
> + FrameGenerator() {}<br>
> +};<br>
> +<br>
> +} /* namespace libcamera */<br>
> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build<br>
> index ba7ff754e..e1e65e68d 100644<br>
> --- a/src/libcamera/pipeline/virtual/meson.build<br>
> +++ b/src/libcamera/pipeline/virtual/meson.build<br>
> @@ -2,4 +2,26 @@<br>
><br>
> libcamera_sources += files([<br>
> 'virtual.cpp',<br>
> + 'test_pattern_generator.cpp',<br>
> ])<br>
> +<br>
> +libyuv_dep = dependency('libyuv', required : false)<br>
> +<br>
> +# Fallback to a subproject if libyuv isn't found, as it's typically not<br>
> +# provided by distributions.<br>
> +if not libyuv_dep.found()<br>
<br>
The Android HAL has exactly the same snippet. I wonder if it should be<br>
moved to the main libcamera build file and only add libyuv as a<br>
dependency in the HAL and here. It might require a bit of<br>
experimentation<br></blockquote><div>Moved to `src/meson.build`. Please check.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + cmake = import('cmake')<br>
> +<br>
> + libyuv_vars = cmake.subproject_options()<br>
> + libyuv_vars.add_cmake_defines({'CMAKE_POSITION_INDEPENDENT_CODE': 'ON'})<br>
> + libyuv_vars.set_override_option('cpp_std', 'c++17')<br>
> + libyuv_vars.append_compile_args('cpp',<br>
> + '-Wno-sign-compare',<br>
> + '-Wno-unused-variable',<br>
> + '-Wno-unused-parameter')<br>
> + libyuv_vars.append_link_args('-ljpeg')<br>
<br>
Do you need jpeg support ?<br></blockquote><div>Well, in the 7th patch, yes. Also, as we try to use the same</div><div>dependency with Android adapter, how about keeping jpeg</div><div>here?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + libyuv = cmake.subproject('libyuv', options : libyuv_vars)<br>
> + libyuv_dep = libyuv.dependency('yuv')<br>
> +endif<br>
> +<br>
> +libcamera_deps += [libyuv_dep]<br>
> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.cpp b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp<br>
> new file mode 100644<br>
> index 000000000..8dfe626e5<br>
> --- /dev/null<br>
> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.cpp<br>
> @@ -0,0 +1,112 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2023, Google Inc.<br>
> + *<br>
> + * test_pattern_generator.cpp - Derived class of FrameGenerator for<br>
> + * generating test patterns<br>
> + */<br>
> +<br>
> +#include "test_pattern_generator.h"<br>
> +<br>
> +#include <libcamera/base/log.h><br>
> +<br>
> +#include "libcamera/internal/mapped_framebuffer.h"<br>
> +<br>
<br>
Empty line please<br></blockquote><div>Done</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +#include "libyuv/convert_from_argb.h"<br>
> +namespace libcamera {<br>
> +<br>
> +LOG_DECLARE_CATEGORY(Virtual)<br>
> +<br>
> +static const unsigned int kARGBSize = 4;<br>
> +<br>
> +void TestPatternGenerator::generateFrame(<br>
> + const Size &size,<br>
> + const FrameBuffer *buffer)<br>
<br>
Weird indent<br>
<br>
void TestPatternGenerator::generateFrame(const Size &size,<br>
const FrameBuffer *buffer)<br>
<br>
<br></blockquote><div>Adopted.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> +{<br>
> + MappedFrameBuffer mappedFrameBuffer(buffer,<br>
> + MappedFrameBuffer::MapFlag::Write);<br>
> +<br>
> + auto planes = mappedFrameBuffer.planes();<br>
> +<br>
> + /* Convert the template_ to the frame buffer */<br>
> + int ret = libyuv::ARGBToNV12(<br>
<br>
As this produces NV12, the pipeline handler should only accept NV12<br>
and adjust all other pixelformats. If I'm not mistaken this is not<br>
enforced in validate() and you return SRGGB10 for Role::Raw (which you<br>
shouldn't support afaict). This can cause issues as the buffers<br>
allocated might be of a different size than the ones you expect</blockquote><div><br></div><div>Updated in the 3rd patch to remove the raw stream and ensure formats::NV12</div><div>for each stream. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> ?<br>
<br>
I agree that generating patterns in ARGB is easier than NV12, but I<br>
wonder why not use RGB888 as the pipeline output format directly so<br>
that you don't have to depend on libyuv at all.<br></blockquote><div><br></div><div>I think it's because the Android adapter requires mostly NV12 [1]</div><div><br></div><div>[1]: <a href="https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/upstream/src/android/camera_capabilities.cpp;l=68" target="_blank">https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/upstream/src/android/camera_capabilities.cpp;l=68</a></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
> + template_.get(), /*src_stride_argb=*/size.width * kARGBSize,<br>
<br>
Why a comment in the middle of the code ?<br></blockquote><div>Removed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + planes[0].begin(), size.width,<br>
> + planes[1].begin(), size.width,<br>
> + size.width, size.height);<br>
> + if (ret != 0) {<br>
> + LOG(Virtual, Error) << "ARGBToNV12() failed with " << ret;<br>
> + }<br>
<br>
No {} for single line statements<br></blockquote><div>Done</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +}<br>
> +<br>
> +std::unique_ptr<ColorBarsGenerator> ColorBarsGenerator::create()<br>
> +{<br>
> + return std::make_unique<ColorBarsGenerator>();<br>
> +}<br>
> +<br>
> +void ColorBarsGenerator::configure(const Size &size)<br>
> +{<br>
> + constexpr uint8_t kColorBar[8][3] = {<br>
> + // R, G, B<br>
> + { 0xff, 0xff, 0xff }, // White<br>
> + { 0xff, 0xff, 0x00 }, // Yellow<br>
> + { 0x00, 0xff, 0xff }, // Cyan<br>
> + { 0x00, 0xff, 0x00 }, // Green<br>
> + { 0xff, 0x00, 0xff }, // Magenta<br>
> + { 0xff, 0x00, 0x00 }, // Red<br>
> + { 0x00, 0x00, 0xff }, // Blue<br>
> + { 0x00, 0x00, 0x00 }, // Black<br>
> + };<br>
> +<br>
> + template_ = std::make_unique<uint8_t[]>(<br>
> + size.width * size.height * kARGBSize);<br>
> +<br>
> + unsigned int colorBarWidth = size.width / std::size(kColorBar);<br>
> +<br>
> + uint8_t *buf = template_.get();<br>
> + for (size_t h = 0; h < size.height; h++) {<br>
> + for (size_t w = 0; w < size.width; w++) {<br>
> + // repeat when the width is exceed<br>
<br>
libcamera doesn't use C++ style comments<br></blockquote><div>Updated.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + int index = (w / colorBarWidth) % std::size(kColorBar);<br>
<br>
unsigned int<br></blockquote><div>Done</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> + *buf++ = kColorBar[index][2]; // B<br>
> + *buf++ = kColorBar[index][1]; // G<br>
> + *buf++ = kColorBar[index][0]; // R<br>
> + *buf++ = 0x00; // A<br>
> + }<br>
> + }<br>
> +}<br>
> +<br>
> +std::unique_ptr<DiagonalLinesGenerator> DiagonalLinesGenerator::create()<br>
> +{<br>
> + return std::make_unique<DiagonalLinesGenerator>();<br>
> +}<br>
> +<br>
> +void DiagonalLinesGenerator::configure(const Size &size)<br>
> +{<br>
> + constexpr uint8_t kColorBar[8][3] = {<br>
> + // R, G, B<br>
> + { 0xff, 0xff, 0xff }, // White<br>
> + { 0x00, 0x00, 0x00 }, // Black<br>
> + };<br>
> +<br>
> + template_ = std::make_unique<uint8_t[]>(<br>
> + size.width * size.height * kARGBSize);<br>
> +<br>
> + unsigned int lineWidth = size.width / 10;<br>
> +<br>
> + uint8_t *buf = template_.get();<br>
> + for (size_t h = 0; h < size.height; h++) {<br>
> + for (size_t w = 0; w < size.width; w++) {<br>
> + // repeat when the width is exceed<br>
> + int index = ((w + h) / lineWidth) % 2;<br>
<br>
same comments<br></blockquote><div>Done</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> + *buf++ = kColorBar[index][2]; // B<br>
> + *buf++ = kColorBar[index][1]; // G<br>
> + *buf++ = kColorBar[index][0]; // R<br>
> + *buf++ = 0x00; // A<br>
> + }<br>
> + }<br>
> +}<br>
> +<br>
> +} /* namespace libcamera */<br>
> diff --git a/src/libcamera/pipeline/virtual/test_pattern_generator.h b/src/libcamera/pipeline/virtual/test_pattern_generator.h<br>
> new file mode 100644<br>
> index 000000000..ed8d4e43b<br>
> --- /dev/null<br>
> +++ b/src/libcamera/pipeline/virtual/test_pattern_generator.h<br>
> @@ -0,0 +1,58 @@<br>
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> +/*<br>
> + * Copyright (C) 2023, Google Inc.<br>
> + *<br>
> + * test_pattern_generator.h - Derived class of FrameGenerator for<br>
> + * generating test patterns<br>
> + */<br>
> +<br>
> +#pragma once<br>
> +<br>
> +#include <memory><br>
> +<br>
> +#include <libcamera/framebuffer.h><br>
> +#include <libcamera/geometry.h><br>
> +<br>
> +#include "frame_generator.h"<br>
> +<br>
> +namespace libcamera {<br>
> +<br>
> +enum class TestPattern : char {<br>
> + ColorBars = 0,<br>
> + DiagonalLines = 1,<br>
> +};<br>
> +<br>
> +class TestPatternGenerator : public FrameGenerator<br>
> +{<br>
> +private:<br>
> + void generateFrame(const Size &size,<br>
> + const FrameBuffer *buffer) override;<br>
<br>
<br>
Maybe I don't get this, but this overrides a public function, and it's<br>
called from the pipeline, why private ?<br></blockquote><div>Hmm, it should be public. Updated.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +protected:<br>
> + /* Shift the buffer by 1 pixel left each frame */<br>
> + void shiftLeft(const Size &size);<br>
<br>
Not implemented afaict<br></blockquote><div>Removed</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + /* Buffer of test pattern template */<br>
> + std::unique_ptr<uint8_t[]> template_;<br>
> +};<br>
> +<br>
> +class ColorBarsGenerator : public TestPatternGenerator<br>
> +{<br>
> +public:<br>
> + static std::unique_ptr<ColorBarsGenerator> create();<br>
<br>
I won't question the class hierarchy design, but this could be done<br>
with a simple public constructur and stored as unique_ptr in the<br>
pipeline handler maybe.<br></blockquote><div>The hierarchy design is because the 6th patch introduces the shift,</div><div>which can be used on all test pattern generators.</div><div>And yes, a static create function is not needed. Updated.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> +private:<br>
> + /* Generate a template buffer of the color bar test pattern. */<br>
> + void configure(const Size &size) override;<br>
<br>
Same questions as the one on generateFrame, it's surely something I<br>
don't well understand then<br>
<br>
> +};<br>
> +<br>
> +class DiagonalLinesGenerator : public TestPatternGenerator<br>
> +{<br>
> +public:<br>
> + static std::unique_ptr<DiagonalLinesGenerator> create();<br>
> +<br>
> +private:<br>
> + /* Generate a template buffer of the diagonal lines test pattern. */<br>
> + void configure(const Size &size) override;<br>
> +};<br>
> +<br>
> +} /* namespace libcamera */<br>
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp<br>
> index 74eb8c7ad..357fdd035 100644<br>
> --- a/src/libcamera/pipeline/virtual/virtual.cpp<br>
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp<br>
> @@ -192,10 +192,14 @@ int PipelineHandlerVirtual::exportFrameBuffers(<br>
> return dmaBufAllocator_.exportBuffers(config.bufferCount, planeSizes, buffers);<br>
> }<br>
><br>
> -int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera,<br>
> +int PipelineHandlerVirtual::start(Camera *camera,<br>
> [[maybe_unused]] const ControlList *controls)<br>
> {<br>
> /* \todo Start reading the virtual video if any. */<br>
> + VirtualCameraData *data = cameraData(camera);<br>
> +<br>
> + data->frameGenerator_->configure(data->stream_.configuration().size);<br>
<br>
Shouldn't this be done at Pipeline::configure() ?</blockquote><div>Right, migrated. Only that the generators will prepare the source</div><div>images, which will be duplicated when applications like Android</div><div>adapters calls configure() multiple times.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +<br>
> return 0;<br>
> }<br>
><br>
> @@ -207,9 +211,14 @@ void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera)<br>
> int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,<br>
> Request *request)<br>
> {<br>
> + VirtualCameraData *data = cameraData(camera);<br>
> +<br>
> /* \todo Read from the virtual video if any. */<br>
> - for (auto it : request->buffers())<br>
> - completeBuffer(request, it.second);<br>
> + for (auto const &[stream, buffer] : request->buffers()) {<br>
<br>
Unless something changes in the next patches, you only have one stream<br></blockquote><div>Let's consider multiple-stream support: I tried to enable multiple streams in</div><div>the next version, while I failed to pass the unit test: multi_stream_test:<br><font face="arial, sans-serif">```</font></div><div>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><font face="arial, sans-serif"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">[295:43:43.910237144] [1441841]<span class="gmail-Apple-converted-space"> </span></span><span class="gmail-s2" style="font-variant-ligatures:no-common-ligatures;color:rgb(47,180,29)"> INFO </span><span class="gmail-s3" style="font-variant-ligatures:no-common-ligatures;color:rgb(192,192,192)">IPAManager </span><span class="gmail-s4" style="font-variant-ligatures:no-common-ligatures;color:rgb(67,126,255)">ipa_manager.cpp:137 </span><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">libcamera is not installed. Adding '/usr/local/google/home/chenghaoyang/Workspace/libca</span></font></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="arial, sans-serif">mera/build/src/ipa' to the IPA search path</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><font face="arial, sans-serif"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">[295:43:43.914030564] [1441841]<span class="gmail-Apple-converted-space"> </span></span><span class="gmail-s2" style="font-variant-ligatures:no-common-ligatures;color:rgb(47,180,29)"> INFO </span><span class="gmail-s3" style="font-variant-ligatures:no-common-ligatures;color:rgb(192,192,192)">Camera </span><span class="gmail-s4" style="font-variant-ligatures:no-common-ligatures;color:rgb(67,126,255)">camera_manager.cpp:325 </span><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">libcamera v0.3.1+79-8ca4f033-dirty (2024-08-29T19:18:51UTC)</span></font></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><font face="arial, sans-serif"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">[295:43:43.915493754] [1441844] </span><span class="gmail-s5" style="font-variant-ligatures:no-common-ligatures;color:rgb(180,36,25)">ERROR </span><span class="gmail-s3" style="font-variant-ligatures:no-common-ligatures;color:rgb(192,192,192)">DmaBufAllocator </span><span class="gmail-s4" style="font-variant-ligatures:no-common-ligatures;color:rgb(67,126,255)">dma_buf_allocator.cpp:120 </span><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">Could not open any dma-buf provider</span></font></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><font face="arial, sans-serif"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">[295:43:43.919118835] [1441841]<span class="gmail-Apple-converted-space"> </span></span><span class="gmail-s2" style="font-variant-ligatures:no-common-ligatures;color:rgb(47,180,29)"> INFO </span><span class="gmail-s3" style="font-variant-ligatures:no-common-ligatures;color:rgb(192,192,192)">IPAManager </span><span class="gmail-s4" style="font-variant-ligatures:no-common-ligatures;color:rgb(67,126,255)">ipa_manager.cpp:137 </span><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">libcamera is not installed. Adding '/usr/local/google/home/chenghaoyang/Workspace/libca</span></font></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="arial, sans-serif">mera/build/src/ipa' to the IPA search path</font></span></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><font face="arial, sans-serif"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">[295:43:43.922245825] [1441841]<span class="gmail-Apple-converted-space"> </span></span><span class="gmail-s2" style="font-variant-ligatures:no-common-ligatures;color:rgb(47,180,29)"> INFO </span><span class="gmail-s3" style="font-variant-ligatures:no-common-ligatures;color:rgb(192,192,192)">Camera </span><span class="gmail-s4" style="font-variant-ligatures:no-common-ligatures;color:rgb(67,126,255)">camera_manager.cpp:325 </span><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">libcamera v0.3.1+79-8ca4f033-dirty (2024-08-29T19:18:51UTC)</span></font></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><font face="arial, sans-serif"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">[295:43:43.922820175] [1441846] </span><span class="gmail-s5" style="font-variant-ligatures:no-common-ligatures;color:rgb(180,36,25)">ERROR </span><span class="gmail-s3" style="font-variant-ligatures:no-common-ligatures;color:rgb(192,192,192)">DmaBufAllocator </span><span class="gmail-s4" style="font-variant-ligatures:no-common-ligatures;color:rgb(67,126,255)">dma_buf_allocator.cpp:120 </span><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures">Could not open any dma-buf provider</span></font></p>
<p class="gmail-p1" style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-variant-alternates:normal;font-size-adjust:none;font-kerning:auto;font-feature-settings:normal;font-stretch:normal;line-height:normal;color:rgb(242,242,242);background-color:rgb(0,0,0)"><span class="gmail-s1" style="font-variant-ligatures:no-common-ligatures"><font face="arial, sans-serif">Unable to set the pipeline to the playing state.</font></span></p><font face="arial, sans-serif">```</font></div><div><font face="arial, sans-serif">gitlab pipeline link: </font><a href="https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/pipelines/1260412">https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/pipelines/1260412</a></div><div><br></div><div>Looking into `gsteramer_multi_stream_test.cpp`,</div><div>I still couldn't figure out what's wrong...</div><div><font face="arial, sans-serif">Do you know what's tested in this unit test? I know that</font></div><div><font face="arial, sans-serif">DmaBufAllocator isn't valid, while I added logs and didn't</font></div><div><font face="arial, sans-serif">find `PipelineHandlerVirtual::exportFrameBuffers` being</font></div><div><font face="arial, sans-serif">called.</font></div><div><br></div><div>I also need your opinion: I think there's nothing, except for the unit test :p,</div><div>to stop Virtual Pipeline Handler from supporting multiple streams, while</div><div>do you think there should be a limitation?</div><div>I was setting the maximum to 3 for StillCapture, VideoRecording, and</div><div>ViewFinder, while you know there can be multiple streams as the same</div><div>StreamRole...</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + /* map buffer and fill test patterns */<br>
> + data->frameGenerator_->generateFrame(stream->configuration().size, buffer);<br>
> + completeBuffer(request, buffer);<br>
> + }<br>
><br>
> request->metadata().set(controls::SensorTimestamp, currentTimestamp());<br>
> completeRequest(request);<br>
> @@ -241,11 +250,24 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator<br>
> std::set<Stream *> streams{ &data->stream_ };<br>
> const std::string id = "Virtual0";<br>
> std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);<br>
> +<br>
> + initFrameGenerator(camera.get());<br>
> +<br>
> registerCamera(std::move(camera));<br>
><br>
> return false; // Prevent infinite loops for now<br>
> }<br>
><br>
> +void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)<br>
> +{<br>
> + auto data = cameraData(camera);<br>
> + if (data->testPattern_ == TestPattern::DiagonalLines) {<br>
<br>
No {} for single lines statements<br></blockquote><div>Done</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Who initializes data->testPattern_ ?<br></blockquote><div>Hmm, it'll be done in the next patch, when we read from the </div><div>configuration file...</div><div>In this patch though, it's using the default value when we create</div><div>CameraData in the match() function. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + data->frameGenerator_ = DiagonalLinesGenerator::create();<br>
> + } else {<br>
> + data->frameGenerator_ = ColorBarsGenerator::create();<br>
> + }<br>
> +}<br>
> +<br>
> REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, "virtual")<br>
><br>
> } /* namespace libcamera */<br>
> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h<br>
> index 6fc6b34d8..fecd9fa6f 100644<br>
> --- a/src/libcamera/pipeline/virtual/virtual.h<br>
> +++ b/src/libcamera/pipeline/virtual/virtual.h<br>
> @@ -13,6 +13,8 @@<br>
> #include "libcamera/internal/dma_buf_allocator.h"<br>
> #include "libcamera/internal/pipeline_handler.h"<br>
><br>
> +#include "test_pattern_generator.h"<br>
> +<br>
> namespace libcamera {<br>
><br>
> class VirtualCameraData : public Camera::Private<br>
> @@ -29,9 +31,13 @@ public:<br>
><br>
> ~VirtualCameraData() = default;<br>
><br>
> + TestPattern testPattern_;<br>
> +<br>
> std::vector<Resolution> supportedResolutions_;<br>
><br>
> Stream stream_;<br>
> +<br>
> + std::unique_ptr<FrameGenerator> frameGenerator_;<br>
> };<br>
><br>
> class VirtualCameraConfiguration : public CameraConfiguration<br>
> @@ -72,6 +78,8 @@ private:<br>
> return static_cast<VirtualCameraData *>(camera->_d());<br>
> }<br>
><br>
> + void initFrameGenerator(Camera *camera);<br>
> +<br>
> DmaBufAllocator dmaBufAllocator_;<br>
> };<br>
><br>
> --<br>
> 2.46.0.184.g6999bdac58-goog<br>
><br>
</blockquote></div></div>