Chroma gain configuration

Chroma gain configuration

[Devin Heitmueller]

I am doing some work on the saa711x driver, and ran into a case where
I need to disable the chroma AGC and manually set the chroma gain.

I see there is an existing boolean control called V4L2_CID_CHROMA_AGC,
which would be the logical candidate for allowing the user to disable
the chroma AGC.  However, once this is done I still need to expose the
ability to set the gain manually (bits 6-0 of register 0x0f).

Is there some existing control I am just missing?  Or do I need to do
this through a private control.

I'm asking because it seems a bit strange that someone would introduce
a v4l2 standard control to disable the AGC but not have the ability to
manually set the gain once it was disabled.

Suggestions welcome.  I obviously would only want to introduce a
private control if absolutely necessary.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Frej Drejhammar]

> I'm asking because it seems a bit strange that someone would introduce
> a v4l2 standard control to disable the AGC but not have the ability to
> manually set the gain once it was disabled.

As the person who introduced V4L2_CID_CHROMA_AGC for cx2388x I can
explain that part. The AGC was actually introduced when only a manual
gain setting was available and the AGC was disabled. The addition of the
V4L2_CID_CHROMA_AGC allowed the AGC to be enabled by default, which is
probably what most users want, but still have a way to set chroma gain
manually. The cx88 driver allows you to set the UV-gain using the
V4L2_CID_SATURATION control when the AGC is disabled.

Regards,

--Frej

Re: Chroma gain configuration

[Andy Walls]

On Sun, 2010-02-21 at 23:07 -0500, Devin Heitmueller wrote:
> I am doing some work on the saa711x driver, and ran into a case where
> I need to disable the chroma AGC and manually set the chroma gain.

Sakari, Hans, or anyone else,

On a somewhat related note, what is the status of the media controller
and of file handles per v4l2_subdev.  Will Sakari's V4L file-handle
changes be all we need for the infrastructure or is there more to be
done after that?

I'd like to implement specific "technician controls", something an
average user wouldn't use, for a few subdevs.



> I see there is an existing boolean control called V4L2_CID_CHROMA_AGC,
> which would be the logical candidate for allowing the user to disable
> the chroma AGC.  However, once this is done I still need to expose the
> ability to set the gain manually (bits 6-0 of register 0x0f).
> 
> Is there some existing control I am just missing?  Or do I need to do
> this through a private control.
> 
> I'm asking because it seems a bit strange that someone would introduce
> a v4l2 standard control to disable the AGC but not have the ability to
> manually set the gain once it was disabled.

Devin,

Well, I can imagine letting hardware do the initial AGC, and then when
it is settled manually disabling it to prevent hardware from getting
"fooled".

Regards,
Andy

> Suggestions welcome.  I obviously would only want to introduce a
> private control if absolutely necessary.
> 
> Devin

Re: Chroma gain configuration

[Mauro Carvalho Chehab]

Andy Walls wrote:
> On Sun, 2010-02-21 at 23:07 -0500, Devin Heitmueller wrote:
>> I am doing some work on the saa711x driver, and ran into a case where
>> I need to disable the chroma AGC and manually set the chroma gain.
> 
> Sakari, Hans, or anyone else,
> 
> On a somewhat related note, what is the status of the media controller
> and of file handles per v4l2_subdev.  Will Sakari's V4L file-handle
> changes be all we need for the infrastructure or is there more to be
> done after that?
> 
> I'd like to implement specific "technician controls", something an
> average user wouldn't use, for a few subdevs.

The exposition of a control to the user or not is a decision of the userspace
software developer. We shouldn't be too concerned about it. Eventually,
we can group some controls on a "raw hardware level" group. I don't think we
need a media controller for it. Also, this won't avoid developers to use the media
controller to expose such controls to userspace.
 
>> I see there is an existing boolean control called V4L2_CID_CHROMA_AGC,
>> which would be the logical candidate for allowing the user to disable
>> the chroma AGC.  However, once this is done I still need to expose the
>> ability to set the gain manually (bits 6-0 of register 0x0f).
>>
>> Is there some existing control I am just missing?  Or do I need to do
>> this through a private control.
>>
>> I'm asking because it seems a bit strange that someone would introduce
>> a v4l2 standard control to disable the AGC but not have the ability to
>> manually set the gain once it was disabled.
> 
> Devin,
> 
> Well, I can imagine letting hardware do the initial AGC, and then when
> it is settled manually disabling it to prevent hardware from getting
> "fooled".

I did some tests on it with cx23881/cx23883 chips. At least on cx88, as far
as I remember, the AGC doesn't affect the saturation register, so, this 
trick won't work. 

The issue with cx88 chips is that, with some video input sources, the 
AGC over-saturates the color pattern. So, depending on the analog video
standard and the quality of the source (TV or Composite/Svideo), it gives
more reallistic colors with different AGC/saturation configuration.

Cheers,
Mauro

Re: Chroma gain configuration

[Devin Heitmueller]

Thanks everybody for the feedback.

On Mon, Feb 22, 2010 at 7:15 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> The issue with cx88 chips is that, with some video input sources, the
> AGC over-saturates the color pattern. So, depending on the analog video
> standard and the quality of the source (TV or Composite/Svideo), it gives
> more reallistic colors with different AGC/saturation configuration.

I'm actually having the same issue with the saa7113.  I have a
specific input source where I am getting too much chroma gain via the
AGC, and need to disable it and manually turn it down a bit.

While I can use the V4L2_CID_CHROMA_AGC to disable the AGC, I still
need to then adjust the value of the gain.  I guess I *could* reuse
the saturation control, this time controlling the chroma gain register
instead of the saturation register, it would seem to make more sense
to have an explicit control, since the controls correspond to
different registers and in theory could be controlled independently.

I guess at this point, I have three options:

1.   Introduce a new user control

2.  Use a private control

3.  Reuse the saturation control (hacking the driver such that the
saturation control pokes different registers depending on whether the
AGC is enabled).

On a related note, has anyone noticed that the v4l2-dbg tool appears
to always insist on using the "extended controls ioctl" for any
attempts to set private controls?  This doesn't seem right to me.  I
believe there probably are cases where extended controls are required,
but I believe just a general user control based on
V4L2_CID_PRIVATE_BASE should probably be able to work even with the
generic VIDIOC_S_CTRL.

I'm just asking because it would mean in order for v4l2-dbg to work
with my solution i would have to add support for extended controls in
general to the saa7115 driver, which shouldn't be necessary.

Cheers,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Mauro Carvalho Chehab]

Devin Heitmueller wrote:
> Thanks everybody for the feedback.
> 
> On Mon, Feb 22, 2010 at 7:15 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> The issue with cx88 chips is that, with some video input sources, the
>> AGC over-saturates the color pattern. So, depending on the analog video
>> standard and the quality of the source (TV or Composite/Svideo), it gives
>> more reallistic colors with different AGC/saturation configuration.
> 
> I'm actually having the same issue with the saa7113.  I have a
> specific input source where I am getting too much chroma gain via the
> AGC, and need to disable it and manually turn it down a bit.
> 
> While I can use the V4L2_CID_CHROMA_AGC to disable the AGC, I still
> need to then adjust the value of the gain.  I guess I *could* reuse
> the saturation control, this time controlling the chroma gain register
> instead of the saturation register, it would seem to make more sense
> to have an explicit control, since the controls correspond to
> different registers and in theory could be controlled independently.
> 
> I guess at this point, I have three options:
> 
> 1.   Introduce a new user control
> 
> 2.  Use a private control
> 
> 3.  Reuse the saturation control (hacking the driver such that the
> saturation control pokes different registers depending on whether the
> AGC is enabled).

I don't like (2). 

That's said, we really need to take a closer look on those color gain
controls. We have already several controls that change color gain:

#define V4L2_CID_SATURATION             (V4L2_CID_BASE+2)
#define V4L2_CID_AUTOGAIN               (V4L2_CID_BASE+18)
#define V4L2_CID_GAIN                   (V4L2_CID_BASE+19)
#define V4L2_CID_CHROMA_AGC                     (V4L2_CID_BASE+29)
#define V4L2_CID_AUTO_WHITE_BALANCE     (V4L2_CID_BASE+12)
#define V4L2_CID_DO_WHITE_BALANCE       (V4L2_CID_BASE+13)
#define V4L2_CID_RED_BALANCE            (V4L2_CID_BASE+14)
#define V4L2_CID_BLUE_BALANCE           (V4L2_CID_BASE+15)
#define V4L2_CID_WHITE_BALANCE_TEMPERATURE      (V4L2_CID_BASE+26)

The map of the above controls are not uniform along the drivers, and the API
is not clear enough about what is expected on each of the above controls.

For example, on some drivers (mostly webcam ones), the red/blue balance as used as 
"red/blue gain", and not as balance.

I remember we've started some discussions about this with some DaVinci patches,
but we never finished those discussions.

I think that the control you want is V4L2_CID_GAIN.
> 
> On a related note, has anyone noticed that the v4l2-dbg tool appears
> to always insist on using the "extended controls ioctl" for any
> attempts to set private controls?  This doesn't seem right to me.

I agree.

> I believe there probably are cases where extended controls are required,
> but I believe just a general user control based on
> V4L2_CID_PRIVATE_BASE should probably be able to work even with the
> generic VIDIOC_S_CTRL
> 
> I'm just asking because it would mean in order for v4l2-dbg to work
> with my solution i would have to add support for extended controls in
> general to the saa7115 driver, which shouldn't be necessary.

The end objective is to have everybody implementing extended controls and
removing the old controls, letting the V4L2 ioctl2 to convert a call to a
simple control into an extended control callback. So, I think it would
be worthy to implement extended control on saa7115.


Cheers,
Mauro

Re: Chroma gain configuration

[Devin Heitmueller]

On Mon, Feb 22, 2010 at 8:40 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> I think that the control you want is V4L2_CID_GAIN.

I would have guessed the CID_GAIN control would have been responsible
for *luma* gain.  I could be wrong about that of course (but that is
what I believe people typically think of when they think of "gain" in
general).

>> I believe there probably are cases where extended controls are required,
>> but I believe just a general user control based on
>> V4L2_CID_PRIVATE_BASE should probably be able to work even with the
>> generic VIDIOC_S_CTRL
>>
>> I'm just asking because it would mean in order for v4l2-dbg to work
>> with my solution i would have to add support for extended controls in
>> general to the saa7115 driver, which shouldn't be necessary.
>
> The end objective is to have everybody implementing extended controls and
> removing the old controls, letting the V4L2 ioctl2 to convert a call to a
> simple control into an extended control callback. So, I think it would
> be worthy to implement extended control on saa7115.

Ok then.  I'll add the 15-20 lines of code which add the extended
controls mechanism to the 7115, which just operates as a passthrough
for the older control interface.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Mauro Carvalho Chehab]

Devin Heitmueller wrote:
> On Mon, Feb 22, 2010 at 8:40 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> I think that the control you want is V4L2_CID_GAIN.
> 
> I would have guessed the CID_GAIN control would have been responsible
> for *luma* gain.  I could be wrong about that of course (but that is
> what I believe people typically think of when they think of "gain" in
> general).

Maybe it is for luma, but I bet that this is also is used for more than one
different kind of control. We should revisit those controls and properly
specify what they should do.
> 
>>> I believe there probably are cases where extended controls are required,
>>> but I believe just a general user control based on
>>> V4L2_CID_PRIVATE_BASE should probably be able to work even with the
>>> generic VIDIOC_S_CTRL
>>>
>>> I'm just asking because it would mean in order for v4l2-dbg to work
>>> with my solution i would have to add support for extended controls in
>>> general to the saa7115 driver, which shouldn't be necessary.
>> The end objective is to have everybody implementing extended controls and
>> removing the old controls, letting the V4L2 ioctl2 to convert a call to a
>> simple control into an extended control callback. So, I think it would
>> be worthy to implement extended control on saa7115.
> 
> Ok then.  I'll add the 15-20 lines of code which add the extended
> controls mechanism to the 7115, which just operates as a passthrough
> for the older control interface.

The better is to do the opposite: extended being the control interface and
the old calls as a passthrough, since the idea is to remove the old interface
after having all drivers converted.

-- 

Cheers,
Mauro

Re: Chroma gain configuration

[Devin Heitmueller]

On Mon, Feb 22, 2010 at 8:58 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
>> Ok then.  I'll add the 15-20 lines of code which add the extended
>> controls mechanism to the 7115, which just operates as a passthrough
>> for the older control interface.
>
> The better is to do the opposite: extended being the control interface and
> the old calls as a passthrough, since the idea is to remove the old interface
> after having all drivers converted.

I gave this a bit of thought, and I'm not sure what you are proposing
is actually possible.  Because the extended controls provides a
superset of the functionality of the older user controls interface, it
is possible to create a extended control callback which just passes
through the request (since any user control can be converted into a
extended control).  However, you cannot convert the extended control
results into the older user control format, since not all the
information could be provided.

In fact, I would be in favor of taking the basic logic found in
cx18_g_ext_ctrls(), and making that generic to the videodev interface,
such that any driver which provides a user control interface but
doesn't provide an extended control function will work if the calling
application makes an extended control call.  This will allow userland
applications to always use the extended controls API, even if the
driver didn't explicitly add support for it.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Mauro Carvalho Chehab]

Devin Heitmueller wrote:
> On Mon, Feb 22, 2010 at 8:58 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>>> Ok then.  I'll add the 15-20 lines of code which add the extended
>>> controls mechanism to the 7115, which just operates as a passthrough
>>> for the older control interface.
>> The better is to do the opposite: extended being the control interface and
>> the old calls as a passthrough, since the idea is to remove the old interface
>> after having all drivers converted.
> 
> I gave this a bit of thought, and I'm not sure what you are proposing
> is actually possible.  Because the extended controls provides a
> superset of the functionality of the older user controls interface, it
> is possible to create a extended control callback which just passes
> through the request (since any user control can be converted into a
> extended control).  However, you cannot convert the extended control
> results into the older user control format, since not all the
> information could be provided.
> 
> In fact, I would be in favor of taking the basic logic found in
> cx18_g_ext_ctrls(), and making that generic to the videodev interface,
> such that any driver which provides a user control interface but
> doesn't provide an extended control function will work if the calling
> application makes an extended control call.  This will allow userland
> applications to always use the extended controls API, even if the
> driver didn't explicitly add support for it.

That's exactly the idea: convert all driverst o use ext_ctrl's and let the
V4L2 core to handle the calls to the non-extended interface. 


-- 

Cheers,
Mauro

Re: Chroma gain configuration

[Devin Heitmueller]

On Mon, Feb 22, 2010 at 4:32 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Devin Heitmueller wrote:
>> In fact, I would be in favor of taking the basic logic found in
>> cx18_g_ext_ctrls(), and making that generic to the videodev interface,
>> such that any driver which provides a user control interface but
>> doesn't provide an extended control function will work if the calling
>> application makes an extended control call.  This will allow userland
>> applications to always use the extended controls API, even if the
>> driver didn't explicitly add support for it.
>
> That's exactly the idea: convert all driverst o use ext_ctrl's and let the
> V4L2 core to handle the calls to the non-extended interface.

I think you actually missed the point of what I'm trying to say:  You
can only do the opposite of what you proposed:  You can have the v4l2
core receive extended interface calls and pass those calls through to
the older interface in drivers (since the older interface is a
*subset* of the newer interface).  However, you cannot provide a way
for callers of the older interface have those requests passed through
to the new interface (since the old interface does not support
multiple controls in one call and there are multiple classes of
controls in the newer interface).

In other words, a caller using the extended interface can
automatically call the old interface, but a caller using the old
interface cannot automatically call into the extended interface.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Hans Verkuil]

On Monday 22 February 2010 22:17:24 Devin Heitmueller wrote:
> On Mon, Feb 22, 2010 at 8:58 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
> >> Ok then.  I'll add the 15-20 lines of code which add the extended
> >> controls mechanism to the 7115, which just operates as a passthrough
> >> for the older control interface.
> >
> > The better is to do the opposite: extended being the control interface and
> > the old calls as a passthrough, since the idea is to remove the old interface
> > after having all drivers converted.
> 
> I gave this a bit of thought, and I'm not sure what you are proposing
> is actually possible.  Because the extended controls provides a
> superset of the functionality of the older user controls interface, it
> is possible to create a extended control callback which just passes
> through the request (since any user control can be converted into a
> extended control).  However, you cannot convert the extended control
> results into the older user control format, since not all the
> information could be provided.
> 
> In fact, I would be in favor of taking the basic logic found in
> cx18_g_ext_ctrls(), and making that generic to the videodev interface,
> such that any driver which provides a user control interface but
> doesn't provide an extended control function will work if the calling
> application makes an extended control call.  This will allow userland
> applications to always use the extended controls API, even if the
> driver didn't explicitly add support for it.

I am still planning to continue my work for a general control handling
framework. I know how to do it and it's just time that I lack.

Converting all drivers to support the extended control API is quite complicated
since the API is fairly complex (esp. with regard to atomicity). In this case
my advice would be to support extended controls only where needed and wait for
this framework before converting all the other drivers.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

Re: Chroma gain configuration

[Devin Heitmueller]

On Mon, Feb 22, 2010 at 4:41 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> I am still planning to continue my work for a general control handling
> framework. I know how to do it and it's just time that I lack.
>
> Converting all drivers to support the extended control API is quite complicated
> since the API is fairly complex (esp. with regard to atomicity). In this case
> my advice would be to support extended controls only where needed and wait for
> this framework before converting all the other drivers.

Hans,

I have no objection to holding off if that's what you recommend.  The
only reason we got onto this thread was because the v4l2-dbg
application seems to implicitly assume that *all* private controls
using V4L2_CID_PRIVATE_BASE can only be accessed via the extended
control interface, meaning you cannot use the utility in conjunction
with a driver that has a private control defined in the the
VIDIOC_G_CTRL function.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Hans Verkuil]

On Monday 22 February 2010 22:38:56 Devin Heitmueller wrote:
> On Mon, Feb 22, 2010 at 4:32 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
> > Devin Heitmueller wrote:
> >> In fact, I would be in favor of taking the basic logic found in
> >> cx18_g_ext_ctrls(), and making that generic to the videodev interface,
> >> such that any driver which provides a user control interface but
> >> doesn't provide an extended control function will work if the calling
> >> application makes an extended control call.  This will allow userland
> >> applications to always use the extended controls API, even if the
> >> driver didn't explicitly add support for it.
> >
> > That's exactly the idea: convert all driverst o use ext_ctrl's and let the
> > V4L2 core to handle the calls to the non-extended interface.
> 
> I think you actually missed the point of what I'm trying to say:  You
> can only do the opposite of what you proposed:  You can have the v4l2
> core receive extended interface calls and pass those calls through to
> the older interface in drivers (since the older interface is a
> *subset* of the newer interface).  However, you cannot provide a way
> for callers of the older interface have those requests passed through
> to the new interface (since the old interface does not support
> multiple controls in one call and there are multiple classes of
> controls in the newer interface).
> 
> In other words, a caller using the extended interface can
> automatically call the old interface, but a caller using the old
> interface cannot automatically call into the extended interface.

Sure you can. See v4l2-ioctl.c, VIDIOC_G/S_CTRL. That's exactly what is
being done there.

It's the other way around that is not in general possible.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

Re: Chroma gain configuration

[Hans Verkuil]

On Monday 22 February 2010 22:43:58 Devin Heitmueller wrote:
> On Mon, Feb 22, 2010 at 4:41 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > I am still planning to continue my work for a general control handling
> > framework. I know how to do it and it's just time that I lack.
> >
> > Converting all drivers to support the extended control API is quite complicated
> > since the API is fairly complex (esp. with regard to atomicity). In this case
> > my advice would be to support extended controls only where needed and wait for
> > this framework before converting all the other drivers.
> 
> Hans,
> 
> I have no objection to holding off if that's what you recommend.  The
> only reason we got onto this thread was because the v4l2-dbg
> application seems to implicitly assume that *all* private controls
> using V4L2_CID_PRIVATE_BASE can only be accessed via the extended
> control interface, meaning you cannot use the utility in conjunction
> with a driver that has a private control defined in the the
> VIDIOC_G_CTRL function.

Ah, that's another matter. The original approach for handling private
controls is seriously flawed. Drivers that want to use private controls
are strongly encouraged to use the extended control mechanism for them,
and to document those controls in the spec.

Actually, it is not so much the extended control API that is relevant
here, but the use of V4L2_CTRL_FLAG_NEXT_CTRL in VIDIOC_QUERYCTRL to
enumerate the controls.

Unfortunately, the current support functions in v4l2-common.c to help
with this are pretty crappy, for which I apologize.

Regards,

	Hans

> 
> Devin
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

Re: Chroma gain configuration

[Devin Heitmueller]

On Mon, Feb 22, 2010 at 4:54 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Ah, that's another matter. The original approach for handling private
> controls is seriously flawed. Drivers that want to use private controls
> are strongly encouraged to use the extended control mechanism for them,
> and to document those controls in the spec.

Yeah, it's just annoying that what should have been a change for
something like six lines of code in the g_ctrl/s_ctrl functions in
saa7115 is actually resulting in me having to extend saa7115 to add
support for the extended control interface.  Yeah, I can do that, but
it's still annoying that it should be necessary.

> Actually, it is not so much the extended control API that is relevant
> here, but the use of V4L2_CTRL_FLAG_NEXT_CTRL in VIDIOC_QUERYCTRL to
> enumerate the controls.

Control enumeration is actually working fine.  The queryctrl does
properly return all of the controls, including my new private control.

> Unfortunately, the current support functions in v4l2-common.c to help
> with this are pretty crappy, for which I apologize.

Of course, if you and Mauro wanted to sign off on the creation of a
new non-private user control called V4L2_CID_CHROMA_GAIN, that would
also resolve my problem.  :-)

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Sakari Ailus]

Andy Walls wrote:
> On Sun, 2010-02-21 at 23:07 -0500, Devin Heitmueller wrote:
>> I am doing some work on the saa711x driver, and ran into a case where
>> I need to disable the chroma AGC and manually set the chroma gain.
> 
> Sakari, Hans, or anyone else,

Hi Andy,

> On a somewhat related note, what is the status of the media controller
> and of file handles per v4l2_subdev.  Will Sakari's V4L file-handle
> changes be all we need for the infrastructure or is there more to be
> done after that?

There are three things:

- V4L2 file handles (and events)
- V4L2 subdev device nodes
- Media controller

The file handles and events appear to be fairly ready.

> I'd like to implement specific "technician controls", something an
> average user wouldn't use, for a few subdevs.

For that you'd need at least V4L2 subdev device nodes, preferrably also
Media controller e.g. for the user space to know the two devices indeed
are connected to the same higher level device. File handles do not
matter here; it's just a generic way to store file handle specific data,
required by events, for example.

Subdev device nodes and Media controller patches live in Laurent's tree
at linuxtv.org at the moment.

Regards,

-- 
Sakari Ailus
sakari.ailus@maxwell.research.nokia.com

Re: Chroma gain configuration

[Hans Verkuil]

On Monday 22 February 2010 23:00:32 Devin Heitmueller wrote:
> On Mon, Feb 22, 2010 at 4:54 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > Ah, that's another matter. The original approach for handling private
> > controls is seriously flawed. Drivers that want to use private controls
> > are strongly encouraged to use the extended control mechanism for them,
> > and to document those controls in the spec.
> 
> Yeah, it's just annoying that what should have been a change for
> something like six lines of code in the g_ctrl/s_ctrl functions in
> saa7115 is actually resulting in me having to extend saa7115 to add
> support for the extended control interface.  Yeah, I can do that, but
> it's still annoying that it should be necessary.
> 
> > Actually, it is not so much the extended control API that is relevant
> > here, but the use of V4L2_CTRL_FLAG_NEXT_CTRL in VIDIOC_QUERYCTRL to
> > enumerate the controls.
> 
> Control enumeration is actually working fine.  The queryctrl does
> properly return all of the controls, including my new private control.

OK. So the problem is that v4l2-ctl uses G/S_EXT_CTRLS for non-user controls,
right? Why not change v4l2-ctl: let it first try the EXT version but if that
fails with EINVAL then try the old control API.

> 
> > Unfortunately, the current support functions in v4l2-common.c to help
> > with this are pretty crappy, for which I apologize.
> 
> Of course, if you and Mauro wanted to sign off on the creation of a
> new non-private user control called V4L2_CID_CHROMA_GAIN, that would
> also resolve my problem.  :-)

Hmm, Mauro is right: the color controls we have now are a bit of a mess.
Perhaps this is a good moment to try and fix them. Suppose we had no color
controls at all: how would we design them in that case? When we know what we
really need, then we can compare that with what we have and figure out what
we need to do to make things right again.

Regards,

	Hans

> 
> Devin
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

Re: Chroma gain configuration

[Andy Walls]

On Tue, 2010-02-23 at 08:53 +0100, Hans Verkuil wrote:
> On Monday 22 February 2010 23:00:32 Devin Heitmueller wrote:
> > On Mon, Feb 22, 2010 at 4:54 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:

> > Of course, if you and Mauro wanted to sign off on the creation of a
> > new non-private user control called V4L2_CID_CHROMA_GAIN, that would
> > also resolve my problem.  :-)
> 
> Hmm, Mauro is right: the color controls we have now are a bit of a mess.
> Perhaps this is a good moment to try and fix them. Suppose we had no color
> controls at all: how would we design them in that case? When we know what we
> really need, then we can compare that with what we have and figure out what
> we need to do to make things right again.

Hmmm:

1. comb filter enable/disable
2. chroma AGC enable/disable
3. chroma kill threshold and enable/disable
4. UV saturation  (C vector magnitude adjustment as long as you adjust U
and V in the same way.)
5. Hue (C vector phase adjustment)
6. chroma coring
7. chroma delay/advance in pixels relative to luma pixels
8. chroma subcarrier locking algorithm: fast, slow, adaptive
9. chroma notch filer settings (when doing Y/C separation from CVBS)
10. additional analog signal gain
11. anti-alias filter enable/disable

And that's just from a quick scan of the public CX25836/7 datasheet.

I left my handbook with all sorts of details about the Human Visual
System and the CIE and NTSC and PAL colorspaces at work.

Regards,
Andy





> Regards,
> 
> 	Hans
> 
> > 
> > Devin
> > 
> > 
> 

Re: Chroma gain configuration

[Hans Verkuil]

> On Tue, 2010-02-23 at 08:53 +0100, Hans Verkuil wrote:
>> On Monday 22 February 2010 23:00:32 Devin Heitmueller wrote:
>> > On Mon, Feb 22, 2010 at 4:54 PM, Hans Verkuil <hverkuil@xs4all.nl>
>> wrote:
>
>> > Of course, if you and Mauro wanted to sign off on the creation of a
>> > new non-private user control called V4L2_CID_CHROMA_GAIN, that would
>> > also resolve my problem.  :-)
>>
>> Hmm, Mauro is right: the color controls we have now are a bit of a mess.
>> Perhaps this is a good moment to try and fix them. Suppose we had no
>> color
>> controls at all: how would we design them in that case? When we know
>> what we
>> really need, then we can compare that with what we have and figure out
>> what
>> we need to do to make things right again.
>
> Hmmm:
>
> 1. comb filter enable/disable
> 2. chroma AGC enable/disable
> 3. chroma kill threshold and enable/disable
> 4. UV saturation  (C vector magnitude adjustment as long as you adjust U
> and V in the same way.)
> 5. Hue (C vector phase adjustment)
> 6. chroma coring
> 7. chroma delay/advance in pixels relative to luma pixels
> 8. chroma subcarrier locking algorithm: fast, slow, adaptive
> 9. chroma notch filer settings (when doing Y/C separation from CVBS)
> 10. additional analog signal gain
> 11. anti-alias filter enable/disable
>
> And that's just from a quick scan of the public CX25836/7 datasheet.
>
> I left my handbook with all sorts of details about the Human Visual
> System and the CIE and NTSC and PAL colorspaces at work.

Let me rephrase my question: how would you design the user color controls?
E.g., the controls that are exported in GUIs to the average user. Most of
the controls you mentioned above are meaningless to most users. When we
have subdev device nodes, then such controls can become accessible to
applications to do fine-tuning, but they do not belong in a GUI in e.g.
tvtime or xawtv.

The problem is of course in that grey area between obviously user-level
controls like brightness and obviously (to me at least) expert-level
controls like chroma coring.

Regards,

        Hans

>
> Regards,
> Andy
>
>
>
>
>
>> Regards,
>>
>> 	Hans
>>
>> >
>> > Devin
>> >
>> >
>>
>
>


-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: Chroma gain configuration

[Devin Heitmueller]

On Tue, Feb 23, 2010 at 2:53 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> OK. So the problem is that v4l2-ctl uses G/S_EXT_CTRLS for non-user controls,
> right? Why not change v4l2-ctl: let it first try the EXT version but if that
> fails with EINVAL then try the old control API.

Well, that's what I'm trying to figure out.  If this is a bug and I
just need to fix v4l2-ctl, then I can do that.  At this point, I was
just trying to figure out how everybody else does private controls,
since this appears to be broken out-of-the-box.

> Hmm, Mauro is right: the color controls we have now are a bit of a mess.
> Perhaps this is a good moment to try and fix them. Suppose we had no color
> controls at all: how would we design them in that case? When we know what we
> really need, then we can compare that with what we have and figure out what
> we need to do to make things right again.

Ok, this whole thread started because a client hit a specific quality
issue, and in order to address that issue I need to expose a single
slider to userland so that this advanced user can twiddle the knob
with v4l2-ctl.  I, perhaps naively, assumed this would be a trivial
change since we already have lots of infrastructure for this and the
driver in question is quite mature.  So we've gone from what I thought
was going to be a six line change in g_ctrl/s_ctrl to converting the
whole driver over to using the extended controls interface, to now the
suggestion that we redesign the way we do color controls across the
entire v4l2 subsystem?

I don't dispute that the way things have ended up is not ideal (and
presumably got that way because of the diversity of different decoders
we support and the differences in the underlying registers they
provide).  But at this point, I'm trying to solve what should be a
rather simple problem, and I haven't been contracted to redesign the
entire subsystem just to expose one advanced control.

So if people agree this is a bug in v4l2-ctl, then I'll dig into that
block of code and submit a patch.  If the real problem is that the
saa7115 driver needs to be converted to the extended control interface
to expose a single private control, well I can do that instead
(although I think that would be a pretty dumb limitation).  At this
point, I'm just trying to find the simplest change required to
accomplish something that should have taken me half an hour and been
done two days ago.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Chroma gain configuration

[Andy Walls]

On Tue, 2010-02-23 at 15:41 +0100, Hans Verkuil wrote:
> > On Tue, 2010-02-23 at 08:53 +0100, Hans Verkuil wrote:
> >> On Monday 22 February 2010 23:00:32 Devin Heitmueller wrote:
> >> > On Mon, Feb 22, 2010 at 4:54 PM, Hans Verkuil <hverkuil@xs4all.nl>
> >> wrote:
> >
> >> > Of course, if you and Mauro wanted to sign off on the creation of a
> >> > new non-private user control called V4L2_CID_CHROMA_GAIN, that would
> >> > also resolve my problem.  :-)
> >>
> >> Hmm, Mauro is right: the color controls we have now are a bit of a mess.
> >> Perhaps this is a good moment to try and fix them. Suppose we had no
> >> color
> >> controls at all: how would we design them in that case? When we know
> >> what we
> >> really need, then we can compare that with what we have and figure out
> >> what
> >> we need to do to make things right again.

> Let me rephrase my question: how would you design the user color controls?

> E.g., the controls that are exported in GUIs to the average user.

Look at the knobs on an old TV or look at the menu on more modern
televisions:

1. Hue (or Tint) (at least NTSC TVs have this control)
2. Brightness
3. Saturation

These are the three parameters to which the Human Visual System
sensitive.

Any other controls are fixing problems that the hardware can't seem to
get right on it's own - right?


>  Most of
> the controls you mentioned above are meaningless to most users. When we
> have subdev device nodes, then such controls can become accessible to
> applications to do fine-tuning, but they do not belong in a GUI in e.g.
> tvtime or xawtv.
> 
> The problem is of course in that grey area between obviously user-level
> controls like brightness and obviously (to me at least) expert-level
> controls like chroma coring.

Right, so an expert can see colors bleeding to the side in portions of
the image and guess that it's a comb filter problem.  What's my recourse
at that point, when I see such a clip submitted from user and identify
it's a comb filter problem?  "Tough, you're not an expert, so I can't
give you manual control over the comb filter so you can fix your
problem" ?

Also, just because a user can't guess what to do, doesn't mean they are
incapable of "mashing buttons" until they find something that works.  I
don't quite see the value in restricting controls from users, when the
only consequence of such restriction is them coming back here asking
what else they can try to solve a problem.  It's frustrating to have a
setting on the chip that could fix a user problem and knowing there is
no control coded up for it.  It just makes the debug cycle longer.


What is the benefit to us or to end users for denying controls to
non-expert users?


OK, I'm done ranting now. :)

Regards,
Andy

Re: Chroma gain configuration

[Andy Walls]

On Tue, 2010-02-23 at 10:33 -0500, Andy Walls wrote:
> On Tue, 2010-02-23 at 15:41 +0100, Hans Verkuil wrote:
> > > On Tue, 2010-02-23 at 08:53 +0100, Hans Verkuil wrote:
> > >> On Monday 22 February 2010 23:00:32 Devin Heitmueller wrote:
> > >> > On Mon, Feb 22, 2010 at 4:54 PM, Hans Verkuil <hverkuil@xs4all.nl>
> > >> wrote:
> > >
> > >> > Of course, if you and Mauro wanted to sign off on the creation of a
> > >> > new non-private user control called V4L2_CID_CHROMA_GAIN, that would
> > >> > also resolve my problem.  :-)
> > >>
> > >> Hmm, Mauro is right: the color controls we have now are a bit of a mess.
> > >> Perhaps this is a good moment to try and fix them. Suppose we had no
> > >> color
> > >> controls at all: how would we design them in that case? When we know
> > >> what we
> > >> really need, then we can compare that with what we have and figure out
> > >> what
> > >> we need to do to make things right again.
> 
> > Let me rephrase my question: how would you design the user color controls?
> 
> > E.g., the controls that are exported in GUIs to the average user.
> 
> Look at the knobs on an old TV or look at the menu on more modern
> televisions:
> 
> 1. Hue (or Tint) (at least NTSC TVs have this control)
> 2. Brightness
> 3. Saturation
> 
> These are the three parameters to which the Human Visual System
> sensitive.
> 
> Any other controls are fixing problems that the hardware can't seem to
> get right on it's own - right?

Bah, I forgot contrast.

Regards,
Andy

> 
> >  Most of
> > the controls you mentioned above are meaningless to most users. When we
> > have subdev device nodes, then such controls can become accessible to
> > applications to do fine-tuning, but they do not belong in a GUI in e.g.
> > tvtime or xawtv.
> > 
> > The problem is of course in that grey area between obviously user-level
> > controls like brightness and obviously (to me at least) expert-level
> > controls like chroma coring.
> 
> Right, so an expert can see colors bleeding to the side in portions of
> the image and guess that it's a comb filter problem.  What's my recourse
> at that point, when I see such a clip submitted from user and identify
> it's a comb filter problem?  "Tough, you're not an expert, so I can't
> give you manual control over the comb filter so you can fix your
> problem" ?
> 
> Also, just because a user can't guess what to do, doesn't mean they are
> incapable of "mashing buttons" until they find something that works.  I
> don't quite see the value in restricting controls from users, when the
> only consequence of such restriction is them coming back here asking
> what else they can try to solve a problem.  It's frustrating to have a
> setting on the chip that could fix a user problem and knowing there is
> no control coded up for it.  It just makes the debug cycle longer.
> 
> 
> What is the benefit to us or to end users for denying controls to
> non-expert users?
> 
> 
> OK, I'm done ranting now. :)
> 
> Regards,
> Andy
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

My TV's standard video controls (Re: Chroma gain configuration)

[Andy Walls]

On Tue, 2010-02-23 at 10:33 -0500, Andy Walls wrote:
> On Tue, 2010-02-23 at 15:41 +0100, Hans Verkuil wrote:
> > > On Tue, 2010-02-23 at 08:53 +0100, Hans Verkuil wrote:
> > >> On Monday 22 February 2010 23:00:32 Devin Heitmueller wrote:
> > >> > On Mon, Feb 22, 2010 at 4:54 PM, Hans Verkuil <hverkuil@xs4all.nl>
> > >> wrote:
> > >
> > >> > Of course, if you and Mauro wanted to sign off on the creation of a
> > >> > new non-private user control called V4L2_CID_CHROMA_GAIN, that would
> > >> > also resolve my problem.  :-)
> > >>
> > >> Hmm, Mauro is right: the color controls we have now are a bit of a mess.
> > >> Perhaps this is a good moment to try and fix them. Suppose we had no
> > >> color
> > >> controls at all: how would we design them in that case? When we know
> > >> what we
> > >> really need, then we can compare that with what we have and figure out
> > >> what
> > >> we need to do to make things right again.
> 
> > Let me rephrase my question: how would you design the user color controls?
> 
> > E.g., the controls that are exported in GUIs to the average user.
> 
> Look at the knobs on an old TV or look at the menu on more modern
> televisions:
> 
> 1. Hue (or Tint) (at least NTSC TVs have this control)
> 2. Brightness
> 3. Saturation
> 
> These are the three parameters to which the Human Visual System
> sensitive.

These are the video controls exposed on the standard user menu of my
Sony KDP-51WS655 TV:


Name
TV's Help comment
Control type
My comments

Picture
Picture white level
Slider: 0 to Max
(No comment)

Brightness
Picture black level
Slider: 0 to Max
(No comment)

Color
Color saturation
Slider: 0 to Max
"UV Saturation"

Hue
Green/Red Tones
Slider: R 31 to 0 to G 31
Called "Tint" on older NTSC TV's, it doesn't give complete control over hue

Sharpness
Picture detail
Slider: 0 to Max
"Contrast"

Color Temp
Color Temperature
Enumerated options:
	Cool
	Bluish-white
	like a "daylight" light bulb

	Neutral
	Neutral
	(No comment)

	Warm
	Reddish-White (NTSC standard)
	like a "soft white" light bulb
Dominant color of light in the image. "Temperature" corresponds to the mean color of light from a blackbody radiator at such a temperature IIRC

ClearEdge VM
Edge enhancement
Enumerated options: High, Medium, Low, Off
(No comment)



Advanced Video controls available via the normal user menu:


Color Axis
Adjust red color axis
Enumerated values:
	Default
	Emphasize red colors
	(No comment)

	Monitor
	De-emphasize red colors
	(No comment)
I have no idea what this control is good for.

Noise Reduction
Reduces repetative random noise
Boolean: on/off
I suspect this may be Luma and Chroma coring.  I'm not sure.



The Service mode menu is available via pushing buttons on the remote:
DISPLAY, 5, VOL +, POWER ON

The service control and status options are cryptic and numerous.  I
suppose setting these wrong and saving would be akin to writing the
wrong values in the I2C EEPROM on one's video card.  (I will note the QM
INFO menu has a fascinating amount of status available.)

Regards,
Andy

Re: Chroma gain configuration

[Devin Heitmueller]

On Tue, Feb 23, 2010 at 2:53 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> Control enumeration is actually working fine.  The queryctrl does
>> properly return all of the controls, including my new private control.
>
> OK. So the problem is that v4l2-ctl uses G/S_EXT_CTRLS for non-user controls,
> right? Why not change v4l2-ctl: let it first try the EXT version but if that
> fails with EINVAL then try the old control API.

For what it's worth, I actually bisected the v4l2-ctl.cpp, and found
out the breakage got introduced in rev 12546:

==
v4l2-ctl: add support for string controls

From: Hans Verkuil <hverkuil@xs4all.nl>

Add support for string controls to v4l2-ctl.
Also refactor the code to generalize the handling of control classes.

Priority: normal
==

And this change breaks the v4l2-ctl application not just with my
driver but with *any* of the drivers which have private controls
implemented in g_ctrl or s_ctrl (including bttv, saa7134, and pwc)

The root of the issue is that private controls are not considered
"user controls".  Hence when getting or setting the control, the
v4l2-ctl app will always insist on attempting to use the
g_ext_ctrls/s_ext_ctrls ioctl calls, even though the underlying driver
doesn't have them implemented as extended controls.

The enumeration of all of control values (using the "-l" argument)
does include the private controls properly because the logic is
written such that it always uses g_ctrl for all cases where the
control ID >= V4L2_CID_PRIVATE_BASE.

I guess I'll see whether I can rework the logic a bit such that the
set/get routines work in a comparable manner to the routine to
enumerate all the controls.  I would prefer to avoid making the
g_ext_ctrls ioctl() call and then retrying it as g_ctrl if it fails,
since that will cause errors to be printed to the screen (due to the
abstraction of doioctl() function) and is generally a bad practice.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com