linux-kernel - Re: [PATCH RFC v2] media: v4l2-ctrl: Add gain controls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 6 Dec 2012 11:54:32 +0200
From:	Sakari Ailus <sakari.ailus@....fi>
To:	Prabhakar Lad <prabhakar.csengg@...il.com>
Cc:	Hans Verkuil <hverkuil@...all.nl>,
	LMML <linux-media@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	LDOC <linux-doc@...r.kernel.org>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Guennadi Liakhovetski <g.liakhovetski@....de>,
	Sylwester Nawrocki <s.nawrocki@...sung.com>,
	Hans Verkuil <hans.verkuil@...co.com>,
	Hans de Goede <hdegoede@...hat.com>,
	Chris MacGregor <chris@...ermato.com>,
	Rob Landley <rob@...dley.net>,
	Jeongtae Park <jtp.park@...sung.com>,
	Mauro Carvalho Chehab <mchehab@...radead.org>
Subject: Re: [PATCH RFC v2] media: v4l2-ctrl: Add gain controls

Hi Prabhakar and Hans,

On Thu, Dec 06, 2012 at 10:24:18AM +0530, Prabhakar Lad wrote:
> Hi Hans,
> 
> On Wed, Dec 5, 2012 at 5:38 PM, Hans Verkuil <hverkuil@...all.nl> wrote:
> > (resend without HTML formatting)
> >
> > On Wed 5 December 2012 12:49:29 Prabhakar Lad wrote:
> >> From: Lad, Prabhakar <prabhakar.csengg@...il.com>
> >>
> >> add support for per color component digital/analog gain controls
> >> and also their corresponding offset.
> >
> > Some obvious questions below...
> >
> >>
> >> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@...il.com>
> >> Cc: Sakari Ailus <sakari.ailus@....fi>
> >> Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> >> Cc: Kyungmin Park <kyungmin.park@...sung.com>
> >> Cc: Guennadi Liakhovetski <g.liakhovetski@....de>
> >> Cc: Sylwester Nawrocki <s.nawrocki@...sung.com>
> >> Cc: Hans Verkuil <hans.verkuil@...co.com>
> >> Cc: Hans de Goede <hdegoede@...hat.com>
> >> Cc: Chris MacGregor <chris@...ermato.com>
> >> Cc: Rob Landley <rob@...dley.net>
> >> Cc: Jeongtae Park <jtp.park@...sung.com>
> >> Cc: Mauro Carvalho Chehab <mchehab@...radead.org>
> >> ---
> >>  Changes for v2:
> >>  1: Fixed review comments pointed by Laurent.
> >>  2: Rebased on latest tree.
> >>
> >>  Documentation/DocBook/media/v4l/controls.xml |   54 ++++++++++++++++++++++++++
> >>  drivers/media/v4l2-core/v4l2-ctrls.c         |   11 +++++
> >>  include/uapi/linux/v4l2-controls.h           |   11 +++++
> >>  3 files changed, 76 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> >> index 7fe5be1..847a9bb 100644
> >> --- a/Documentation/DocBook/media/v4l/controls.xml
> >> +++ b/Documentation/DocBook/media/v4l/controls.xml
> >> @@ -4543,6 +4543,60 @@ interface and may change in the future.</para>
> >>           specific test patterns can be used to test if a device is working
> >>           properly.</entry>
> >>         </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_RED</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_RED</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN_BLUE</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_BLUE</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_GREEN</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="descr"> Some capture/sensor devices have
> >> +         the capability to set per color component digital/analog gain values.</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GAIN_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_BLUE_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_RED_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_RED_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="id"><constant>V4L2_CID_GREEN_BLUE_OFFSET</constant></entry>
> >> +         <entry>integer</entry>
> >> +       </row>
> >> +       <row>
> >> +         <entry spanname="descr"> Some capture/sensor devices have the
> >> +         capability to set per color component digital/analog gain offset values.
> >> +         V4L2_CID_GAIN_OFFSET is the global gain offset and the rest are per
> >> +         color component gain offsets.</entry>
> >
> > If I set both V4L2_CID_GAIN_RED and V4L2_CID_RED_OFFSET, how are they supposed
> > to interact? Or are they mutually exclusive?
> >
> > And if I set both V4L2_CID_GAIN_OFFSET and V4L2_CID_RED_OFFSET, how are they supposed
> > to interact?
> >
> > This questions should be answered in the documentation...
> >
> I haven’t worked on the hardware which supports both, What is the general
> behaviour when the hardware supports both per color component and global
> and both of them are set ? That could be helpful for me to document.

I'd guess most of the time only either one is supported, and when someone
thinks of supporting both on the same device, we can start thinking of the
interaction of per-component and global ones. That may be hardware specific
as well, so standardising it might not be possible.

I think it'd be far more important to know which unit is it. Many such
controls are indeed fixed point values but the location of the point varies.
For unstance, u16,u16 and u8,u8 aren't uncommon. We currently have no way to
tell this to the user space. This isn't in any way specific to gain or
offset controls, though.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@....fi	XMPP: sailus@...iisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/