All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Extended driver private controls?
       [not found] <494B59B5.2030800@nokia.com>
@ 2008-12-19 14:48 ` Hans Verkuil
  2009-01-08 15:46   ` Sakari Ailus
  0 siblings, 1 reply; 4+ messages in thread
From: Hans Verkuil @ 2008-12-19 14:48 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: v4l, Tuukka.O Toivonen, linux-omap@vger.kernel.org

Hi Sakari,

On Friday 19 December 2008 09:22:13 Sakari Ailus wrote:
> Hello,
>
> I'm wondering how to use extended driver private controls. Controls
> starting from V4L2_CID_PRIVATE_BASE do not work (check_ext_ctrls() in
> v4l2-ioctl.c:
>
> ---
>          /* V4L2_CID_PRIVATE_BASE cannot be used as control class
>             when using extended controls.
>             Only when passed in through VIDIOC_G_CTRL and VIDIOC_S_CTRL
>             is it allowed for backwards compatibility.
>           */
>          if (!allow_priv && c->ctrl_class == V4L2_CID_PRIVATE_BASE)
>                  return 0;
> ---
>
> check_ext_ctrls() is called from __video_do_ioctl() with allow_priv set
> to zero for extended controls.
>
> I know I could create another control class and define that in
> videodev2.h but that feels like a bit of overkill for two integer
> controls... :-) Are there any alternatives if I would like my private
> controls to be available through extended controls as well?

Good question. The situation is as follows: the whole PRIVATE_BASE stuff is 
a really bad design. Controls are part of the public API of V4L2 and as 
such should be documented in the v4l2 spec and clearly defined. Using 
PRIVATE_BASE, however, means that different drivers will have controls with 
the same ID but that do different things. Furthermore, there is no way to 
classify controls like that (e.g. is it a general user control, an mpeg 
control, a camera control?).

So when I designed the extended controls API I wanted to force developers to 
use a different approach. If you look at videodev2.h you will see that a 
control ID is divided up in bitranges:

Bits 28-31 are for flags (currently on V4L2_CTRL_FLAG_NEXT_CTRL is used).

Bits 16-27 denote the class (USER, MPEG and CAMERA are currently defined).

Bits 0-15 are the control ID within the class.

In addition there is the guarantee that bits 0-15 can never be all 0 for any 
valid control.

The current PRIVATE_BASE stuff doesn't really fit in any of this.

What I did to support private controls is to allow drivers to add their own 
controls to the various classes, as long as the low 16 bits are >= 0x1000.

You add them to videodev2.h (see the CX2341X private controls), and you just 
pick a starting base such as 'V4L2_CTRL_CLASS_MPEG | 0x1100'.

This way the controls are together in one header, they have unique IDs, and 
you can assign them to a specific class.

Unfortunately, there are three obstacles:

1) Many apps and drivers still don't use V4L2_CTRL_FLAG_NEXT_CTRL and only 
know about PRIVATE_BASE.
2) It's a bit tricky to implement correct control handling in v4l drivers.
3) Documenting is a bit hard at the moment since the V4L2 spec is maintained 
elsewhere.

Since drivers that implement VIDIOC_S_EXT_CTRLS are generally new drivers 
I've blocked the use of PRIVATE_BASE with that ioctl, hopefully encouraging 
driver writers to properly define their controls in videodev2.h, document 
them in the v4l2 spec and to implement V4L2_CTRL_FLAG_NEXT_CTRL handling.

That doesn't address the fact that many applications still don't know about 
it. It depends on what sort of environment the driver will be used in 
whether you need to add duplicate PRIVATE_BASE controls for the time being. 
When I created the MPEG controls I didn't make duplicates since any 
application that needed to support MPEG had to make changes anyway. In the 
case of embedded platforms you probably have a similar situation. But if 
you make a simple TV grabber driver, then many apps like xawtv or tvtime 
only know about PRIVATE_BASE controls. In that case the only solution is to 
provide a duplicate set of PRIVATE_BASE controls.

The other disadvantage is that it is more work at the moment. I hope to 
address that in the coming months by adding proper control support to the 
v4l2 framework. I have several ideas for this and now that v4l2_device and 
v4l2_subdev are both in it should be quite feasible. If it all works as I 
hope then the driver control support will become much, much simpler.

The final problem relates to the fact that the documentation is currently 
maintained out-of-tree. We want to move it all to the repository in 1-2 
months from now and that should make it much easier to update the docs when 
you add new private controls.

So, to summarize: for new drivers I recommend using V4L2_CTRL_FLAG_NEXT_CTRL 
and adding the private controls to videodev2.h and to prepare documentation 
for inclusion with the v4l2 spec. If you really must support old apps, then 
you might have to make duplicate PRIVATE_BASE controls (yuck!), but I hope 
that won't be necessary.

It's not an ideal situation, but I hope that it will improve considerably in 
the next 3-6 months.

Regards,

	Hans

PS: please note that there are several control utility functions in 
v4l2-common.c! These might prove useful.

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Extended driver private controls?
  2008-12-19 14:48 ` Extended driver private controls? Hans Verkuil
@ 2009-01-08 15:46   ` Sakari Ailus
  2009-01-08 16:30     ` Aguirre Rodriguez, Sergio Alberto
  0 siblings, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2009-01-08 15:46 UTC (permalink / raw)
  To: ext Hans Verkuil; +Cc: v4l, Tuukka.O Toivonen, linux-omap@vger.kernel.org

ext Hans Verkuil wrote:
> Hi Sakari,

Hello,

...
> What I did to support private controls is to allow drivers to add their own 
> controls to the various classes, as long as the low 16 bits are >= 0x1000.
> 
> You add them to videodev2.h (see the CX2341X private controls), and you just 
> pick a starting base such as 'V4L2_CTRL_CLASS_MPEG | 0x1100'.
> 
> This way the controls are together in one header, they have unique IDs, and 
> you can assign them to a specific class.
...
> So, to summarize: for new drivers I recommend using V4L2_CTRL_FLAG_NEXT_CTRL 
> and adding the private controls to videodev2.h and to prepare documentation 
> for inclusion with the v4l2 spec. If you really must support old apps, then 
> you might have to make duplicate PRIVATE_BASE controls (yuck!), but I hope 
> that won't be necessary.

Thanks, Hans, for the definitive answer. This is what we'll do then.

Cheers,

-- 
Sakari Ailus
sakari.ailus@nokia.com


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: Extended driver private controls?
  2009-01-08 15:46   ` Sakari Ailus
@ 2009-01-08 16:30     ` Aguirre Rodriguez, Sergio Alberto
  2009-01-08 17:15       ` Hans Verkuil
  0 siblings, 1 reply; 4+ messages in thread
From: Aguirre Rodriguez, Sergio Alberto @ 2009-01-08 16:30 UTC (permalink / raw)
  To: Sakari Ailus, ext Hans Verkuil
  Cc: v4l, Tuukka.O Toivonen, linux-omap@vger.kernel.org



> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Sakari Ailus
> Sent: Thursday, January 08, 2009 9:47 AM
> ext Hans Verkuil wrote:
> > Hi Sakari,
> 
> Hello,
> 
> ...
> > What I did to support private controls is to allow drivers to add their
> own
> > controls to the various classes, as long as the low 16 bits are >=
> 0x1000.
> >
> > You add them to videodev2.h (see the CX2341X private controls), and you
> just
> > pick a starting base such as 'V4L2_CTRL_CLASS_MPEG | 0x1100'.
> >
> > This way the controls are together in one header, they have unique IDs,
> and
> > you can assign them to a specific class.
> ...
> > So, to summarize: for new drivers I recommend using
> V4L2_CTRL_FLAG_NEXT_CTRL
> > and adding the private controls to videodev2.h and to prepare
> documentation
> > for inclusion with the v4l2 spec. If you really must support old apps,
> then
> > you might have to make duplicate PRIVATE_BASE controls (yuck!), but I
> hope
> > that won't be necessary.
> 
> Thanks, Hans, for the definitive answer. This is what we'll do then.
> 


Hi,

Just for clarifying: So this replaces the use of PRIVATE_BASE?:

CLASS_MPEG + 0x1000 + Custom number

Or its like:

PRIVATE_BASE + CLASS_MPEG + 0x1000 + Custom number ?

Thanks,
Sergio

> Cheers,
> 
> --
> Sakari Ailus
> sakari.ailus@nokia.com
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Extended driver private controls?
  2009-01-08 16:30     ` Aguirre Rodriguez, Sergio Alberto
@ 2009-01-08 17:15       ` Hans Verkuil
  0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2009-01-08 17:15 UTC (permalink / raw)
  To: Aguirre Rodriguez, Sergio Alberto
  Cc: Sakari Ailus, v4l, Tuukka.O Toivonen, linux-omap@vger.kernel.org

On Thursday 08 January 2009 17:30:47 Aguirre Rodriguez, Sergio Alberto 
wrote:
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Sakari Ailus
> > Sent: Thursday, January 08, 2009 9:47 AM
> >
> > ext Hans Verkuil wrote:
> > > Hi Sakari,
> >
> > Hello,
> >
> > ...
> >
> > > What I did to support private controls is to allow drivers to add
> > > their
> >
> > own
> >
> > > controls to the various classes, as long as the low 16 bits are
> > > >=
> >
> > 0x1000.
> >
> > > You add them to videodev2.h (see the CX2341X private controls),
> > > and you
> >
> > just
> >
> > > pick a starting base such as 'V4L2_CTRL_CLASS_MPEG | 0x1100'.
> > >
> > > This way the controls are together in one header, they have
> > > unique IDs,
> >
> > and
> >
> > > you can assign them to a specific class.
> >
> > ...
> >
> > > So, to summarize: for new drivers I recommend using
> >
> > V4L2_CTRL_FLAG_NEXT_CTRL
> >
> > > and adding the private controls to videodev2.h and to prepare
> >
> > documentation
> >
> > > for inclusion with the v4l2 spec. If you really must support old
> > > apps,
> >
> > then
> >
> > > you might have to make duplicate PRIVATE_BASE controls (yuck!),
> > > but I
> >
> > hope
> >
> > > that won't be necessary.
> >
> > Thanks, Hans, for the definitive answer. This is what we'll do
> > then.
>
> Hi,
>
> Just for clarifying: So this replaces the use of PRIVATE_BASE?:
>
> CLASS_MPEG + 0x1000 + Custom number

Yes, this is the right one.


>
> Or its like:
>
> PRIVATE_BASE + CLASS_MPEG + 0x1000 + Custom number ?

No, forget about PRIVATE_BASE.

Regards,

	Hans

>
> Thanks,
> Sergio
>
> > Cheers,
> >
> > --
> > Sakari Ailus
> > sakari.ailus@nokia.com
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-omap" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2009-01-08 17:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <494B59B5.2030800@nokia.com>
2008-12-19 14:48 ` Extended driver private controls? Hans Verkuil
2009-01-08 15:46   ` Sakari Ailus
2009-01-08 16:30     ` Aguirre Rodriguez, Sergio Alberto
2009-01-08 17:15       ` Hans Verkuil

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.