<div dir="ltr"><div dir="ltr">Hi Laurent and Kieran,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Aug 1, 2024 at 10:47 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@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">On Thu, Aug 01, 2024 at 03:39:51PM +0800, Cheng-Hao Yang wrote:<br>
> On Wed, Jul 31, 2024 at 9:42 PM Laurent Pinchart wrote:<br>
> > On Mon, Jul 29, 2024 at 04:03:05PM +0100, Kieran Bingham wrote:<br>
> > > Quoting Cheng-Hao Yang (2024-07-26 12:41:40)<br>
> > > > On Fri, Jul 26, 2024 at 6:56 PM Kieran Bingham wrote:<br>
> > > > > Quoting Harvey Yang (2024-07-26 10:54:21)<br>
> > > > > > Hi folks,<br>
> > > > > ><br>
> > > > > > Sorry for the super late update. It's been a while since the last series<br>
> > > > > > of patches, as we were busy with mtkisp7's bringup & migration.<br>
> > > > > ><br>
> > > > > > Thanks to Hans de Goede's work on DmaBufAllocator, the prerequisites of<br>
> > > > > > VirtualPipelineHandler are almost landed. Only a helper function added<br>
> > > > > > in the first patch.<br>
> > > > ><br>
> > > > > Excellent, Yes - I had this virtual pipeline work in mind as well when<br>
> > > > > the buffer allocator landed!<br>
> > > > ><br>
> > > > > I've just tried to run this and built locally then tried to run but I<br>
> > > > > get:<br>
> > > > ><br>
> > > > > kbingham@Monstersaurus:~/iob/libcamera/libcamera$<br>
> > > > > ./build/gcc/src/apps/cam/cam --list<br>
> > > > > [686:32:50.927990678] [2690143] INFO IPAManager ipa_manager.cpp:143 libcamera is not installed. Adding '/home/kbingham/iob/libcamera/libcamera/build/gcc/src/ipa' to the IPA search path<br>
> > > > > [686:32:50.933827146] [2690143] INFO Camera camera_manager.cpp:313 libcamera v0.3.1+7-e4713a22<br>
> > > > > [686:32:51.016903716] [2690146] WARN CameraSensorProperties camera_sensor_properties.cpp:283 No static properties available for 'Sensor B'<br>
> > > > > [686:32:51.016924335] [2690146] WARN CameraSensorProperties camera_sensor_properties.cpp:285 Please consider updating the camera sensor properties database<br>
> > > > > [686:32:51.016941437] [2690146] WARN CameraSensor camera_sensor.cpp:479 'Sensor B': Failed to retrieve the camera location<br>
> > > > > [686:32:51.016953209] [2690146] WARN CameraSensor camera_sensor.cpp:501 'Sensor B': Rotation control not available, default to 0 degrees<br>
> > > > > [686:32:51.019533566] [2690146] INFO IPAProxy ipa_proxy.cpp:130 libcamera is not installed. Loading IPA configuration from '/home/kbingham/iob/libcamera/libcamera/src/ipa/vimc/data'<br>
> > > > > [686:32:51.019729155] [2690146] ERROR DmaBufAllocator dma_buf_allocator.cpp:121 Could not open any dma-buf provider<br>
> > > ><br>
> > > > JFYI, currently virtual pipeline handler uses the default type<br>
> > > > `DmaBufAllocatorFlag::CmaHeap`. If none of the dma heaps or udmabuf are<br>
> > > > available, we can use memfd_create [1] to allocate local memory.<br>
> > > ><br>
> > > > [1]: <a href="https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/4739525/2" rel="noreferrer" target="_blank">https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/4739525/2</a><br>
> > > ><br>
> > > > > [686:32:51.020135800] [2690146] INFO Pipeline pipeline_handler.cpp:575<br>
> > > > > libcamera is not installed. Loading platform configuration file from<br>
> > > > > '/home/kbingham/iob/libcamera/libcamera/src/libcamera/pipeline/virtual/data/virtual.yaml'<br>
> > > > > terminate called after throwing an instance of<br>
> > > > > 'std::filesystem::__cxx11::filesystem_error'<br>
> > > > > what(): filesystem error: directory iterator cannot open directory: No<br>
> > > > > such file or directory [files/libcamera_short_anime/]<br>
> > > > > Aborted (core dumped)<br>
> > > ><br>
> > > > The virtual pipeline handler has a config file<br>
> > > > `src/libcamera/pipeline/virtual/data/virtual.yaml` to load and config the<br>
> > > > cameras listed. In the current default yaml (We can discuss what should be<br>
> > > > the default config file though) expects `files/libcamera_short_anime/` (I<br>
> > > > believe it's the relative path from which you run libcamera) to contain at<br>
> > > > least one `0.jpg` (and `1.jpg`, `2.jpg`, ...) to support camera `Virtual0`;<br>
> > > > `files/bike-min.jpg` to support camera `Virtual1`;<br>
> > > > `files/chrome_anniversary.jpg` to support camera `Virtual5`. For the<br>
> > > > formats and details, please check<br>
> > > > `src/libcamera/pipeline/virtual/README.md`.<br>
> > > ><br>
> > > > I also need your opinions: maybe we should upload some images used in the<br>
> > > > sample yaml config file in the patch?<br>
> > ><br>
> > > I don't know if you should upload files, but if it requires files then<br>
> > > they need to be there!<br>
> > ><br>
> > > The files perhaps should be an optional additional feature - with fall<br>
> > > back to a (simple) test pattern generator perhaps?<br>
> ><br>
> > That's what I was about to propose, so I think it's a good idea.<br>
> ><br>
> ><br>
> Hmm, I still find it weird to have a fallback mechanism if an image file<br>
> is not installed as the configuration file indicates.<br>
> <br>
> If you still think that it's a proper mechanism, how about we add it in<br>
> another series of patches? We might also need to discuss the format<br>
> and fields of the configuration file.<br>
<br>
I don't see it as much as a fallback than a default behaviour. I don't<br>
think we should have a set of images in the source tree for the virtual<br>
pipeline handler. The default configuration file should enable a TPG,<br>
and users can optionally modify the configuration and add their own<br>
images if they desire.<br>
<br></blockquote><div><br></div><div>I agree that we might not want a set of images in the source tree.</div><div><br></div><div>Just to clarify: in the latest patches, the default configuration file only</div><div>uses TPGs, and yes, users can optionally modify the config file and</div><div>add images.<br><br>I thought that Kieran and you want the config file to support a camera</div><div>with both TPG and an image generator, and use the latter if the images</div><div>are in the data path. Maybe I misunderstood.</div><div><br></div><div>BR,</div><div>Harvey</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">
> > > > Does it have to be installed to run? Is there anything else missing?<br>
> > ><br>
> > > It should be possible to run without being installed.<br>
> > ><br>
> > > I've pushed this branch to the CI and it fails in many of the instances:<br>
> > ><br>
> > > - <a href="https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1235732" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1235732</a><br>
> > ><br>
> > > It might be worth making a fork of<br>
> > > <a href="https://gitlab.freedesktop.org/camera/libcamera" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/camera/libcamera</a> for yourself on gitlab<br>
> > > so that you can run the integration tests while developing. But I'm<br>
> > > happy to push any patches you send to the list and report the CI url for<br>
> > > you. (I do expect that to be automated 'soon'*)<br>
> > ><br>
> > > * Soon is such a hard term to define ;-)<br>
> > ><br>
> > > > > > Konami, our intern in 2023, helped to add test patterns and real image<br>
> > > > > > loading into VirtualPipelineHandler.<br>
> > > > > ><br>
> > > > > > I tried to address the previous comments, while it's very likely that I<br>
> > > > > > left some behind. Please leave the comments again if I do. Many thanks!<br>
> > > > > ><br>
> > > > > ><br>
> > > > > > BR,<br>
> > > > > > Harvey<br>
> > > > > ><br>
> > > > > > Harvey Yang (3):<br>
> > > > > > Add a helper function exportFrameBuffers in DmaBufAllocator to make it<br>
> > > > > > easier to use.<br>
> > > > > > The Fatal check of having at least one MediaDevice was to prevent<br>
> > > > > > pipeline handler implementations searching and owning media devices<br>
> > > > > > with custom conventions, instead of using the base function<br>
> > > > > > |acquireMediaDevice|. It also has the assumption that there's at<br>
> > > > > > least one media device to make a camera work.<br>
> > > > > > libcamera: pipeline: Add VirtualPipelineHandler<br>
> > > > > ><br>
> > > > > > Konami Shu (4):<br>
> > > > > > libcamera: pipeline: Add test pattern for VirtualPipelineHandler<br>
> > > > > > libcamera: pipeline: Read config and register cameras based on the<br>
> > > > > > config<br>
> > > > > > libcamera: pipeline: Shift test pattern by 1 pixel left every frame<br>
> > > > > > libcamera: pipeline: Load images<br>
> > > > > ><br>
> > > > > > .../libcamera/internal/dma_buf_allocator.h | 10 +<br>
> > > > > > meson.build | 1 +<br>
> > > > > > meson_options.txt | 3 +-<br>
> > > > > > src/libcamera/dma_buf_allocator.cpp | 57 +++-<br>
> > > > > > src/libcamera/pipeline/virtual/README.md | 76 +++++<br>
> > > > > > .../pipeline/virtual/common_functions.cpp | 27 ++<br>
> > > > > > .../pipeline/virtual/common_functions.h | 18 ++<br>
> > > > > > .../pipeline/virtual/data/virtual.yaml | 51 ++++<br>
> > > > > > .../pipeline/virtual/frame_generator.h | 33 +++<br>
> > > > > > .../virtual/image_frame_generator.cpp | 154 ++++++++++<br>
> > > > > > .../pipeline/virtual/image_frame_generator.h | 65 ++++<br>
> > > > > > src/libcamera/pipeline/virtual/meson.build | 32 ++<br>
> > > > > > src/libcamera/pipeline/virtual/parser.cpp | 243 +++++++++++++++<br>
> > > > > > src/libcamera/pipeline/virtual/parser.h | 48 +++<br>
> > > > > > .../virtual/test_pattern_generator.cpp | 148 ++++++++++<br>
> > > > > > .../pipeline/virtual/test_pattern_generator.h | 58 ++++<br>
> > > > > > src/libcamera/pipeline/virtual/virtual.cpp | 279 ++++++++++++++++++<br>
> > > > > > src/libcamera/pipeline/virtual/virtual.h | 96 ++++++<br>
> > > > > > src/libcamera/pipeline_handler.cpp | 11 +-<br>
> > > > > > 19 files changed, 1404 insertions(+), 6 deletions(-)<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/README.md<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/common_functions.cpp<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/common_functions.h<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/frame_generator.h<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.cpp<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.h<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/meson.build<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/parser.cpp<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/parser.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>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/virtual.cpp<br>
> > > > > > create mode 100644 src/libcamera/pipeline/virtual/virtual.h<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>