[libcamera-devel,v2] v4l2: Provide libcamerify wrapper script
diff mbox series

Message ID 20220131210757.105299-1-kieran.bingham@ideasonboard.com
State Superseded
Delegated to: Kieran Bingham
Headers show
Series
  • [libcamera-devel,v2] v4l2: Provide libcamerify wrapper script
Related show

Commit Message

Kieran Bingham Jan. 31, 2022, 9:07 p.m. UTC
Support easier usage of the v4l2 compatibility layer with a script that
handles the LD_PRELOAD for applications.

The wrapper can be prefixed to launch any application with the preload
set:

 $ libcamerify v4l2-ctl --list-devices
 \_SB_.PCI0.GP13.XHC0.RHUB.PRT4- (libcamera:0):
 	/dev/video0

 platform/vimc.0 Sensor B (libcamera:1):
 	/dev/video2
 	/dev/video3
 	/dev/video4

Specifying '-d' once before the command to run will enable V4L2Compat
layer debug output from libcamera.

Specifying '-d' twice will enable all debug levels from all libcamera
components and provide a very detailed log for analysis.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
v2:
 - rename to libcamerify (from libcamera-v4l2)
 - add extending support for setting debug log levels.


 src/v4l2/libcamerify.in | 34 ++++++++++++++++++++++++++++++++++
 src/v4l2/meson.build    | 14 ++++++++++++++
 2 files changed, 48 insertions(+)
 create mode 100755 src/v4l2/libcamerify.in

Comments

Laurent Pinchart Feb. 1, 2022, 7:20 a.m. UTC | #1
Hi Kieran,

Thank you for the patch.

On Mon, Jan 31, 2022 at 09:07:57PM +0000, Kieran Bingham wrote:
> Support easier usage of the v4l2 compatibility layer with a script that

s/v4l2/V4L2/

> handles the LD_PRELOAD for applications.
> 
> The wrapper can be prefixed to launch any application with the preload
> set:
> 
>  $ libcamerify v4l2-ctl --list-devices
>  \_SB_.PCI0.GP13.XHC0.RHUB.PRT4- (libcamera:0):
>  	/dev/video0
> 
>  platform/vimc.0 Sensor B (libcamera:1):
>  	/dev/video2
>  	/dev/video3
>  	/dev/video4
> 
> Specifying '-d' once before the command to run will enable V4L2Compat
> layer debug output from libcamera.
> 
> Specifying '-d' twice will enable all debug levels from all libcamera
> components and provide a very detailed log for analysis.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
> v2:
>  - rename to libcamerify (from libcamera-v4l2)
>  - add extending support for setting debug log levels.
> 
> 
>  src/v4l2/libcamerify.in | 34 ++++++++++++++++++++++++++++++++++
>  src/v4l2/meson.build    | 14 ++++++++++++++
>  2 files changed, 48 insertions(+)
>  create mode 100755 src/v4l2/libcamerify.in
> 
> diff --git a/src/v4l2/libcamerify.in b/src/v4l2/libcamerify.in
> new file mode 100755
> index 000000000000..161f5e49190e
> --- /dev/null
> +++ b/src/v4l2/libcamerify.in
> @@ -0,0 +1,34 @@
> +#!/bin/sh

License ?

> +
> +if [ "$LD_PRELOAD" = "" ] ; then
> +   LD_PRELOAD='@LIBCAMERA_V4L2_SO@'
> +else
> +   LD_PRELOAD="$LD_PRELOAD "'@LIBCAMERA_V4L2_SO@'
> +fi
> +
> +export LD_PRELOAD

Can you move this just before the exec call, to avoid LD_PRELOADing the
library in other commands executed by thie script ?

> +
> +help() {
> +	echo "$0: Load an application with libcamera V4L2 compatibility layer preload"
> +	echo " $0 [OPTIONS...] executable [args]"
> +	echo " -d, --debug	Increase log level"
> +	exit 1;
> +}
> +
> +debug=0
> +while [ $# -gt 0 ]; do
> +	case $1 in
> +		-d|--debug) debug=$((debug+1));;
> +		-h) help; break;;

"break" should be "exit 0".

> +		--) shift; break;;
> +		-*) echo "Unrecognised option: $1"; help; break;;

Same here.

> +		*) break;;

I'd find this more readable:

		-d|--debug)
			debug=$((debug+1))
			;;
		-h)
			help
			break
			;;
		--)
			shift
			break
			;;
		-*)
			echo "Unrecognised option: $1"
			help
			break
			;;
		*)
			break
			;;

> +	esac
> +	shift
> +done
> +
> +[ $debug -gt 0 ] && loglevel=V4L2Compat:0
> +[ $debug -gt 1 ] && loglevel=0
> +[ "$loglevel" != "" ] && export LIBCAMERA_LOG_LEVELS=$loglevel
> +
> +exec "$@"
> diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
> index f78497b6799b..07ff9a6939d2 100644
> --- a/src/v4l2/meson.build
> +++ b/src/v4l2/meson.build
> @@ -33,3 +33,17 @@ v4l2_compat = shared_library('v4l2-compat',
>                               install : true,
>                               dependencies : [libcamera_private, libdl],
>                               cpp_args : v4l2_compat_cpp_args)
> +
> +# Provide a wrapper script to support easily loading applications with the V4L2
> +# adaptation layer
> +
> +config_h.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')

I don't think this should be added to config.h. Can you create a
separate variable ?

cdata = configuration_data()
cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')

I also wonder if we could use v4l2_compat.name() instead of
'v4l2-compat.so' (but that's only available starting in meson 0.54).

> +
> +summary({
> +    'LIBCAMERA_V4L2_SO' : config_h.get('LIBCAMERA_V4L2_SO'),
> +}, section : 'Paths')

I would have skipped this. It's a bit out of place, with all the other
paths being directories.

> +
> +configure_file(input : 'libcamerify.in',
> +               output : 'libcamerify',
> +               configuration : config_h,
> +               install_dir : get_option('bindir'))
Kieran Bingham Feb. 1, 2022, 10:26 a.m. UTC | #2
Quoting Laurent Pinchart (2022-02-01 07:20:52)
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Mon, Jan 31, 2022 at 09:07:57PM +0000, Kieran Bingham wrote:
> > Support easier usage of the v4l2 compatibility layer with a script that
> 
> s/v4l2/V4L2/
> 
> > handles the LD_PRELOAD for applications.
> > 
> > The wrapper can be prefixed to launch any application with the preload
> > set:
> > 
> >  $ libcamerify v4l2-ctl --list-devices
> >  \_SB_.PCI0.GP13.XHC0.RHUB.PRT4- (libcamera:0):
> >       /dev/video0
> > 
> >  platform/vimc.0 Sensor B (libcamera:1):
> >       /dev/video2
> >       /dev/video3
> >       /dev/video4
> > 
> > Specifying '-d' once before the command to run will enable V4L2Compat
> > layer debug output from libcamera.
> > 
> > Specifying '-d' twice will enable all debug levels from all libcamera
> > components and provide a very detailed log for analysis.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> > v2:
> >  - rename to libcamerify (from libcamera-v4l2)
> >  - add extending support for setting debug log levels.
> > 
> > 
> >  src/v4l2/libcamerify.in | 34 ++++++++++++++++++++++++++++++++++
> >  src/v4l2/meson.build    | 14 ++++++++++++++
> >  2 files changed, 48 insertions(+)
> >  create mode 100755 src/v4l2/libcamerify.in
> > 
> > diff --git a/src/v4l2/libcamerify.in b/src/v4l2/libcamerify.in
> > new file mode 100755
> > index 000000000000..161f5e49190e
> > --- /dev/null
> > +++ b/src/v4l2/libcamerify.in
> > @@ -0,0 +1,34 @@
> > +#!/bin/sh
> 
> License ?
> 
> > +
> > +if [ "$LD_PRELOAD" = "" ] ; then
> > +   LD_PRELOAD='@LIBCAMERA_V4L2_SO@'
> > +else
> > +   LD_PRELOAD="$LD_PRELOAD "'@LIBCAMERA_V4L2_SO@'
> > +fi
> > +
> > +export LD_PRELOAD
> 
> Can you move this just before the exec call, to avoid LD_PRELOADing the
> library in other commands executed by thie script ?

Eeep yes. How did I miss that.

> 
> > +
> > +help() {
> > +     echo "$0: Load an application with libcamera V4L2 compatibility layer preload"
> > +     echo " $0 [OPTIONS...] executable [args]"
> > +     echo " -d, --debug      Increase log level"
> > +     exit 1;
> > +}
> > +
> > +debug=0
> > +while [ $# -gt 0 ]; do
> > +     case $1 in
> > +             -d|--debug) debug=$((debug+1));;
> > +             -h) help; break;;
> 
> "break" should be "exit 0".

help() was already calling exit 1;, but yes, I can move that in here.


> 
> > +             --) shift; break;;
> > +             -*) echo "Unrecognised option: $1"; help; break;;
> 
> Same here.
> 
> > +             *) break;;
> 
> I'd find this more readable:
> 
>                 -d|--debug)
>                         debug=$((debug+1))
>                         ;;
>                 -h)
>                         help
>                         break
>                         ;;
>                 --)
>                         shift
>                         break
>                         ;;
>                 -*)
>                         echo "Unrecognised option: $1"
>                         help
>                         break
>                         ;;
>                 *)
>                         break
>                         ;;
> 

sure

> > +     esac
> > +     shift
> > +done
> > +
> > +[ $debug -gt 0 ] && loglevel=V4L2Compat:0
> > +[ $debug -gt 1 ] && loglevel=0
> > +[ "$loglevel" != "" ] && export LIBCAMERA_LOG_LEVELS=$loglevel
> > +
> > +exec "$@"
> > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
> > index f78497b6799b..07ff9a6939d2 100644
> > --- a/src/v4l2/meson.build
> > +++ b/src/v4l2/meson.build
> > @@ -33,3 +33,17 @@ v4l2_compat = shared_library('v4l2-compat',
> >                               install : true,
> >                               dependencies : [libcamera_private, libdl],
> >                               cpp_args : v4l2_compat_cpp_args)
> > +
> > +# Provide a wrapper script to support easily loading applications with the V4L2
> > +# adaptation layer
> > +
> > +config_h.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')
> 
> I don't think this should be added to config.h. Can you create a
> separate variable ?
> 
> cdata = configuration_data()
> cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')
> 
> I also wonder if we could use v4l2_compat.name() instead of
> 'v4l2-compat.so' (but that's only available starting in meson 0.54).

I also asked in IRC #meson about this, hoping I could get a binary
target to tell me it's full installation path but alas...

> 
> > +
> > +summary({
> > +    'LIBCAMERA_V4L2_SO' : config_h.get('LIBCAMERA_V4L2_SO'),
> > +}, section : 'Paths')
> 
> I would have skipped this. It's a bit out of place, with all the other
> paths being directories.

But it's also a path/file that (at least without this script) people need to
know, so that tehy can do LD_PRELOAD= themselves.

I can drop it though, as I expect this script will help exactly those
users.


> > +
> > +configure_file(input : 'libcamerify.in',
> > +               output : 'libcamerify',
> > +               configuration : config_h,
> > +               install_dir : get_option('bindir'))
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Laurent Pinchart Feb. 1, 2022, 10:37 a.m. UTC | #3
Hi Kieran,

On Tue, Feb 01, 2022 at 10:26:06AM +0000, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2022-02-01 07:20:52)
> > On Mon, Jan 31, 2022 at 09:07:57PM +0000, Kieran Bingham wrote:
> > > Support easier usage of the v4l2 compatibility layer with a script that
> > 
> > s/v4l2/V4L2/
> > 
> > > handles the LD_PRELOAD for applications.
> > > 
> > > The wrapper can be prefixed to launch any application with the preload
> > > set:
> > > 
> > >  $ libcamerify v4l2-ctl --list-devices
> > >  \_SB_.PCI0.GP13.XHC0.RHUB.PRT4- (libcamera:0):
> > >       /dev/video0
> > > 
> > >  platform/vimc.0 Sensor B (libcamera:1):
> > >       /dev/video2
> > >       /dev/video3
> > >       /dev/video4
> > > 
> > > Specifying '-d' once before the command to run will enable V4L2Compat
> > > layer debug output from libcamera.
> > > 
> > > Specifying '-d' twice will enable all debug levels from all libcamera
> > > components and provide a very detailed log for analysis.
> > > 
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > ---
> > > v2:
> > >  - rename to libcamerify (from libcamera-v4l2)
> > >  - add extending support for setting debug log levels.
> > > 
> > > 
> > >  src/v4l2/libcamerify.in | 34 ++++++++++++++++++++++++++++++++++
> > >  src/v4l2/meson.build    | 14 ++++++++++++++
> > >  2 files changed, 48 insertions(+)
> > >  create mode 100755 src/v4l2/libcamerify.in
> > > 
> > > diff --git a/src/v4l2/libcamerify.in b/src/v4l2/libcamerify.in
> > > new file mode 100755
> > > index 000000000000..161f5e49190e
> > > --- /dev/null
> > > +++ b/src/v4l2/libcamerify.in
> > > @@ -0,0 +1,34 @@
> > > +#!/bin/sh
> > 
> > License ?
> > 
> > > +
> > > +if [ "$LD_PRELOAD" = "" ] ; then
> > > +   LD_PRELOAD='@LIBCAMERA_V4L2_SO@'
> > > +else
> > > +   LD_PRELOAD="$LD_PRELOAD "'@LIBCAMERA_V4L2_SO@'
> > > +fi
> > > +
> > > +export LD_PRELOAD
> > 
> > Can you move this just before the exec call, to avoid LD_PRELOADing the
> > library in other commands executed by thie script ?
> 
> Eeep yes. How did I miss that.
> 
> > > +
> > > +help() {
> > > +     echo "$0: Load an application with libcamera V4L2 compatibility layer preload"
> > > +     echo " $0 [OPTIONS...] executable [args]"
> > > +     echo " -d, --debug      Increase log level"
> > > +     exit 1;
> > > +}
> > > +
> > > +debug=0
> > > +while [ $# -gt 0 ]; do
> > > +     case $1 in
> > > +             -d|--debug) debug=$((debug+1));;
> > > +             -h) help; break;;
> > 
> > "break" should be "exit 0".
> 
> help() was already calling exit 1;, but yes, I can move that in here.

Ah yes I didn't notice. Do applications typically return an error status
with -h ("exit 1") or a success status ("exit 0") ? I usually code the
latter, but I may be wrong.

> > > +             --) shift; break;;
> > > +             -*) echo "Unrecognised option: $1"; help; break;;
> > 
> > Same here.

Here it should be "exit 1".

> > > +             *) break;;
> > 
> > I'd find this more readable:
> > 
> >                 -d|--debug)
> >                         debug=$((debug+1))
> >                         ;;
> >                 -h)
> >                         help
> >                         break
> >                         ;;
> >                 --)
> >                         shift
> >                         break
> >                         ;;
> >                 -*)
> >                         echo "Unrecognised option: $1"
> >                         help
> >                         break
> >                         ;;
> >                 *)
> >                         break
> >                         ;;
> > 
> 
> sure
> 
> > > +     esac
> > > +     shift
> > > +done
> > > +
> > > +[ $debug -gt 0 ] && loglevel=V4L2Compat:0
> > > +[ $debug -gt 1 ] && loglevel=0
> > > +[ "$loglevel" != "" ] && export LIBCAMERA_LOG_LEVELS=$loglevel
> > > +
> > > +exec "$@"
> > > diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
> > > index f78497b6799b..07ff9a6939d2 100644
> > > --- a/src/v4l2/meson.build
> > > +++ b/src/v4l2/meson.build
> > > @@ -33,3 +33,17 @@ v4l2_compat = shared_library('v4l2-compat',
> > >                               install : true,
> > >                               dependencies : [libcamera_private, libdl],
> > >                               cpp_args : v4l2_compat_cpp_args)
> > > +
> > > +# Provide a wrapper script to support easily loading applications with the V4L2
> > > +# adaptation layer
> > > +
> > > +config_h.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')
> > 
> > I don't think this should be added to config.h. Can you create a
> > separate variable ?
> > 
> > cdata = configuration_data()
> > cdata.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')
> > 
> > I also wonder if we could use v4l2_compat.name() instead of
> > 'v4l2-compat.so' (but that's only available starting in meson 0.54).
> 
> I also asked in IRC #meson about this, hoping I could get a binary
> target to tell me it's full installation path but alas...
> 
> > > +
> > > +summary({
> > > +    'LIBCAMERA_V4L2_SO' : config_h.get('LIBCAMERA_V4L2_SO'),
> > > +}, section : 'Paths')
> > 
> > I would have skipped this. It's a bit out of place, with all the other
> > paths being directories.
> 
> But it's also a path/file that (at least without this script) people need to
> know, so that tehy can do LD_PRELOAD= themselves.
> 
> I can drop it though, as I expect this script will help exactly those
> users.

That was my thinking too, as there's now a script, the binary path is
less important.

> > > +
> > > +configure_file(input : 'libcamerify.in',
> > > +               output : 'libcamerify',
> > > +               configuration : config_h,
> > > +               install_dir : get_option('bindir'))

Patch
diff mbox series

diff --git a/src/v4l2/libcamerify.in b/src/v4l2/libcamerify.in
new file mode 100755
index 000000000000..161f5e49190e
--- /dev/null
+++ b/src/v4l2/libcamerify.in
@@ -0,0 +1,34 @@ 
+#!/bin/sh
+
+if [ "$LD_PRELOAD" = "" ] ; then
+   LD_PRELOAD='@LIBCAMERA_V4L2_SO@'
+else
+   LD_PRELOAD="$LD_PRELOAD "'@LIBCAMERA_V4L2_SO@'
+fi
+
+export LD_PRELOAD
+
+help() {
+	echo "$0: Load an application with libcamera V4L2 compatibility layer preload"
+	echo " $0 [OPTIONS...] executable [args]"
+	echo " -d, --debug	Increase log level"
+	exit 1;
+}
+
+debug=0
+while [ $# -gt 0 ]; do
+	case $1 in
+		-d|--debug) debug=$((debug+1));;
+		-h) help; break;;
+		--) shift; break;;
+		-*) echo "Unrecognised option: $1"; help; break;;
+		*) break;;
+	esac
+	shift
+done
+
+[ $debug -gt 0 ] && loglevel=V4L2Compat:0
+[ $debug -gt 1 ] && loglevel=0
+[ "$loglevel" != "" ] && export LIBCAMERA_LOG_LEVELS=$loglevel
+
+exec "$@"
diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
index f78497b6799b..07ff9a6939d2 100644
--- a/src/v4l2/meson.build
+++ b/src/v4l2/meson.build
@@ -33,3 +33,17 @@  v4l2_compat = shared_library('v4l2-compat',
                              install : true,
                              dependencies : [libcamera_private, libdl],
                              cpp_args : v4l2_compat_cpp_args)
+
+# Provide a wrapper script to support easily loading applications with the V4L2
+# adaptation layer
+
+config_h.set('LIBCAMERA_V4L2_SO', get_option('prefix') / get_option('libdir') / 'v4l2-compat.so')
+
+summary({
+    'LIBCAMERA_V4L2_SO' : config_h.get('LIBCAMERA_V4L2_SO'),
+}, section : 'Paths')
+
+configure_file(input : 'libcamerify.in',
+               output : 'libcamerify',
+               configuration : config_h,
+               install_dir : get_option('bindir'))