[PATCH v6 0/7] Add VirtualPipelineHandler

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Aug 1 10:46:39 CEST 2024


On Thu, Aug 01, 2024 at 03:39:51PM +0800, Cheng-Hao Yang wrote:
> On Wed, Jul 31, 2024 at 9:42 PM Laurent Pinchart wrote:
> > On Mon, Jul 29, 2024 at 04:03:05PM +0100, Kieran Bingham wrote:
> > > Quoting Cheng-Hao Yang (2024-07-26 12:41:40)
> > > > On Fri, Jul 26, 2024 at 6:56 PM Kieran Bingham wrote:
> > > > > Quoting Harvey Yang (2024-07-26 10:54:21)
> > > > > > Hi folks,
> > > > > >
> > > > > > Sorry for the super late update. It's been a while since the last series
> > > > > > of patches, as we were busy with mtkisp7's bringup & migration.
> > > > > >
> > > > > > Thanks to Hans de Goede's work on DmaBufAllocator, the prerequisites of
> > > > > > VirtualPipelineHandler are almost landed. Only a helper function added
> > > > > > in the first patch.
> > > > >
> > > > > Excellent, Yes - I had this virtual pipeline work in mind as well when
> > > > > the buffer allocator landed!
> > > > >
> > > > > I've just tried to run this and built locally then tried to run but I
> > > > > get:
> > > > >
> > > > > kbingham at Monstersaurus:~/iob/libcamera/libcamera$
> > > > > ./build/gcc/src/apps/cam/cam --list
> > > > > [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
> > > > > [686:32:50.933827146] [2690143]  INFO Camera camera_manager.cpp:313 libcamera v0.3.1+7-e4713a22
> > > > > [686:32:51.016903716] [2690146]  WARN CameraSensorProperties camera_sensor_properties.cpp:283 No static properties available for 'Sensor B'
> > > > > [686:32:51.016924335] [2690146]  WARN CameraSensorProperties camera_sensor_properties.cpp:285 Please consider updating the camera sensor properties database
> > > > > [686:32:51.016941437] [2690146]  WARN CameraSensor camera_sensor.cpp:479 'Sensor B': Failed to retrieve the camera location
> > > > > [686:32:51.016953209] [2690146]  WARN CameraSensor camera_sensor.cpp:501 'Sensor B': Rotation control not available, default to 0 degrees
> > > > > [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'
> > > > > [686:32:51.019729155] [2690146] ERROR DmaBufAllocator dma_buf_allocator.cpp:121 Could not open any dma-buf provider
> > > >
> > > > JFYI, currently virtual pipeline handler uses the default type
> > > > `DmaBufAllocatorFlag::CmaHeap`. If none of the dma heaps or udmabuf are
> > > > available, we can use memfd_create [1] to allocate local memory.
> > > >
> > > > [1]: https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/4739525/2
> > > >
> > > > > [686:32:51.020135800] [2690146]  INFO Pipeline pipeline_handler.cpp:575
> > > > > libcamera is not installed. Loading platform configuration file from
> > > > > '/home/kbingham/iob/libcamera/libcamera/src/libcamera/pipeline/virtual/data/virtual.yaml'
> > > > > terminate called after throwing an instance of
> > > > > 'std::filesystem::__cxx11::filesystem_error'
> > > > >   what():  filesystem error: directory iterator cannot open directory: No
> > > > > such file or directory [files/libcamera_short_anime/]
> > > > > Aborted (core dumped)
> > > >
> > > > The virtual pipeline handler has a config file
> > > > `src/libcamera/pipeline/virtual/data/virtual.yaml` to load and config the
> > > > cameras listed. In the current default yaml (We can discuss what should be
> > > > the default config file though) expects `files/libcamera_short_anime/` (I
> > > > believe it's the relative path from which you run libcamera) to contain at
> > > > least one `0.jpg` (and `1.jpg`, `2.jpg`, ...) to support camera `Virtual0`;
> > > > `files/bike-min.jpg` to support camera `Virtual1`;
> > > > `files/chrome_anniversary.jpg` to support camera `Virtual5`. For the
> > > > formats and details, please check
> > > > `src/libcamera/pipeline/virtual/README.md`.
> > > >
> > > > I also need your opinions: maybe we should upload some images used in the
> > > > sample yaml config file in the patch?
> > >
> > > I don't know if you should upload files, but if it requires files then
> > > they need to be there!
> > >
> > > The files perhaps should be an optional additional feature - with fall
> > > back to a (simple) test pattern generator perhaps?
> >
> > That's what I was about to propose, so I think it's a good idea.
> >
> >
> Hmm, I still find it weird to have a fallback mechanism if an image file
> is not installed as the configuration file indicates.
> 
> If you still think that it's a proper mechanism, how about we add it in
> another series of patches? We might also need to discuss the format
> and fields of the configuration file.

I don't see it as much as a fallback than a default behaviour. I don't
think we should have a set of images in the source tree for the virtual
pipeline handler. The default configuration file should enable a TPG,
and users can optionally modify the configuration and add their own
images if they desire.

> > > > Does it have to be installed to run? Is there anything else missing?
> > >
> > > It should be possible to run without being installed.
> > >
> > > I've pushed this branch to the CI and it fails in many of the instances:
> > >
> > >  - https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1235732
> > >
> > > It might be worth making a fork of
> > > https://gitlab.freedesktop.org/camera/libcamera for yourself on gitlab
> > > so that you can run the integration tests while developing. But I'm
> > > happy to push any patches you send to the list and report the CI url for
> > > you. (I do expect that to be automated 'soon'*)
> > >
> > > * Soon is such a hard term to define ;-)
> > >
> > > > > > Konami, our intern in 2023, helped to add test patterns and real image
> > > > > > loading into VirtualPipelineHandler.
> > > > > >
> > > > > > I tried to address the previous comments, while it's very likely that I
> > > > > > left some behind. Please leave the comments again if I do. Many thanks!
> > > > > >
> > > > > >
> > > > > > BR,
> > > > > > Harvey
> > > > > >
> > > > > > Harvey Yang (3):
> > > > > >   Add a helper function exportFrameBuffers in DmaBufAllocator to make it
> > > > > >     easier to use.
> > > > > >   The Fatal check of having at least one MediaDevice was to prevent
> > > > > >     pipeline handler implementations searching and owning media devices
> > > > > >     with custom conventions, instead of using the base function
> > > > > >     |acquireMediaDevice|. It also has the assumption that there's at
> > > > > >     least one media device to make a camera work.
> > > > > >   libcamera: pipeline: Add VirtualPipelineHandler
> > > > > >
> > > > > > Konami Shu (4):
> > > > > >   libcamera: pipeline: Add test pattern for VirtualPipelineHandler
> > > > > >   libcamera: pipeline: Read config and register cameras based on the
> > > > > >     config
> > > > > >   libcamera: pipeline: Shift test pattern by 1 pixel left every frame
> > > > > >   libcamera: pipeline: Load images
> > > > > >
> > > > > >  .../libcamera/internal/dma_buf_allocator.h    |  10 +
> > > > > >  meson.build                                   |   1 +
> > > > > >  meson_options.txt                             |   3 +-
> > > > > >  src/libcamera/dma_buf_allocator.cpp           |  57 +++-
> > > > > >  src/libcamera/pipeline/virtual/README.md      |  76 +++++
> > > > > >  .../pipeline/virtual/common_functions.cpp     |  27 ++
> > > > > >  .../pipeline/virtual/common_functions.h       |  18 ++
> > > > > >  .../pipeline/virtual/data/virtual.yaml        |  51 ++++
> > > > > >  .../pipeline/virtual/frame_generator.h        |  33 +++
> > > > > >  .../virtual/image_frame_generator.cpp         | 154 ++++++++++
> > > > > >  .../pipeline/virtual/image_frame_generator.h  |  65 ++++
> > > > > >  src/libcamera/pipeline/virtual/meson.build    |  32 ++
> > > > > >  src/libcamera/pipeline/virtual/parser.cpp     | 243 +++++++++++++++
> > > > > >  src/libcamera/pipeline/virtual/parser.h       |  48 +++
> > > > > >  .../virtual/test_pattern_generator.cpp        | 148 ++++++++++
> > > > > >  .../pipeline/virtual/test_pattern_generator.h |  58 ++++
> > > > > >  src/libcamera/pipeline/virtual/virtual.cpp    | 279 ++++++++++++++++++
> > > > > >  src/libcamera/pipeline/virtual/virtual.h      |  96 ++++++
> > > > > >  src/libcamera/pipeline_handler.cpp            |  11 +-
> > > > > >  19 files changed, 1404 insertions(+), 6 deletions(-)
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/README.md
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/common_functions.cpp
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/common_functions.h
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/frame_generator.h
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.cpp
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/image_frame_generator.h
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/meson.build
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/parser.cpp
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/parser.h
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/test_pattern_generator.cpp
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/test_pattern_generator.h
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/virtual.cpp
> > > > > >  create mode 100644 src/libcamera/pipeline/virtual/virtual.h

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list