[EXT] [PATCH v2] libcamera: virtual: Install configuration file

[Laurent Pinchart]

On Fri, Jan 10, 2025 at 04:08:02AM +0200, Laurent Pinchart wrote:
> On Fri, Jan 03, 2025 at 06:43:18PM +0100, Julien Vuillaumier wrote:
> > On 20/12/2024 17:44, Barnabás Pőcze wrote:
> > > 2024. december 20., péntek 17:17 keltezéssel, Julien Vuillaumier írta:
> > >> On 20/12/2024 14:54, Barnabás Pőcze wrote:
> > >>
> > >>> Install the example configuration file of the virtual pipeline
> > >>> handler as it serves documentation purposes, and to make the
> > >>> virtual pipeline handler easily usable in CI.
> > >>>
> > >>> Nonetheless, the file is installed with the ".example" suffix
> > >>> so that it will not be used by default, to avoid cluttering
> > >>> the camera lists of users whose distributions decide to
> > >>> enable the virtual pipeline handler.
> > >>>
> > >>> The file is installed in the proper location for convenience:
> > >>> (1) is is easier to use it in the CI;
> > >>> (2) users need not browse documentation to determine where
> > >>> they should place the file.
> > >>
> > >> Virtual pipeline, when enabled, outputs a noisy error message during the
> > >> match() when its configuration file is not installed:
> > >>
> > >> DEBUG Camera camera_manager.cpp:140 Found registered pipeline handler
> > >> 'virtual'
> > >> ERROR Pipeline pipeline_handler.cpp:629 Configuration file
> > >> '/usr/share/libcamera/pipeline/virtual/virtual.yaml' not found for
> > >> pipeline handler 'virtual'
> > >> ERROR Virtual virtual.cpp:336 Failed to open config file:
> > >>
> > >> Should virtual.yaml be installed with its actual name without the
> > >> '.example' suffix. That would clear the error, though augmenting the
> > >> camera list.
> > > 
> > > That is indeed correct, but there should be no issues apart from that.
> > > Is it causing any problems?
> > 
> > There is no functional issue - however it is confusing to have a 
> > pipeline handler reporting errors in the libcamera log during the match.
> > 
> > > I don't believe adding the virtual cameras is acceptable for any end user.
> > 
> > I am not sure that adding virtual cameras is really an issue for the end 
> > user:
> > - virtual pipeline being present means that 'test' meson config option 
> > equals true - option may not be used by distributions (just speculating)
> 
> The virtual pipeline handler can also be enabled by setting the
> 'pipelines' meson option, without enabling the 'test' option.
> 
> > - vimc pipeline handler is already adding virtual cameras
> 
> Correct, but only if the vimc driver is loaded.
> 
> > - vimc 
> > pipeline presence also depends on the 'test' meson option.
> 
> It can also be enabled through the 'pipelines' option.
> 
> > It is only my view but I see it preferable for the end user to have 
> > virtual cameras added and no error messages.
> 
> I think we all agree that the error message is wrong.
> 
> By default, I believe no virtual camera should be created. That can be
> achieved by ensuring that distributions either do not enable the virtual
> pipeline handler in their libcamera package (either explicitly, or
> through the 'test' option), or do not install the virtual pipeline
> handler configuration file.
> 
> I also believe that end users could find the virtual pipeline handler
> useful, which would call for enabling it in distribution packages. If
> that's the case, distributions should not install the configuration
> file.
> 
> At the end of the day distributions will decide what to do, regardless
> of what we do in meson.build. They can decide not to package the virtual
> pipeline handler configuration file even if we install it. Conversely,
> they can also decide to strip the .example suffix from the file we
> install, if we decide to install an example file only.
> 
> My personal opinion is that we should install the sample configuration
> file with a .example suffix, and demote the error message to an info
> message.

Or even a debug message.

> When running libcamera from the build directory the virtual
> pipeline handler configuration file will be loaded, making it easy to
> develop the pipeline handler without having to install libcamera or
> rename files.
> 
> > > Installing it with the ".example" suffix means that it is trivial for
> > > distributions to ship the virtual pipeline with an example configuration
> > > without it interfering normal operations.
> > > 
> > > I suppose a placeholder `virtual.yaml` file could be installed with the
> > > following content: { }
> > > 
> > > But I am not entirely sure if it's worth it >
> > > 
> > >>> Signed-off-by: Barnabás Pőcze pobrn at protonmail.com
> > >>> Reviewed-by: Kieran Bingham kieran.bingham at ideasonboard.com
> > >>> Reviewed-by: Harvey Yang chenghaoyang at chromium.org
> > >>> Reviewed-by: Laurent Pinchart laurent.pinchart at ideasonboard.com
> > >>> ---
> > >>> src/libcamera/pipeline/virtual/data/meson.build | 4 ++++
> > >>> src/libcamera/pipeline/virtual/meson.build | 2 ++
> > >>> 2 files changed, 6 insertions(+)
> > >>> create mode 100644 src/libcamera/pipeline/virtual/data/meson.build
> > >>>
> > >>> diff --git a/src/libcamera/pipeline/virtual/data/meson.build b/src/libcamera/pipeline/virtual/data/meson.build
> > >>> new file mode 100644
> > >>> index 000000000..ce63f9a27
> > >>> --- /dev/null
> > >>> +++ b/src/libcamera/pipeline/virtual/data/meson.build
> > >>> @@ -0,0 +1,4 @@
> > >>> +install_data('virtual.yaml',
> > >>> + install_dir : pipeline_data_dir / 'virtual',
> > >>> + install_tag : 'runtime',
> > >>> + rename: 'virtual.yaml.example')
> > >>> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build
> > >>> index 4786fe2e0..c84345936 100644
> > >>> --- a/src/libcamera/pipeline/virtual/meson.build
> > >>> +++ b/src/libcamera/pipeline/virtual/meson.build
> > >>> @@ -11,3 +11,5 @@ libjpeg = dependency('libjpeg', required : true)
> > >>>
> > >>> libcamera_deps += [libyuv_dep]
> > >>> libcamera_deps += [libjpeg]
> > >>> +
> > >>> +subdir('data')

-- 
Regards,

Laurent Pinchart