[v6,0/7] Add VirtualPipelineHandler
mbox series

Message ID 20240726095626.3632402-1-chenghaoyang@chromium.org
Headers show
Series
  • Add VirtualPipelineHandler
Related show

Message

Harvey Yang July 26, 2024, 9:54 a.m. UTC
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.

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

Comments

Kieran Bingham July 26, 2024, 10:56 a.m. UTC | #1
Hi Harvey,

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@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
[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)


Does it have to be installed to run? Is there anything else missing?

--
Kieran


> 
> 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
> 
> -- 
> 2.46.0.rc1.232.g9752f9e123-goog
>
Harvey Yang July 26, 2024, 11:41 a.m. UTC | #2
Hi Kieran,

Thanks for looking into this!

On Fri, Jul 26, 2024 at 6:56 PM Kieran Bingham <
kieran.bingham@ideasonboard.com> wrote:

> Hi Harvey,
>
> 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@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?

BR,
Harvey

Does it have to be installed to run? Is there anything else missing?
>
> --
> Kieran
>
>
> >
> > 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
> >
> > --
> > 2.46.0.rc1.232.g9752f9e123-goog
> >
>
Kieran Bingham July 29, 2024, 3:03 p.m. UTC | #3
Quoting Cheng-Hao Yang (2024-07-26 12:41:40)
> Hi Kieran,
> 
> Thanks for looking into this!
> 
> On Fri, Jul 26, 2024 at 6:56 PM Kieran Bingham <
> kieran.bingham@ideasonboard.com> wrote:
> 
> > Hi Harvey,
> >
> > 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@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?

> 
> BR,
> Harvey
> 
> 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'*)

--
Kieran


* Soon is such a hard term to define ;-)




> >
> > --
> > Kieran
> >
> >
> > >
> > > 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
> > >
> > > --
> > > 2.46.0.rc1.232.g9752f9e123-goog
> > >
> >
Laurent Pinchart July 31, 2024, 1:41 p.m. UTC | #4
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@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.

> > 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
Harvey Yang Aug. 1, 2024, 7:39 a.m. UTC | #5
Hi Laurent,

On Wed, Jul 31, 2024 at 9:42 PM Laurent Pinchart <
laurent.pinchart@ideasonboard.com> 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@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.

BR,
Harvey


> > > 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
>
Laurent Pinchart Aug. 1, 2024, 8:46 a.m. UTC | #6
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@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
Harvey Yang Aug. 6, 2024, 7:09 a.m. UTC | #7
Hi Laurent and Kieran,

On Thu, Aug 1, 2024 at 10:47 AM Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> 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@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.
>
>
I agree that we might not want a set of images in the source tree.

Just to clarify: in the latest patches, the default configuration file only
uses TPGs, and yes, users can optionally modify the config file and
add images.

I thought that Kieran and you want the config file to support a camera
with both TPG and an image generator, and use the latter if the images
are in the data path. Maybe I misunderstood.

BR,
Harvey

> > > > 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
>