[RFC,v1] libcamera: virtual: Install configuration file
diff mbox series

Message ID 20241219123722.513083-1-pobrn@protonmail.com
State Superseded
Headers show
Series
  • [RFC,v1] libcamera: virtual: Install configuration file
Related show

Commit Message

Barnabás Pőcze Dec. 19, 2024, 12:37 p.m. UTC 
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.

Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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

Comments

Kieran Bingham Dec. 19, 2024, 12:49 p.m. UTC | #1 
Quoting Barnabás Pőcze (2024-12-19 12:37:24)
> 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.
> 
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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')

Oh that's a very neat way to handle this.

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

I think we normally handle subdir calls early in the meson.build, but I
think this is fine too.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> -- 
> 2.47.1
> 
>
Cheng-Hao Yang Dec. 19, 2024, 1:37 p.m. UTC | #2 
Hi Barnabás,

Thanks for handling this!

Reviewed-by: Harvey Yang <chenghaoyang@chromium.org>

On Thu, Dec 19, 2024 at 8:49 PM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Barnabás Pőcze (2024-12-19 12:37:24)
> > 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.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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')
>
> Oh that's a very neat way to handle this.
>
> > 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')
>
> I think we normally handle subdir calls early in the meson.build, but I
> think this is fine too.
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> > --
> > 2.47.1
> >
> >
Laurent Pinchart Dec. 19, 2024, 11:27 p.m. UTC | #3 
Hi Barnabás,

Thank you for the patch.

On Thu, Dec 19, 2024 at 12:37:24PM +0000, 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.
> 
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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')

I'm no specialist when it comes to file system layouts, but it seems to
be a common practice for example files to be installed in
/usr/share/doc/$pkgname/. Some software seem to use user locations
though. What that be a better target directory ? I'm not entirely sure
myself.

> 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')
Barnabás Pőcze Dec. 20, 2024, 9:41 a.m. UTC | #4 
Hi


2024. december 20., péntek 0:27 keltezéssel, Laurent Pinchart <laurent.pinchart@ideasonboard.com> írta:

> Hi Barnabás,
> 
> Thank you for the patch.
> 
> On Thu, Dec 19, 2024 at 12:37:24PM +0000, 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.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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')
> 
> I'm no specialist when it comes to file system layouts, but it seems to
> be a common practice for example files to be installed in
> /usr/share/doc/$pkgname/. Some software seem to use user locations
> though. What that be a better target directory ? I'm not entirely sure
> myself.
> [...]

I can find examples of both on my machine, and I did consider that, but during the
implementation I came to the conclusion that this is a better choice:

  * the documentation directory has the libcamera version number, so it is a bit more
    inconvenient to address it in the CI;
  * it is easier and more convenient if the file is already where it is supposed to be.

So my preference would be installing it into the proper configuration file directory
with the `runtime` installation tag.


Regards,
Barnabás Pőcze
Laurent Pinchart Dec. 20, 2024, 9:44 a.m. UTC | #5 
On Fri, Dec 20, 2024 at 09:41:11AM +0000, Barnabás Pőcze wrote:
> 2024. december 20., péntek 0:27 keltezéssel, Laurent Pinchart írta:
> > On Thu, Dec 19, 2024 at 12:37:24PM +0000, 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.
> > >
> > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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')
> > 
> > I'm no specialist when it comes to file system layouts, but it seems to
> > be a common practice for example files to be installed in
> > /usr/share/doc/$pkgname/. Some software seem to use user locations
> > though. What that be a better target directory ? I'm not entirely sure
> > myself.
> > [...]
> 
> I can find examples of both on my machine, and I did consider that, but during the
> implementation I came to the conclusion that this is a better choice:
> 
>   * the documentation directory has the libcamera version number, so it is a bit more
>     inconvenient to address it in the CI;
>   * it is easier and more convenient if the file is already where it is supposed to be.
> 
> So my preference would be installing it into the proper configuration file directory
> with the `runtime` installation tag.

Capturing that rationale in the commit message would be nice.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Christian Rauch Dec. 20, 2024, 4:05 p.m. UTC | #6 
Hi Barnabás,

Thank you for looking into this. I am also interested in making the
virtual pipeline available in an "install" space for testing
applications that use an installed libcamera or binary (Debian, etc.)
packages.

At the moment, the search path is hard coded through
"LIBCAMERA_DATA_DIR", the virtual pipeline will search in
"<install>/share/libcamera/pipeline/virtual/virtual.yaml".

How would you make the "virtual.yaml.example" available in the CI. Would
you move/rename/symlink "virtual.yaml.example" to match that path in CI?
If so, instead, I think it would be more convenient to set the
configuration file path at runtime with an environment variable instead
of changing the installed package. What do you think?

Best,
Christian


Am 19.12.24 um 13:37 schrieb Barnabás Pőcze:
> 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.
>
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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')
Barnabás Pőcze Dec. 20, 2024, 4:32 p.m. UTC | #7 
Hi


2024. december 20., péntek 17:05 keltezéssel, Christian Rauch <Rauch.Christian@gmx.de> írta:

> Hi Barnabás,
> 
> Thank you for looking into this. I am also interested in making the
> virtual pipeline available in an "install" space for testing
> applications that use an installed libcamera or binary (Debian, etc.)
> packages.
> 
> At the moment, the search path is hard coded through
> "LIBCAMERA_DATA_DIR", the virtual pipeline will search in
> "<install>/share/libcamera/pipeline/virtual/virtual.yaml".
> 
> How would you make the "virtual.yaml.example" available in the CI. Would
> you move/rename/symlink "virtual.yaml.example" to match that path in CI?

That's correct: https://patchwork.libcamera.org/patch/22407/


> If so, instead, I think it would be more convenient to set the
> configuration file path at runtime with an environment variable instead
> of changing the installed package. What do you think?

It's not clear to me if you're proposing the addition of a virtual pipeline handler
specific environmental variable, or if you want something like `LIBCAMERA_PIPELINE_CONFIG_PATH`?

And indeed, you're right, but currently the way configuration files are looked up
and used is a bit fragmented in libcamera. Ideally, that should be unified, and
possibly support for configuration fragments should be added. In that case this
file could be installed as "virtual.yaml", and then an appropriately placed fragment
(e.g. `~/.config/libcamera/libcamera.conf.d/10-enable-virtual.conf`) could simply
enable the pipeline handler (which would be disabled in the default configuration).
I hope you can understand that with that goal in mind, I am personally not a fan
of adding temporary measures for the being. But if I misunderstand your proposal,
please don't hesitate to correct me.


Regards,
Barnabás Pőcze


> 
> Best,
> Christian
> 
> 
> Am 19.12.24 um 13:37 schrieb Barnabás Pőcze:
> > 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.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.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')
>

Patch
diff mbox series 

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