Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1630041 > unrolled thread

Re: support autofocus / autogain in libv4l2

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: support autofocus / autogain in libv4l2 Pavel Machek <pavel@ucw.cz> - 2017-04-24 23:30 +0200
    Re: support autofocus / autogain in libv4l2 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-25 03:50 +0200
      Re: support autofocus / autogain in libv4l2 Pavel Machek <pavel@ucw.cz> - 2017-04-25 10:10 +0200
        Re: support autofocus / autogain in libv4l2 Pali Rohár <pali.rohar@gmail.com> - 2017-04-25 10:20 +0200
          Re: support autofocus / autogain in libv4l2 Pavel Machek <pavel@ucw.cz> - 2017-04-25 13:30 +0200
            Re: support autofocus / autogain in libv4l2 Pali Rohár <pali.rohar@gmail.com> - 2017-04-25 13:40 +0200
              Re: support autofocus / autogain in libv4l2 Pavel Machek <pavel@ucw.cz> - 2017-04-25 14:30 +0200
                Re: support autofocus / autogain in libv4l2 Pali Rohár <pali.rohar@gmail.com> - 2017-04-25 15:00 +0200
              Re: support autofocus / autogain in libv4l2 Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-04-25 19:00 +0200
          Re: support autofocus / autogain in libv4l2 Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-04-25 19:00 +0200
        Re: support autofocus / autogain in libv4l2 Nicolas Dufresne <nicolas@ndufresne.ca> - 2017-04-25 19:00 +0200
          Re: support autofocus / autogain in libv4l2 Pavel Machek <pavel@ucw.cz> - 2017-04-26 13:00 +0200
      [patch] propagating controls in libv4l2 was Re: support autofocus /  autogain in libv4l2 Pavel Machek <pavel@ucw.cz> - 2017-04-26 13:00 +0200
        Re: [patch] propagating controls in libv4l2 was Re: support  autofocus / autogain in libv4l2 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-26 13:20 +0200
          [patch] autogain support for bayer10 format (was Re: [patch]  propagating controls in libv4l2) Pavel Machek <pavel@ucw.cz> - 2017-04-26 15:30 +0200
            Re: [patch] autogain support for bayer10 format (was Re: [patch]  propagating controls in libv4l2) Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2017-04-26 17:50 +0200
              Re: [patch] autogain support for bayer10 format (was Re: [patch]  propagating controls in libv4l2) Pavel Machek <pavel@ucw.cz> - 2017-04-27 01:00 +0200
                Re: [patch] autogain support for bayer10 format (was Re: [patch]  propagating controls in libv4l2) Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> - 2017-04-27 08:00 +0200
              Re: [patch] autogain support for bayer10 format (was Re: [patch]  propagating controls in libv4l2) Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-05-03 21:10 +0200
                Re: [patch] autogain support for bayer10 format (was Re: [patch]  propagating controls in libv4l2) Pavel Machek <pavel@ucw.cz> - 2017-05-03 22:00 +0200
            Re: [patch] autogain support for bayer10 format (was Re: [patch]  propagating controls in libv4l2) Pavel Machek <pavel@ucw.cz> - 2017-05-01 00:50 +0200
        Re: [patch] propagating controls in libv4l2 was Re: support  autofocus / autogain in libv4l2 Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-04-26 13:30 +0200
          Re: [patch] propagating controls in libv4l2 was Re: support  autofocus / autogain in libv4l2 Pavel Machek <pavel@ucw.cz> - 2017-04-29 11:20 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1633504 — Re: [patch] autogain support for bayer10 format (was Re: [patch] propagating controls in libv4l2)

[Multipart message — attachments visible in raw view] — view raw

On Wed 2017-04-26 15:23:37, Pavel Machek wrote:
> Hi!
> 
> > > > 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
> 
> Actually there are three: 0) autogain. Unfortunately, I need autogain
> first before autofocus has a chance...
> 
> And that means... bayer10 support for autogain.
> 
> Plus, I changed avg_lum to long long. Quick calculation tells me int
> could overflow with few megapixel sensor.
> 
> Oh, btw http://ytse.tricolour.net/docs/LowLightOptimization.html no
> longer works.

Can I get some comments here? Patch will need fixup (constants need
adjusting), but is style/design acceptable?

Thanks,
								Pavel

> diff --git a/lib/libv4lconvert/processing/autogain.c b/lib/libv4lconvert/processing/autogain.c
> index c6866d6..0b52d0f 100644
> --- a/lib/libv4lconvert/processing/autogain.c
> +++ b/lib/libv4lconvert/processing/autogain.c
> @@ -68,6 +71,41 @@ static void autogain_adjust(struct v4l2_queryctrl *ctrl, int *value,
>  	}
>  }
>  
> +static int get_luminosity_bayer10(uint16_t *buf, const struct v4l2_format *fmt)
> +{
> +	long long avg_lum = 0;
> +	int x, y;
> +	
> +	buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> +		fmt->fmt.pix.width / 4;
> +
> +	for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> +		for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> +			avg_lum += *buf++;
> +		buf += fmt->fmt.pix.bytesperline - fmt->fmt.pix.width / 2;
> +	}
> +	avg_lum /= fmt->fmt.pix.height * fmt->fmt.pix.width / 4;
> +	avg_lum /= 4;
> +	return avg_lum;
> +}
> +
> +static int get_luminosity_bayer8(unsigned char *buf, const struct v4l2_format *fmt)
> +{
> +	long long avg_lum = 0;
> +	int x, y;
> +	
> +	buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> +		fmt->fmt.pix.width / 4;
> +
> +	for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> +		for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> +			avg_lum += *buf++;
> +		buf += fmt->fmt.pix.bytesperline - fmt->fmt.pix.width / 2;
> +	}
> +	avg_lum /= fmt->fmt.pix.height * fmt->fmt.pix.width / 4;
> +	return avg_lum;
> +}
> +
>  /* auto gain and exposure algorithm based on the knee algorithm described here:
>  http://ytse.tricolour.net/docs/LowLightOptimization.html */
>  static int autogain_calculate_lookup_tables(
> @@ -100,17 +142,16 @@ static int autogain_calculate_lookup_tables(
>  	switch (fmt->fmt.pix.pixelformat) {
> +	case V4L2_PIX_FMT_SGBRG10:
> +	case V4L2_PIX_FMT_SGRBG10:
> +	case V4L2_PIX_FMT_SBGGR10:
> +	case V4L2_PIX_FMT_SRGGB10:
> +		avg_lum = get_luminosity_bayer10((void *) buf, fmt);
> +		break;
> +
>  	case V4L2_PIX_FMT_SGBRG8:
>  	case V4L2_PIX_FMT_SGRBG8:
>  	case V4L2_PIX_FMT_SBGGR8:
>  	case V4L2_PIX_FMT_SRGGB8:
> -		buf += fmt->fmt.pix.height * fmt->fmt.pix.bytesperline / 4 +
> -			fmt->fmt.pix.width / 4;
> -
> -		for (y = 0; y < fmt->fmt.pix.height / 2; y++) {
> -			for (x = 0; x < fmt->fmt.pix.width / 2; x++)
> -				avg_lum += *buf++;
> -			buf += fmt->fmt.pix.bytesperline - fmt->fmt.pix.width / 2;
> -		}
> -		avg_lum /= fmt->fmt.pix.height * fmt->fmt.pix.width / 4;
> +		avg_lum = get_luminosity_bayer8(buf, fmt);
>  		break;
>  
>  	case V4L2_PIX_FMT_RGB24:
> diff --git a/lib/libv4lconvert/processing/libv4lprocessing.c b/lib/libv4lconvert/processing/libv4lprocessing.c
> index b061f50..b98d024 100644
> --- a/lib/libv4lconvert/processing/libv4lprocessing.c
> +++ b/lib/libv4lconvert/processing/libv4lprocessing.c
> @@ -164,6 +165,10 @@ void v4lprocessing_processing(struct v4lprocessing_data *data,
>  	case V4L2_PIX_FMT_SGRBG8:
>  	case V4L2_PIX_FMT_SBGGR8:
>  	case V4L2_PIX_FMT_SRGGB8:
> +	case V4L2_PIX_FMT_SGBRG10:
> +	case V4L2_PIX_FMT_SGRBG10:
> +	case V4L2_PIX_FMT_SBGGR10:
> +	case V4L2_PIX_FMT_SRGGB10:
>  	case V4L2_PIX_FMT_RGB24:
>  	case V4L2_PIX_FMT_BGR24:
>  		break;
> 
> 
> 



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1631409 — Re: [patch] propagating controls in libv4l2 was Re: support autofocus / autogain in libv4l2

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

[toc] | [prev] | [next] | [standalone]

#1633281 — Re: [patch] propagating controls in libv4l2 was Re: support autofocus / autogain in libv4l2

[Multipart message — attachments visible in raw view] — view raw

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web