diff mbox

propagating controls in libv4l2 was Re: support autofocus / autogain in libv4l2

Message ID 20170426105300.GA857@amd (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Machek April 26, 2017, 10:53 a.m. UTC
Hi!

> > > IMO, the best place for autofocus is at libv4l2. Putting it on a
> > > separate "video server" application looks really weird for me.  
> > 
> > Well... let me see. libraries are quite limited -- it is hard to open
> > files, or use threads/have custom main loop. It may be useful to
> > switch resolutions -- do autofocus/autogain at lower resolution, then
> > switch to high one for taking picture. It would be good to have that
> > in "system" code, but I'm not at all sure libv4l2 design will allow
> > that.
> 
> I don't see why it would be hard to open files or have threads inside
> a library. There are several libraries that do that already, specially
> the ones designed to be used on multimidia apps.

Well, This is what the libv4l2 says:

   This file implements libv4l2, which offers v4l2_ prefixed versions
   of
      open/close/etc. The API is 100% the same as directly opening
   /dev/videoX
      using regular open/close/etc, the big difference is that format
   conversion
   
but if I open additional files in v4l2_open(), API is no longer the
same, as unix open() is defined to open just one file descriptor.

Now. There is autogain support in libv4lconvert, but it expects to use
same fd for camera and for the gain... which does not work with
subdevs.

Of course, opening subdevs by name like this is not really
acceptable. But can you suggest a method that is?

Thanks,
								Pavel

commit 4cf9d10ead014c0db25452e4bb9cd144632407c3
Author: Pavel <pavel@ucw.cz>
Date:   Wed Apr 26 11:38:04 2017 +0200

    Add subdevices.

Comments

Mauro Carvalho Chehab April 26, 2017, 11:13 a.m. UTC | #1
Hi Pavel,

Em Wed, 26 Apr 2017 12:53:00 +0200
Pavel Machek <pavel@ucw.cz> escreveu:

> Hi!
> 
> > > > IMO, the best place for autofocus is at libv4l2. Putting it on a
> > > > separate "video server" application looks really weird for me.    
> > > 
> > > Well... let me see. libraries are quite limited -- it is hard to open
> > > files, or use threads/have custom main loop. It may be useful to
> > > switch resolutions -- do autofocus/autogain at lower resolution, then
> > > switch to high one for taking picture. It would be good to have that
> > > in "system" code, but I'm not at all sure libv4l2 design will allow
> > > that.  
> > 
> > I don't see why it would be hard to open files or have threads inside
> > a library. There are several libraries that do that already, specially
> > the ones designed to be used on multimidia apps.  
> 
> Well, This is what the libv4l2 says:
> 
>    This file implements libv4l2, which offers v4l2_ prefixed versions
>    of
>       open/close/etc. The API is 100% the same as directly opening
>    /dev/videoX
>       using regular open/close/etc, the big difference is that format
>    conversion
>    
> but if I open additional files in v4l2_open(), API is no longer the
> same, as unix open() is defined to open just one file descriptor.
> 
> Now. There is autogain support in libv4lconvert, but it expects to use
> same fd for camera and for the gain... which does not work with
> subdevs.
> 
> Of course, opening subdevs by name like this is not really
> acceptable. But can you suggest a method that is?

There are two separate things here:

1) Autofoucs for a device that doesn't use subdev API
2) libv4l2 support for devices that require MC and subdev API

for (1), it should use the /dev/videoX device that was opened with
v4l2_open().

For (2), libv4l2 should be aware of MC and subdev APIs. Sakari
once tried to write a libv4l2 plugin for OMAP3, but never finished it.
A more recent trial were to add a libv4l2 plugin for Exynos.
Unfortunately, none of those code got merged. Last time I checked,
the Exynos plugin was almost ready to be merged, but Sakari asked
some changes on it. The developer that was working on it got job on
some other company. Last time I heard from him, he was still interested
on finishing his work, but in the need to setup a test environment
using his own devices.

So, currently, there's no code at all adding MC/subdev API
support merged at libv4l2.

-

IMHO, the right thing to do with regards to autofocus is to
implement it via a processing module, assuming that just one
video device is opened.

Then, add a N900 plugin to make libv4l2 aware of OMAP3 specifics.

After that, rework at the processing module to let it use a 
different file descriptor if such plugin is in usage.

-

The hole idea is that a libv4l2 client, running on a N900 device
would just open a fake /dev/video0. Internally, libv4l2 will
open whatever video nodes it needs to control the device, exporting
all hardware capabilities (video formats, controls, resolutions,
etc) as if it was a normal V4L2 camera, hiding all dirty details
about MC and subdev APIs from userspace application.

This way, a normal application, like xawtv, tvtime, camorama,
zbar, mplayer, vlc, ... will work without any changes.


> 
> Thanks,
> 								Pavel
> 
> commit 4cf9d10ead014c0db25452e4bb9cd144632407c3
> Author: Pavel <pavel@ucw.cz>
> Date:   Wed Apr 26 11:38:04 2017 +0200
> 
>     Add subdevices.
> 
> diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
> index 343db5e..a6bc48e 100644
> --- a/lib/libv4l2/libv4l2-priv.h
> +++ b/lib/libv4l2/libv4l2-priv.h
> @@ -26,6 +26,7 @@
>  #include "../libv4lconvert/libv4lsyscall-priv.h"
>  
>  #define V4L2_MAX_DEVICES 16
> +#define V4L2_MAX_SUBDEVS 8
>  /* Warning when making this larger the frame_queued and frame_mapped members of
>     the v4l2_dev_info struct can no longer be a bitfield, so the code needs to
>     be adjusted! */
> @@ -104,6 +105,7 @@ struct v4l2_dev_info {
>  	void *plugin_library;
>  	void *dev_ops_priv;
>  	const struct libv4l_dev_ops *dev_ops;
> +        int subdev_fds[V4L2_MAX_SUBDEVS];
>  };
>  
>  /* From v4l2-plugin.c */
> diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
> index 0ba0a88..edc9642 100644
> --- a/lib/libv4l2/libv4l2.c
> +++ b/lib/libv4l2/libv4l2.c
> @@ -1,3 +1,4 @@
> +/* -*- c-file-style: "linux" -*- */
>  /*
>  #             (C) 2008 Hans de Goede <hdegoede@redhat.com>
>  
> @@ -789,18 +790,25 @@ no_capture:
>  
>  	/* Note we always tell v4lconvert to optimize src fmt selection for
>  	   our default fps, the only exception is the app explicitly selecting
> -	   a fram erate using the S_PARM ioctl after a S_FMT */
> +	   a frame rate using the S_PARM ioctl after a S_FMT */
>  	if (devices[index].convert)
>  		v4lconvert_set_fps(devices[index].convert, V4L2_DEFAULT_FPS);
>  	v4l2_update_fps(index, &parm);
>  
> +	devices[index].subdev_fds[0] = SYS_OPEN("/dev/video_sensor", O_RDWR, 0);
> +	devices[index].subdev_fds[1] = SYS_OPEN("/dev/video_focus", O_RDWR, 0);
> +	devices[index].subdev_fds[2] = -1;
> +
> +	printf("Sensor: %d, focus: %d\n", devices[index].subdev_fds[0], 
> +	       devices[index].subdev_fds[1]);
> +
>  	V4L2_LOG("open: %d\n", fd);
>  
>  	return fd;
>  }
>  
>  /* Is this an fd for which we are emulating v4l1 ? */
> -static int v4l2_get_index(int fd)
> +int v4l2_get_index(int fd)
>  {
>  	int index;
>  
> 
> commit 1d6a9ce121f53e8f2e38549eed597a3c3dea5233
> Author: Pavel <pavel@ucw.cz>
> Date:   Wed Apr 26 12:34:04 2017 +0200
> 
>     Enable ioctl propagation.
> 
> diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
> index edc9642..6dab661 100644
> --- a/lib/libv4l2/libv4l2.c
> +++ b/lib/libv4l2/libv4l2.c
> @@ -1064,6 +1064,23 @@ static int v4l2_s_fmt(int index, struct v4l2_format *dest_fmt)
>  	return 0;
>  }
>  
> +static int v4l2_propagate_ioctl(int index, unsigned long request, void *arg)
> +{
> +	int i = 0;
> +	int result;
> +	while (1) {
> +		if (devices[index].subdev_fds[i] == -1)
> +			return -1;
> +		printf("g_ctrl failed, trying...\n");
> +		result = SYS_IOCTL(devices[index].subdev_fds[i], request, arg);
> +		printf("subdev %d result %d\n", i, result);
> +		if (result == 0)
> +			return 0;
> +		i++;
> +	}
> +	return -1;
> +}
> +
>  int v4l2_ioctl(int fd, unsigned long int request, ...)
>  {
>  	void *arg;
> @@ -1193,14 +1210,20 @@ no_capture_request:
>  	switch (request) {
>  	case VIDIOC_QUERYCTRL:
>  		result = v4lconvert_vidioc_queryctrl(devices[index].convert, arg);
> +		if (result == -1)
> +			result = v4l2_propagate_ioctl(index, request, arg);
>  		break;
>  
>  	case VIDIOC_G_CTRL:
>  		result = v4lconvert_vidioc_g_ctrl(devices[index].convert, arg);
> +		if (result == -1)
> +			result = v4l2_propagate_ioctl(index, request, arg);
>  		break;
>  
>  	case VIDIOC_S_CTRL:
>  		result = v4lconvert_vidioc_s_ctrl(devices[index].convert, arg);
> +		if (result == -1)
> +			result = v4l2_propagate_ioctl(index, request, arg);
>  		break;
>  
>  	case VIDIOC_G_EXT_CTRLS:
> 
> 



Thanks,
Mauro
Mauro Carvalho Chehab April 26, 2017, 11:26 a.m. UTC | #2
Em Wed, 26 Apr 2017 12:53:00 +0200
Pavel Machek <pavel@ucw.cz> escreveu:

> Hi!
> 
> > > > IMO, the best place for autofocus is at libv4l2. Putting it on a
> > > > separate "video server" application looks really weird for me.    
> > > 
> > > Well... let me see. libraries are quite limited -- it is hard to open
> > > files, or use threads/have custom main loop. It may be useful to
> > > switch resolutions -- do autofocus/autogain at lower resolution, then
> > > switch to high one for taking picture. It would be good to have that
> > > in "system" code, but I'm not at all sure libv4l2 design will allow
> > > that.  
> > 
> > I don't see why it would be hard to open files or have threads inside
> > a library. There are several libraries that do that already, specially
> > the ones designed to be used on multimidia apps.  
> 
> Well, This is what the libv4l2 says:
> 
>    This file implements libv4l2, which offers v4l2_ prefixed versions
>    of
>       open/close/etc. The API is 100% the same as directly opening
>    /dev/videoX
>       using regular open/close/etc, the big difference is that format
>    conversion
>    
> but if I open additional files in v4l2_open(), API is no longer the
> same, as unix open() is defined to open just one file descriptor.
> 
> Now. There is autogain support in libv4lconvert, but it expects to use
> same fd for camera and for the gain... which does not work with
> subdevs.
> 
> Of course, opening subdevs by name like this is not really
> acceptable. But can you suggest a method that is?
> 
> Thanks,
> 								Pavel
> 
> commit 4cf9d10ead014c0db25452e4bb9cd144632407c3
> Author: Pavel <pavel@ucw.cz>
> Date:   Wed Apr 26 11:38:04 2017 +0200
> 
>     Add subdevices.
> 
> diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
> index 343db5e..a6bc48e 100644
> --- a/lib/libv4l2/libv4l2-priv.h
> +++ b/lib/libv4l2/libv4l2-priv.h
> @@ -26,6 +26,7 @@
>  #include "../libv4lconvert/libv4lsyscall-priv.h"
>  
>  #define V4L2_MAX_DEVICES 16
> +#define V4L2_MAX_SUBDEVS 8

Isn't it a short number?

>  /* Warning when making this larger the frame_queued and frame_mapped members of
>     the v4l2_dev_info struct can no longer be a bitfield, so the code needs to
>     be adjusted! */
> @@ -104,6 +105,7 @@ struct v4l2_dev_info {
>  	void *plugin_library;
>  	void *dev_ops_priv;
>  	const struct libv4l_dev_ops *dev_ops;
> +        int subdev_fds[V4L2_MAX_SUBDEVS];
>  };
>  
>  /* From v4l2-plugin.c */
> diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
> index 0ba0a88..edc9642 100644
> --- a/lib/libv4l2/libv4l2.c
> +++ b/lib/libv4l2/libv4l2.c
> @@ -1,3 +1,4 @@
> +/* -*- c-file-style: "linux" -*- */

No emacs comments, please.

>  /*
>  #             (C) 2008 Hans de Goede <hdegoede@redhat.com>
>  
> @@ -789,18 +790,25 @@ no_capture:
>  
>  	/* Note we always tell v4lconvert to optimize src fmt selection for
>  	   our default fps, the only exception is the app explicitly selecting
> -	   a fram erate using the S_PARM ioctl after a S_FMT */
> +	   a frame rate using the S_PARM ioctl after a S_FMT */
>  	if (devices[index].convert)
>  		v4lconvert_set_fps(devices[index].convert, V4L2_DEFAULT_FPS);
>  	v4l2_update_fps(index, &parm);
>  
> +	devices[index].subdev_fds[0] = SYS_OPEN("/dev/video_sensor", O_RDWR, 0);
> +	devices[index].subdev_fds[1] = SYS_OPEN("/dev/video_focus", O_RDWR, 0);
> +	devices[index].subdev_fds[2] = -1;

Hardcoding names here is not a good idea. Ideally, it should open
the MC, using the newgen API, and parse the media graph.

The problem is that, even with newgen API, without the properties API
you likely won't be able to write a generic parser. So, we need a
plugin specific for OMAP3 (or at least some database that would teach
a generic plugin about OMAP3 specifics).

I guess that the approach that Jacek was taken were very close to what
a generic plugin would need:
	https://lwn.net/Articles/619449/

The last version of his patch set is here:
	https://patchwork.linuxtv.org/patch/37496/

I didn't review his patchset, but from what I saw, Sakari is the one
that found some issues on v7.1 patchset.

Sakari,

Could you shed us a light about why this patchset was not merged?

Are there anything really bad at the code, or just minor issues that
could be fixed later?

If it is the last case, perhaps we could merge the code, if this
would make easier for Pavel to work on a N9 solution using the
same approach.


Thanks,
Mauro
Pavel Machek April 29, 2017, 9:19 a.m. UTC | #3
Hi!

> > +	devices[index].subdev_fds[0] = SYS_OPEN("/dev/video_sensor", O_RDWR, 0);
> > +	devices[index].subdev_fds[1] = SYS_OPEN("/dev/video_focus", O_RDWR, 0);
> > +	devices[index].subdev_fds[2] = -1;
> 
> Hardcoding names here is not a good idea. Ideally, it should open
> the MC, using the newgen API, and parse the media graph.
> 
> The problem is that, even with newgen API, without the properties API
> you likely won't be able to write a generic parser. So, we need a
> plugin specific for OMAP3 (or at least some database that would teach
> a generic plugin about OMAP3 specifics).
> 
> I guess that the approach that Jacek was taken were very close to what
> a generic plugin would need:
> 	https://lwn.net/Articles/619449/
> 
> The last version of his patch set is here:
> 	https://patchwork.linuxtv.org/patch/37496/
> 
> I didn't review his patchset, but from what I saw, Sakari is the one
> that found some issues on v7.1 patchset.
> 
> Sakari,
> 
> Could you shed us a light about why this patchset was not merged?
> 
> Are there anything really bad at the code, or just minor issues that
> could be fixed later?
> 
> If it is the last case, perhaps we could merge the code, if this
> would make easier for Pavel to work on a N9 solution using the
> same approach.

It would be nice to get some solution here. Camera without libv4l
support is pretty much useless :-(.

									Pavel
Pavel Machek July 13, 2017, 9:49 a.m. UTC | #4
Hi!

> > Now. There is autogain support in libv4lconvert, but it expects to use
> > same fd for camera and for the gain... which does not work with
> > subdevs.
> > 
> > Of course, opening subdevs by name like this is not really
> > acceptable. But can you suggest a method that is?
> 
> There are two separate things here:
> 
> 1) Autofoucs for a device that doesn't use subdev API
> 2) libv4l2 support for devices that require MC and subdev API
> 
> for (1), it should use the /dev/videoX device that was opened with
> v4l2_open().
> 
> For (2), libv4l2 should be aware of MC and subdev APIs. Sakari
> once tried to write a libv4l2 plugin for OMAP3, but never finished it.
> A more recent trial were to add a libv4l2 plugin for Exynos.
> Unfortunately, none of those code got merged. Last time I checked,
> the Exynos plugin was almost ready to be merged, but Sakari asked
> some changes on it. The developer that was working on it got job on
> some other company. Last time I heard from him, he was still interested
> on finishing his work, but in the need to setup a test environment
> using his own devices.

First... we should not have hardware-specific code in the
userspace. Hardware abstraction should be kernel's job.

Second... we really should solve "ioctl propagation" in
libv4l2. Because otherwise existing userspace is unusable in MC/subdev
drivers.

> IMHO, the right thing to do with regards to autofocus is to
> implement it via a processing module, assuming that just one
> video device is opened.

Ok, I have done that.

> The hole idea is that a libv4l2 client, running on a N900 device
> would just open a fake /dev/video0. Internally, libv4l2 will
> open whatever video nodes it needs to control the device, exporting
> all hardware capabilities (video formats, controls, resolutions,
> etc) as if it was a normal V4L2 camera, hiding all dirty details
> about MC and subdev APIs from userspace application.
> 
> This way, a normal application, like xawtv, tvtime, camorama,
> zbar, mplayer, vlc, ... will work without any changes.

Well, yes, we'd like to get there eventually. But we are not there at
the moment, and ioctl() propagation is one of the steps.

(I do have support specific for N900, but currently it is in python;
this is enough to make the camera usable.)

Regards,
								Pavel

								
> > diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
> > index 343db5e..a6bc48e 100644
> > --- a/lib/libv4l2/libv4l2-priv.h
> > +++ b/lib/libv4l2/libv4l2-priv.h
> > @@ -26,6 +26,7 @@
> >  #include "../libv4lconvert/libv4lsyscall-priv.h"
> >  
> >  #define V4L2_MAX_DEVICES 16
> > +#define V4L2_MAX_SUBDEVS 8
> >  /* Warning when making this larger the frame_queued and frame_mapped members of
> >     the v4l2_dev_info struct can no longer be a bitfield, so the code needs to
> >     be adjusted! */
> > @@ -104,6 +105,7 @@ struct v4l2_dev_info {
> >  	void *plugin_library;
> >  	void *dev_ops_priv;
> >  	const struct libv4l_dev_ops *dev_ops;
> > +        int subdev_fds[V4L2_MAX_SUBDEVS];
> >  };
> >  
> >  /* From v4l2-plugin.c */
> > diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
> > index 0ba0a88..edc9642 100644
> > --- a/lib/libv4l2/libv4l2.c
> > +++ b/lib/libv4l2/libv4l2.c
> > @@ -1,3 +1,4 @@
> > +/* -*- c-file-style: "linux" -*- */
> >  /*
> >  #             (C) 2008 Hans de Goede <hdegoede@redhat.com>
> >  
> > @@ -789,18 +790,25 @@ no_capture:
> >  
> >  	/* Note we always tell v4lconvert to optimize src fmt selection for
> >  	   our default fps, the only exception is the app explicitly selecting
> > -	   a fram erate using the S_PARM ioctl after a S_FMT */
> > +	   a frame rate using the S_PARM ioctl after a S_FMT */
> >  	if (devices[index].convert)
> >  		v4lconvert_set_fps(devices[index].convert, V4L2_DEFAULT_FPS);
> >  	v4l2_update_fps(index, &parm);
> >  
> > +	devices[index].subdev_fds[0] = SYS_OPEN("/dev/video_sensor", O_RDWR, 0);
> > +	devices[index].subdev_fds[1] = SYS_OPEN("/dev/video_focus", O_RDWR, 0);
> > +	devices[index].subdev_fds[2] = -1;
> > +
> > +	printf("Sensor: %d, focus: %d\n", devices[index].subdev_fds[0], 
> > +	       devices[index].subdev_fds[1]);
> > +
> >  	V4L2_LOG("open: %d\n", fd);
> >  
> >  	return fd;
> >  }
> >  
> >  /* Is this an fd for which we are emulating v4l1 ? */
> > -static int v4l2_get_index(int fd)
> > +int v4l2_get_index(int fd)
> >  {
> >  	int index;
> >  
> > 
> > commit 1d6a9ce121f53e8f2e38549eed597a3c3dea5233
> > Author: Pavel <pavel@ucw.cz>
> > Date:   Wed Apr 26 12:34:04 2017 +0200
> > 
> >     Enable ioctl propagation.
> > 
> > diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
> > index edc9642..6dab661 100644
> > --- a/lib/libv4l2/libv4l2.c
> > +++ b/lib/libv4l2/libv4l2.c
> > @@ -1064,6 +1064,23 @@ static int v4l2_s_fmt(int index, struct v4l2_format *dest_fmt)
> >  	return 0;
> >  }
> >  
> > +static int v4l2_propagate_ioctl(int index, unsigned long request, void *arg)
> > +{
> > +	int i = 0;
> > +	int result;
> > +	while (1) {
> > +		if (devices[index].subdev_fds[i] == -1)
> > +			return -1;
> > +		printf("g_ctrl failed, trying...\n");
> > +		result = SYS_IOCTL(devices[index].subdev_fds[i], request, arg);
> > +		printf("subdev %d result %d\n", i, result);
> > +		if (result == 0)
> > +			return 0;
> > +		i++;
> > +	}
> > +	return -1;
> > +}
> > +
> >  int v4l2_ioctl(int fd, unsigned long int request, ...)
> >  {
> >  	void *arg;
> > @@ -1193,14 +1210,20 @@ no_capture_request:
> >  	switch (request) {
> >  	case VIDIOC_QUERYCTRL:
> >  		result = v4lconvert_vidioc_queryctrl(devices[index].convert, arg);
> > +		if (result == -1)
> > +			result = v4l2_propagate_ioctl(index, request, arg);
> >  		break;
> >  
> >  	case VIDIOC_G_CTRL:
> >  		result = v4lconvert_vidioc_g_ctrl(devices[index].convert, arg);
> > +		if (result == -1)
> > +			result = v4l2_propagate_ioctl(index, request, arg);
> >  		break;
> >  
> >  	case VIDIOC_S_CTRL:
> >  		result = v4lconvert_vidioc_s_ctrl(devices[index].convert, arg);
> > +		if (result == -1)
> > +			result = v4l2_propagate_ioctl(index, request, arg);
> >  		break;
> >  
> >  	case VIDIOC_G_EXT_CTRLS:
> > 
> > 
> 
> 
> 
> Thanks,
> Mauro
Hans Verkuil Oct. 22, 2017, 7:36 a.m. UTC | #5
On 22/10/17 00:00, Pavel Machek wrote:
> Hi!
> 
> I'd still like to get some reasonable support for cellphone camera in
> Linux.
> 
> IMO first reasonable step is to merge sdlcam, then we can implement
> autofocus, improve autogain... and rest of the boring stuff. Ouch and
> media graph support would be nice. Currently, _nothing_ works with
> media graph device, such as N900.

Can you post your latest rebased patch for sdlcam for v4l-utils?

I'll do a review and will likely merge it for you. Yes, I've changed my
mind on that.

> 
> I'll talk about the issues at ELCE in few days:
> 
> https://osseu17.sched.com/event/ByYH/cheap-complex-cameras-pavel-machek-denx-software-engineering-gmbh
> 
> Will someone else be there? Is there some place where v4l people meet?

Why don't we discuss this Tuesday morning at 9am? I have no interest in the
keynotes on that day, so those who are interested can get together.

I'll be at your presentation tomorrow and we can discuss a bit during
the following coffee break if time permits.

Regards,

	Hans
Pavel Machek Oct. 22, 2017, 8:31 a.m. UTC | #6
Hi!

> > I'd still like to get some reasonable support for cellphone camera in
> > Linux.
> > 
> > IMO first reasonable step is to merge sdlcam, then we can implement
> > autofocus, improve autogain... and rest of the boring stuff. Ouch and
> > media graph support would be nice. Currently, _nothing_ works with
> > media graph device, such as N900.
> 
> Can you post your latest rebased patch for sdlcam for v4l-utils?
> 
> I'll do a review and will likely merge it for you. Yes, I've changed my
> mind on that.

Ok, will do, thanks!

> > I'll talk about the issues at ELCE in few days:
> > 
> > https://osseu17.sched.com/event/ByYH/cheap-complex-cameras-pavel-machek-denx-software-engineering-gmbh
> > 
> > Will someone else be there? Is there some place where v4l people meet?
> 
> Why don't we discuss this Tuesday morning at 9am? I have no interest in the
> keynotes on that day, so those who are interested can get together.
> 
> I'll be at your presentation tomorrow and we can discuss a bit during
> the following coffee break if time permits.

Ok, sounds like a plan. Lets confirm Tuesday morning tommorow.

Thanks,
									Pavel
Pavel Machek Oct. 23, 2017, 6:54 p.m. UTC | #7
Hi!

> > I'll talk about the issues at ELCE in few days:
> > 
> > https://osseu17.sched.com/event/ByYH/cheap-complex-cameras-pavel-machek-denx-software-engineering-gmbh
> > 
> > Will someone else be there? Is there some place where v4l people meet?
> 
> Why don't we discuss this Tuesday morning at 9am? I have no interest in the
> keynotes on that day, so those who are interested can get together.

Ok, what about 9:30am? The place near the elevators where we were
talking last time?

And.... sorry for late reply.
									Pavel
Hans Verkuil Oct. 23, 2017, 7:24 p.m. UTC | #8
Sounds good. That's the elevators on level LL by the way.

Regards,

Hans

On October 23, 2017 8:54:49 PM GMT+02:00, Pavel Machek <pavel@ucw.cz> wrote:
>Hi!
>
>> > I'll talk about the issues at ELCE in few days:
>> > 
>> >
>https://osseu17.sched.com/event/ByYH/cheap-complex-cameras-pavel-machek-denx-software-engineering-gmbh
>> > 
>> > Will someone else be there? Is there some place where v4l people
>meet?
>> 
>> Why don't we discuss this Tuesday morning at 9am? I have no interest
>in the
>> keynotes on that day, so those who are interested can get together.
>
>Ok, what about 9:30am? The place near the elevators where we were
>talking last time?
>
>And.... sorry for late reply.
>									Pavel
Sakari Ailus Oct. 23, 2017, 8:15 p.m. UTC | #9
On Mon, Oct 23, 2017 at 09:24:24PM +0200, Hans Verkuil wrote:
> Sounds good. That's the elevators on level LL by the way.

I'll be there, too!
Mauro Carvalho Chehab Oct. 23, 2017, 9:02 p.m. UTC | #10
Em Mon, 23 Oct 2017 23:15:57 +0300
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> On Mon, Oct 23, 2017 at 09:24:24PM +0200, Hans Verkuil wrote:
> > Sounds good. That's the elevators on level LL by the way.  
> 
> I'll be there, too!
> 

I'll try to join you there too.


Cheers,
Mauro
Pavel Machek Nov. 1, 2017, 6:36 a.m. UTC | #11
Hi!

> Sakari, I am actually playing with N9 camera, not N950. That comes
> next.
> 
> And the clock error I mentioned ... seems to be
> -EPROBE_DEFER. So... not an issue.

Hmm, and with similar config, I got N950 to work. ... which should
give me enough clues to get N9 to work. I guess I forgot to reset the
pipeline between the tries, or something.

For the record, this got me some data on n950:

 m.media_ctl( [ '-f', '"OMAP3 ISP CSI2a":0 [fmt:%s/%dx%d]' % (m.fmt, m.cap_x, m.cap_y) ] )
 m.media_ctl( [ '-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]' ] )

 # WORKS!!!!
 # pavel@n900:~/g/tui/camera$ sudo /my/tui/yavta/yavta
 # --capture=8 --skip 0 --format SGRBG10 --size 4272x3016 /dev/video1 --file=/tmp/delme#

...ouch. It only worked twice :-(. Either driver gets confused by my
attempts, or it relied on some other initialization code. Strange.

Best regards,
									Pavel
Pavel Machek Nov. 1, 2017, 3:32 p.m. UTC | #12
Hi!

> > Sakari, I am actually playing with N9 camera, not N950. That comes
> > next.
> > 
> > And the clock error I mentioned ... seems to be
> > -EPROBE_DEFER. So... not an issue.
> 
> Hmm, and with similar config, I got N950 to work. ... which should
> give me enough clues to get N9 to work. I guess I forgot to reset the
> pipeline between the tries, or something.
> 
> For the record, this got me some data on n950:
> 
>  m.media_ctl( [ '-f', '"OMAP3 ISP CSI2a":0 [fmt:%s/%dx%d]' % (m.fmt, m.cap_x, m.cap_y) ] )
>  m.media_ctl( [ '-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]' ] )
> 
>  # WORKS!!!!
>  # pavel@n900:~/g/tui/camera$ sudo /my/tui/yavta/yavta
>  # --capture=8 --skip 0 --format SGRBG10 --size 4272x3016 /dev/video1 --file=/tmp/delme#
> 
> ...ouch. It only worked twice :-(. Either driver gets confused by my
> attempts, or it relied on some other initialization code. Strange.

Hmm, so it works "reliably" after boot. But it also locks up machine
with high probability. If you have N950, it might still be handy...

Messages are:

['-r']
['-f', '"OMAP3 ISP CSI2a":0 [fmt:SGRBG10/4272x3016]']
Warning: the -f option is deprecated and has been replaced by -V.
['-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]']
Testing: is raw
Testing:  /my/tui/yavta/yavta --capture=8 --skip 0 --format SGRBG10
--size 4272x3016 /dev/video_sensor --file=/tmp/delme#
Error opening device /dev/video_sensor: Permission denied (13).
Raw mode is  1
Raw mode is  1
Testing: is raw
Testing:  /my/tui/yavta/yavta --capture=8 --skip 0 --format SGRBG10
--size 4272x3016 /dev/video_sensor --file=/tmp/delme#
Device /dev/video_sensor opened.
Device `OMAP3 ISP CSI2a output' on `media' is a video capture (without
mplanes) device.
Video format set: SGRBG10 (30314142) 4272x3016 (stride 8544) field
none buffer size 25768704
Video format: SGRBG10 (30314142) 4272x3016 (stride 8544) field none
buffer size 25768704
1 buffers requested.
length: 25768704 offset: 0 timestamp type/source: mono/EoF
Buffer 0/0 mapped at address 0xb54e9000.
0 (0) [-] none 0 25768704 B 691.756907 691.758403 6.212 fps ts
mono/EoF
1 (0) [-] none 1 25768704 B 694.821025 694.821422 0.326 fps ts
mono/EoF
2 (0) [E] none 2 25768704 B 698.530833 698.534739 0.270 fps ts
mono/EoF
3 (0) [-] none 3 25768704 B 700.788127 700.790996 0.443 fps ts
mono/EoF
(and here it locked up :-(. Sometimes it captures more.)

Thanks,

								Pavel
diff mbox

Patch

diff --git a/lib/libv4l2/libv4l2-priv.h b/lib/libv4l2/libv4l2-priv.h
index 343db5e..a6bc48e 100644
--- a/lib/libv4l2/libv4l2-priv.h
+++ b/lib/libv4l2/libv4l2-priv.h
@@ -26,6 +26,7 @@ 
 #include "../libv4lconvert/libv4lsyscall-priv.h"
 
 #define V4L2_MAX_DEVICES 16
+#define V4L2_MAX_SUBDEVS 8
 /* Warning when making this larger the frame_queued and frame_mapped members of
    the v4l2_dev_info struct can no longer be a bitfield, so the code needs to
    be adjusted! */
@@ -104,6 +105,7 @@  struct v4l2_dev_info {
 	void *plugin_library;
 	void *dev_ops_priv;
 	const struct libv4l_dev_ops *dev_ops;
+        int subdev_fds[V4L2_MAX_SUBDEVS];
 };
 
 /* From v4l2-plugin.c */
diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
index 0ba0a88..edc9642 100644
--- a/lib/libv4l2/libv4l2.c
+++ b/lib/libv4l2/libv4l2.c
@@ -1,3 +1,4 @@ 
+/* -*- c-file-style: "linux" -*- */
 /*
 #             (C) 2008 Hans de Goede <hdegoede@redhat.com>
 
@@ -789,18 +790,25 @@  no_capture:
 
 	/* Note we always tell v4lconvert to optimize src fmt selection for
 	   our default fps, the only exception is the app explicitly selecting
-	   a fram erate using the S_PARM ioctl after a S_FMT */
+	   a frame rate using the S_PARM ioctl after a S_FMT */
 	if (devices[index].convert)
 		v4lconvert_set_fps(devices[index].convert, V4L2_DEFAULT_FPS);
 	v4l2_update_fps(index, &parm);
 
+	devices[index].subdev_fds[0] = SYS_OPEN("/dev/video_sensor", O_RDWR, 0);
+	devices[index].subdev_fds[1] = SYS_OPEN("/dev/video_focus", O_RDWR, 0);
+	devices[index].subdev_fds[2] = -1;
+
+	printf("Sensor: %d, focus: %d\n", devices[index].subdev_fds[0], 
+	       devices[index].subdev_fds[1]);
+
 	V4L2_LOG("open: %d\n", fd);
 
 	return fd;
 }
 
 /* Is this an fd for which we are emulating v4l1 ? */
-static int v4l2_get_index(int fd)
+int v4l2_get_index(int fd)
 {
 	int index;
 

commit 1d6a9ce121f53e8f2e38549eed597a3c3dea5233
Author: Pavel <pavel@ucw.cz>
Date:   Wed Apr 26 12:34:04 2017 +0200

    Enable ioctl propagation.

diff --git a/lib/libv4l2/libv4l2.c b/lib/libv4l2/libv4l2.c
index edc9642..6dab661 100644
--- a/lib/libv4l2/libv4l2.c
+++ b/lib/libv4l2/libv4l2.c
@@ -1064,6 +1064,23 @@  static int v4l2_s_fmt(int index, struct v4l2_format *dest_fmt)
 	return 0;
 }
 
+static int v4l2_propagate_ioctl(int index, unsigned long request, void *arg)
+{
+	int i = 0;
+	int result;
+	while (1) {
+		if (devices[index].subdev_fds[i] == -1)
+			return -1;
+		printf("g_ctrl failed, trying...\n");
+		result = SYS_IOCTL(devices[index].subdev_fds[i], request, arg);
+		printf("subdev %d result %d\n", i, result);
+		if (result == 0)
+			return 0;
+		i++;
+	}
+	return -1;
+}
+
 int v4l2_ioctl(int fd, unsigned long int request, ...)
 {
 	void *arg;
@@ -1193,14 +1210,20 @@  no_capture_request:
 	switch (request) {
 	case VIDIOC_QUERYCTRL:
 		result = v4lconvert_vidioc_queryctrl(devices[index].convert, arg);
+		if (result == -1)
+			result = v4l2_propagate_ioctl(index, request, arg);
 		break;
 
 	case VIDIOC_G_CTRL:
 		result = v4lconvert_vidioc_g_ctrl(devices[index].convert, arg);
+		if (result == -1)
+			result = v4l2_propagate_ioctl(index, request, arg);
 		break;
 
 	case VIDIOC_S_CTRL:
 		result = v4lconvert_vidioc_s_ctrl(devices[index].convert, arg);
+		if (result == -1)
+			result = v4l2_propagate_ioctl(index, request, arg);
 		break;
 
 	case VIDIOC_G_EXT_CTRLS: