All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	linux-media@vger.kernel.org, pavel@ucw.cz
Subject: Re: [PATCH v3 1/2] v4l: Add camera voice coil lens control class, current control
Date: Wed, 19 Apr 2017 10:51:18 -0300	[thread overview]
Message-ID: <20170419105118.72b8e284@vento.lan> (raw)
In-Reply-To: <20170416091209.GB7456@valkosipuli.retiisi.org.uk>

Em Sun, 16 Apr 2017 12:12:10 +0300
Sakari Ailus <sakari.ailus@iki.fi> escreveu:

> Hi Mauro,
> 
> On Fri, Apr 14, 2017 at 11:23:32PM -0300, Mauro Carvalho Chehab wrote:
> > Hi Sakari,
> > 
> > Em Tue, 14 Feb 2017 14:20:22 +0200
> > Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> >   
> > > Add a V4L2 control class for voice coil lens driver devices. These are
> > > simple devices that are used to move a camera lens from its resting
> > > position.  
> > 
> > From some past threads with this patch, you mentioned that:
> > 
> > "The FOCUS_ABSOLUTE control really is not a best control ID to
> >  control a voice coil driver's current."
> > 
> > However, I'm not seeing any explanation there at the thread, at
> > the patch description or at the documentation explaining why, and,
> > more important, when someone should use the focus control or the
> > camera voice coil control.  
> 
> It should be available in the thread.

The email thread is not registered at git logs nor at the API spec.

> Nevertheless, V4L2_CID_FOCUS_ABSOLUTE
> is documented as follows (emphasis mine):
> 
> 	This control sets the *focal point* of the camera to the specified
> 	position. The unit is undefined. Positive values set the focus
> 	closer to the camera, negative values towards infinity.
> 
> What you control in voice coil devices is current (in Ampères) and the
> current only has a relatively loose relation to the focal point.

The real problem I'm seeing here is that this control is already
used by voice coil motor (VCM). Several UVC-based Logitech cameras
come with VCM, like their QuickCam Pro-series webcams:

	https://secure.logitech.com/en-hk/articles/3231

The voice coil can be seen on this picture:
	https://photo.stackexchange.com/questions/48678/can-i-modify-a-logitech-c615-webcam-for-infinity-focus

> Additionally, increasing the current brings the focus closer, not farther.

That's just a hardware/software implementation detail. One could use
a voice coil to do just the reverse: without any current, it would
be getting a minimal focus distance; with max current it would
go to infinite, or may have some firmware inside the hardware that
would be inverting the signal.

I have here a C920 camera. This model has V4L2_CID_FOCUS_ABSOLUTE:

ioctl(3, VIDIOC_G_EXT_CTRLS, {ctrl_class=V4L2_CTRL_CLASS_CAMERA, count=1, controls=[{id=V4L2_CID_FOCUS_ABSOLUTE, size=0, value=0, value64=0}]}) = 0
write(1, "                 focus_absolute "..., 97                 focus_absolute (int)    : min=0 max=250 step=5 default=0 value=0 flags=inactive

On this model, V4L2_CID_FOCUS_ABSOLUTE == infinite.

On a quick check at uvc driver, it seems that the driver itself
doesn't invert the value before sending to the device. So, 
I guess that, either the camera firmware do something like:
	current = 250 - control_value
Or the VCM here is mounted, in hardware, to use less current
when focusing on a close object.

Looking on it on another side, this control was added on this changeset:

    commit f9bd5843658e18a7097fc7258c60fb840109eaa8
    Author: Brandon Philips <bphilips@suse.de>
    Date:   Tue Apr 22 14:42:02 2008 -0300

    V4L/DVB (7167): [v4l] Add camera class control definitions

Meant to be used on USB cameras. Until this changeset:

	commit bee3d51156113363e952674504833b4bc92cf15e
	Author: Pavel Machek <pavel@ucw.cz>
	Date:   Fri Aug 5 07:26:11 2016 -0300

	[media] ad5820: Add driver for auto-focus coil

The only driver that were using it were uvcvideo.

As far as I remember, Brandon was working together with a Logitech
developer, in order to add support for those Quickcam Pro cameras.
So, what this control actually sets is the VCM.

That's why I don't see any need to add another control to do the same
thing.

> I anticipate adding controls for ringing compensation in the future.
> Virtually all other devices except this one do ringing compensation and
> there's some control to be done for that.

Hmm... if the idea is to have a control that doesn't do ringing
compensation, then it should be clear at the control's descriptions
that:

- V4L2_CID_FOCUS_ABSOLUTE should be used if the VCM has ringing
  compensation;
- V4L2_CID_VOICE_COIL_CURRENT and V4L2_CID_VOICE_COIL_RING_COMPENSATION
  should be used otherwise.

Btw, if the rationale for this patch is to support devices without
ring compensation, so, both controls and their descriptions should
be added at the same time, together with a patchset that would be
using both.

> How about adding such an explanation added to the commit message?

It is not enough. Documentation should be clear that VCM devices
with ring compensation should use V4L2_CID_FOCUS_ABSOLUTE.

> 
> > 
> > Worse than that, patch 2/2 gives the false sensation that both
> > controls are equal.
> > 
> > Ok, I understand that they need to be identical on the existing
> > driver, in order to keep backward compatibility, but I'm afraid
> > that, without a clear distinction between them at the documentation,
> > people may just clone the existing code on other drivers.  
> 
> Indeed. The only reason that I'm not just replacing FOCUS_ABSOLUTE with the
> new contorol is backwards compatibility. But as Pavel pointed out, he's
> likely the sole user of this device that can only be found (as far as we
> commonly are aware) in the Nokia N900.
> 
> I'm happy to just switch the control, and Pavel mentioned he's happy with
> that. It would avoid copying the code in new drivers --- which I would most
> certainly point out anyway.
> 
> > 
> > So, please add more details to patch 1/2.  
> 
> Let me know if you're happy with the above.
> 



Thanks,
Mauro

  reply	other threads:[~2017-04-19 13:51 UTC|newest]

Thread overview: 152+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-14 12:20 [PATCH v3 0/2] v4l: Add camera voice coil lens control class, current control Sakari Ailus
2017-02-14 12:20 ` [PATCH v3 1/2] " Sakari Ailus
2017-02-14 22:47   ` Pavel Machek
2017-02-15  7:15     ` Sakari Ailus
2017-02-15  8:09       ` Pavel Machek
2017-02-20 22:26         ` Sakari Ailus
2017-02-20 22:48           ` Pavel Machek
2017-04-15  2:23   ` Mauro Carvalho Chehab
2017-04-15  7:12     ` Pavel Machek
2017-04-16  9:12     ` Sakari Ailus
2017-04-19 13:51       ` Mauro Carvalho Chehab [this message]
2017-04-24  9:30         ` support autofocus / autogain in libv4l2 Pavel Machek
2017-04-24 13:38           ` Mauro Carvalho Chehab
2017-04-24 13:38             ` Mauro Carvalho Chehab
2017-04-24 21:29             ` Pavel Machek
2017-04-24 21:29               ` Pavel Machek
2017-04-25  1:47               ` Mauro Carvalho Chehab
2017-04-25  1:47                 ` Mauro Carvalho Chehab
2017-04-25  8:05                 ` Pavel Machek
2017-04-25  8:05                   ` Pavel Machek
2017-04-25  8:08                   ` Pali Rohár
2017-04-25  8:08                     ` Pali Rohár
2017-04-25  8:08                     ` Pali Rohár
2017-04-25 11:23                     ` Pavel Machek
2017-04-25 11:23                       ` Pavel Machek
2017-04-25 11:30                       ` Pali Rohár
2017-04-25 11:30                         ` Pali Rohár
2017-04-25 12:28                         ` Pavel Machek
2017-04-25 12:28                           ` Pavel Machek
2017-04-25 12:51                           ` Pali Rohár
2017-04-25 12:51                             ` Pali Rohár
2017-04-25 16:55                         ` Nicolas Dufresne
2017-04-25 16:55                           ` Nicolas Dufresne
2017-04-25 16:51                     ` Nicolas Dufresne
2017-04-25 16:51                       ` Nicolas Dufresne
2017-04-25 16:53                   ` Nicolas Dufresne
2017-04-25 16:53                     ` Nicolas Dufresne
2017-04-25 16:53                     ` Nicolas Dufresne
2017-04-26 10:53                     ` Pavel Machek
2017-04-26 10:53                       ` Pavel Machek
2017-04-26 10:53                 ` [patch] propagating controls in libv4l2 was " Pavel Machek
2017-04-26 10:53                   ` Pavel Machek
2017-04-26 11:13                   ` Mauro Carvalho Chehab
2017-04-26 11:13                     ` Mauro Carvalho Chehab
2017-04-26 13:23                     ` [patch] autogain support for bayer10 format (was Re: [patch] propagating controls in libv4l2) Pavel Machek
2017-04-26 13:23                       ` Pavel Machek
2017-04-26 15:43                       ` Ivaylo Dimitrov
2017-04-26 15:43                         ` Ivaylo Dimitrov
2017-04-26 22:51                         ` Pavel Machek
2017-04-26 22:51                           ` Pavel Machek
2017-04-26 22:51                           ` Pavel Machek
2017-04-27  5:52                           ` Ivaylo Dimitrov
2017-04-27  5:52                             ` Ivaylo Dimitrov
2017-07-13  7:57                             ` Pavel Machek
2017-07-13  7:57                               ` Pavel Machek
2017-05-03 19:05                         ` Russell King - ARM Linux
2017-05-03 19:05                           ` Russell King - ARM Linux
2017-05-03 19:58                           ` Pavel Machek
2017-05-03 19:58                             ` Pavel Machek
2017-05-08 22:28                         ` [patch, libv4l]: fix integer overflow Pavel Machek
2017-05-09  6:29                           ` Hans Verkuil
2017-05-09  6:32                             ` Hans Verkuil
2017-05-09  8:02                               ` Pavel Machek
2017-05-09 10:59                             ` [patch, libv4l]: fix typos Pavel Machek
2017-05-09 11:01                             ` [patch, libv4l]: Add support for GRBG10 format conversion Pavel Machek
2017-05-09 11:04                             ` [patch, libv4l]: Introduce define for lookup table size Pavel Machek
2017-05-16 11:17                               ` Hans Verkuil
2017-05-16 12:45                                 ` Pavel Machek
2017-05-16 12:56                                   ` Hans Verkuil
2017-05-16 23:23                                     ` Pavel Machek
2017-05-19  9:13                                     ` [libv4l]: How to do 10-bit support? Stand-alone conversions? Pavel Machek
2018-03-16 20:55                                     ` [RFC, libv4l]: Make libv4l2 usable on devices with complex pipeline Pavel Machek
2018-03-19  9:47                                       ` Hans Verkuil
2018-03-19 10:23                                         ` Pavel Machek
2018-03-19 10:47                                           ` Mauro Carvalho Chehab
2018-03-19 11:11                                             ` Hans Verkuil
2018-03-19 12:00                                               ` Pavel Machek
2018-03-19 12:15                                                 ` Hans Verkuil
2018-03-19 12:48                                                   ` Pavel Machek
2018-03-19 13:29                                                     ` Hans Verkuil
2018-03-19 22:18                                                       ` Pavel Machek
2018-03-19 13:45                                                     ` Mauro Carvalho Chehab
2018-03-19 13:26                                                   ` Mauro Carvalho Chehab
2018-03-20  7:50                                                     ` Pavel Machek
2018-05-15 20:01                                                     ` Pavel Machek
2018-05-15 22:03                                                       ` Mauro Carvalho Chehab
2018-05-16 20:53                                                         ` Pavel Machek
2018-06-02 21:01                                                         ` Pavel Machek
2018-06-06  6:18                                                           ` Tomasz Figa
2018-06-06  8:46                                                             ` Pavel Machek
2018-06-06  8:53                                                               ` Tomasz Figa
2018-06-06 10:01                                                                 ` Pavel Machek
2018-06-06 16:57                                                                   ` Mauro Carvalho Chehab
2018-06-06 21:27                                                                     ` Pavel Machek
2018-06-07 12:22                                                                     ` [PATCH, " Pavel Machek
2018-06-06 10:51                                                                 ` [RFC, " Pavel Machek
2018-06-06 11:16                                                                   ` Tomasz Figa
2018-06-06 20:37                                                                     ` Pavel Machek
2018-06-06 17:13                                                                   ` Mauro Carvalho Chehab
2018-06-06 10:23                                                             ` Pavel Machek
2018-06-07  7:25                                                             ` Pavel Machek
2017-05-09 11:10                             ` [patch, libv4l]: fix integer overflow Pavel Machek
2017-05-16 10:42                             ` Pavel Machek
2017-05-21 10:33                             ` [patch, libv4l]: add sdlcam example for testing digital still camera functionality Pavel Machek
2017-05-26 20:41                               ` Pavel Machek
2017-05-27  9:27                                 ` Hans Verkuil
2017-05-29  6:13                               ` Hans Verkuil
2017-05-29  7:32                                 ` Pavel Machek
2017-05-29  8:02                                   ` Hans Verkuil
2017-06-14 11:16                                     ` Sakari Ailus
2017-06-14 20:41                                       ` Pavel Machek
2017-07-13  8:36                                       ` Pavel Machek
2017-05-09  8:07                           ` [patch, libv4l]: fix integer overflow Pavel Machek
2017-04-30 22:48                       ` [patch] autogain support for bayer10 format (was Re: [patch] propagating controls in libv4l2) Pavel Machek
2017-04-30 22:48                         ` Pavel Machek
2017-07-13  9:49                     ` [patch] propagating controls in libv4l2 was Re: support autofocus / autogain in libv4l2 Pavel Machek
2017-07-13  9:49                       ` Pavel Machek
2017-04-26 11:26                   ` Mauro Carvalho Chehab
2017-04-26 11:26                     ` Mauro Carvalho Chehab
2017-04-29  9:19                     ` Pavel Machek
2017-04-29  9:19                       ` Pavel Machek
2017-10-21 22:00                     ` Camera support, Prague next week, sdlcam Pavel Machek
2017-10-22  7:36                       ` Hans Verkuil
2017-10-22  7:36                         ` Hans Verkuil
2017-10-22  8:31                         ` Pavel Machek
2017-10-22  8:31                           ` Pavel Machek
2017-10-23 18:54                         ` Pavel Machek
2017-10-23 18:54                           ` Pavel Machek
2017-10-23 19:24                           ` Hans Verkuil
2017-10-23 19:24                             ` Hans Verkuil
2017-10-23 20:15                             ` Sakari Ailus
2017-10-23 20:15                               ` Sakari Ailus
2017-10-23 21:02                               ` Mauro Carvalho Chehab
2017-10-23 21:02                                 ` Mauro Carvalho Chehab
2017-10-31 21:28                               ` Nokia N9: fun with camera Pavel Machek
2017-11-01  6:36                                 ` Pavel Machek
2017-11-01  6:36                                   ` Pavel Machek
2017-11-01 15:32                                   ` Pavel Machek
2017-11-01 15:32                                     ` Pavel Machek
2017-04-24 22:07             ` support autofocus / autogain in libv4l2 Pavel Machek
2017-04-24 22:07               ` Pavel Machek
2017-04-25  1:57               ` Mauro Carvalho Chehab
2017-04-25  1:57                 ` Mauro Carvalho Chehab
2017-04-25  8:20                 ` Pavel Machek
2017-04-25  8:20                   ` Pavel Machek
2017-04-25 11:23                 ` Pavel Machek
2017-04-25 11:23                   ` Pavel Machek
2017-04-28 22:00         ` [PATCH v3 1/2] v4l: Add camera voice coil lens control class, current control Pavel Machek
2017-04-29  1:46           ` Mauro Carvalho Chehab
2017-05-12 10:49         ` Sakari Ailus
2017-05-13 11:08           ` Pavel Machek
2017-02-14 12:20 ` [PATCH v3 2/2] ad5820: Use VOICE_COIL_CURRENT control Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170419105118.72b8e284@vento.lan \
    --to=mchehab@s-opensource.com \
    --cc=linux-media@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=sakari.ailus@iki.fi \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.