All of lore.kernel.org
 help / color / mirror / Atom feed
* RFC: exposing controls in sysfs
@ 2010-04-05 21:47 Hans Verkuil
  2010-04-05 22:12 ` Hans Verkuil
  2010-04-06 14:33 ` Mike Isely
  0 siblings, 2 replies; 28+ messages in thread
From: Hans Verkuil @ 2010-04-05 21:47 UTC (permalink / raw)
  To: linux-media; +Cc: Mike Isely

Hi all,

The new control framework makes it very easy to expose controls in sysfs.
The way it is implemented now in the framework is that each device node
will get a 'controls' subdirectory in sysfs. Below which are all the controls
associated with that device node.

So different device nodes can have different controls if so desired.

The name of each sysfs file is derived from the control name, basically making
it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non-
alphanumerical characters. Seems to work well.

For numerical controls you can write numbers in decimal, octal or hexadecimal.

When you write to a button control it will ignore what you wrote, but still
execute the action.

It looks like this for ivtv:

$ ls /sys/class/video4linux/video1
controls  dev  device  index  name  power  subsystem  uevent

$ ls /sys/class/video4linux/video1/controls
audio_crc                    chroma_gain                   spatial_chroma_filter_type  video_bitrate_mode
audio_emphasis               contrast                      spatial_filter              video_encoding
audio_encoding               hue                           spatial_filter_mode         video_gop_closure
audio_layer_ii_bitrate       insert_navigation_packets     spatial_luma_filter_type    video_gop_size
audio_mute                   median_chroma_filter_maximum  stream_type                 video_mute
audio_sampling_frequency     median_chroma_filter_minimum  stream_vbi_format           video_mute_yuv
audio_stereo_mode            median_filter_type            temporal_filter             video_peak_bitrate
audio_stereo_mode_extension  median_luma_filter_maximum    temporal_filter_mode        video_temporal_decimation
balance                      median_luma_filter_minimum    video_aspect                volume
brightness                   mute                          video_b_frames
chroma_agc                   saturation                    video_bitrate


The question is, is this sufficient?

One of the few drivers that exposes controls in sysfs is pvrusb2. As far as
I can tell from the source it will create subdirectories under the device
node for each control. Those subdirs have the name ctl_<control-name> (e.g.
ctl_volume), and below that are files exposing all the attributes of that
control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val
and bit_val. Most are clear, but some are a bit more obscure. enum_val is
basically a QUERYMENU and returns all menu options. bit_val seems to be used
for some non-control values like the TV standard that pvrusb2 also exposes
and where bit_val is a bit mask of all the valid bits that can be used.

Mike, if you have any additional information, just let us know. My pvrusb2
is in another country at the moment so I can't do any testing.

Personally I think that it is overkill to basically expose the whole
QUERYCTRL information to sysfs. I see it as an easy and quick way to read and
modify controls via a command line.

Mike, do you know of anyone actively using that additional information?

And which non-control values do you at the moment expose in pvrusb2 through
sysfs? Can you perhaps give an ls -R of all the files you create in sysfs
for pvrusb2?

I am wondering whether some of those should get a place in the framework as
well. While I don't think e.g. cropping parameters are useful, things like the
current input or tuner frequency can be handy. However, for those to be useful
they would have to be wired up internally through the framework. For example,
VIDIOC_S_FREQUENCY would have to be hooked up internally to a control. That
would ensure that however you access it (ioctl or sysfs) it will both end up
in the same s_ctrl function.

It will be nice to hear from you what your experience is.

Comments? Ideas? Once we commit to something it is there for a long time to
come since sysfs is after all a public API (although it seems to be more
changable than ioctls).

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

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

* Re: RFC: exposing controls in sysfs
  2010-04-05 21:47 RFC: exposing controls in sysfs Hans Verkuil
@ 2010-04-05 22:12 ` Hans Verkuil
  2010-04-06  6:37   ` Hans Verkuil
  2010-04-06 14:41   ` Mike Isely
  2010-04-06 14:33 ` Mike Isely
  1 sibling, 2 replies; 28+ messages in thread
From: Hans Verkuil @ 2010-04-05 22:12 UTC (permalink / raw)
  To: linux-media; +Cc: Mike Isely

On Monday 05 April 2010 23:47:10 Hans Verkuil wrote:
> Hi all,
> 
> The new control framework makes it very easy to expose controls in sysfs.
> The way it is implemented now in the framework is that each device node
> will get a 'controls' subdirectory in sysfs. Below which are all the controls
> associated with that device node.
> 
> So different device nodes can have different controls if so desired.
> 
> The name of each sysfs file is derived from the control name, basically making
> it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non-
> alphanumerical characters. Seems to work well.
> 
> For numerical controls you can write numbers in decimal, octal or hexadecimal.
> 
> When you write to a button control it will ignore what you wrote, but still
> execute the action.
> 
> It looks like this for ivtv:
> 
> $ ls /sys/class/video4linux/video1
> controls  dev  device  index  name  power  subsystem  uevent
> 
> $ ls /sys/class/video4linux/video1/controls
> audio_crc                    chroma_gain                   spatial_chroma_filter_type  video_bitrate_mode
> audio_emphasis               contrast                      spatial_filter              video_encoding
> audio_encoding               hue                           spatial_filter_mode         video_gop_closure
> audio_layer_ii_bitrate       insert_navigation_packets     spatial_luma_filter_type    video_gop_size
> audio_mute                   median_chroma_filter_maximum  stream_type                 video_mute
> audio_sampling_frequency     median_chroma_filter_minimum  stream_vbi_format           video_mute_yuv
> audio_stereo_mode            median_filter_type            temporal_filter             video_peak_bitrate
> audio_stereo_mode_extension  median_luma_filter_maximum    temporal_filter_mode        video_temporal_decimation
> balance                      median_luma_filter_minimum    video_aspect                volume
> brightness                   mute                          video_b_frames
> chroma_agc                   saturation                    video_bitrate
> 
> 
> The question is, is this sufficient?

One thing that might be useful is to prefix the name with the control class
name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
groups them better. Or one could make a controls/user and controls/mpeg
directory. That might not be such a bad idea actually.

> One of the few drivers that exposes controls in sysfs is pvrusb2. As far as
> I can tell from the source it will create subdirectories under the device
> node for each control. Those subdirs have the name ctl_<control-name> (e.g.
> ctl_volume), and below that are files exposing all the attributes of that
> control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val
> and bit_val. Most are clear, but some are a bit more obscure. enum_val is
> basically a QUERYMENU and returns all menu options. bit_val seems to be used
> for some non-control values like the TV standard that pvrusb2 also exposes
> and where bit_val is a bit mask of all the valid bits that can be used.
> 
> Mike, if you have any additional information, just let us know. My pvrusb2
> is in another country at the moment so I can't do any testing.
> 
> Personally I think that it is overkill to basically expose the whole
> QUERYCTRL information to sysfs. I see it as an easy and quick way to read and
> modify controls via a command line.

An in between solution would be to add _type files. So you would have 'hue' and
'hue_type'. 'cat hue_type' would give something like:

int 0 255 1 128 0x0000 Hue

In other words 'type min max step flags name'.

And for menu controls like stream_type (hmm, that would become stream_type_type...)
you would get:

menu 0 5 1 0 Stream Type
MPEG-2 Program Stream

MPEG-1 System Stream
MPEG-2 DVD-compatible Stream
MPEG-1 VCD-compatible Stream
MPEG-2 SVCD-compatible Stream

Note the empty line to denote the unsupported menu item (transport stream).

This would give the same information with just a single extra file. Still not
sure whether it is worth it though.

Regards,

	Hans

> 
> Mike, do you know of anyone actively using that additional information?
> 
> And which non-control values do you at the moment expose in pvrusb2 through
> sysfs? Can you perhaps give an ls -R of all the files you create in sysfs
> for pvrusb2?
> 
> I am wondering whether some of those should get a place in the framework as
> well. While I don't think e.g. cropping parameters are useful, things like the
> current input or tuner frequency can be handy. However, for those to be useful
> they would have to be wired up internally through the framework. For example,
> VIDIOC_S_FREQUENCY would have to be hooked up internally to a control. That
> would ensure that however you access it (ioctl or sysfs) it will both end up
> in the same s_ctrl function.
> 
> It will be nice to hear from you what your experience is.
> 
> Comments? Ideas? Once we commit to something it is there for a long time to
> come since sysfs is after all a public API (although it seems to be more
> changable than ioctls).
> 
> Regards,
> 
> 	Hans
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

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

* Re: RFC: exposing controls in sysfs
  2010-04-05 22:12 ` Hans Verkuil
@ 2010-04-06  6:37   ` Hans Verkuil
  2010-04-06 11:06     ` Andy Walls
  2010-04-06 13:16     ` Mauro Carvalho Chehab
  2010-04-06 14:41   ` Mike Isely
  1 sibling, 2 replies; 28+ messages in thread
From: Hans Verkuil @ 2010-04-06  6:37 UTC (permalink / raw)
  To: linux-media; +Cc: Mike Isely

On Tuesday 06 April 2010 00:12:48 Hans Verkuil wrote:
> On Monday 05 April 2010 23:47:10 Hans Verkuil wrote:
> > Hi all,
> > 
> > The new control framework makes it very easy to expose controls in sysfs.
> > The way it is implemented now in the framework is that each device node
> > will get a 'controls' subdirectory in sysfs. Below which are all the controls
> > associated with that device node.
> > 
> > So different device nodes can have different controls if so desired.
> > 
> > The name of each sysfs file is derived from the control name, basically making
> > it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non-
> > alphanumerical characters. Seems to work well.
> > 
> > For numerical controls you can write numbers in decimal, octal or hexadecimal.
> > 
> > When you write to a button control it will ignore what you wrote, but still
> > execute the action.
> > 
> > It looks like this for ivtv:
> > 
> > $ ls /sys/class/video4linux/video1
> > controls  dev  device  index  name  power  subsystem  uevent
> > 
> > $ ls /sys/class/video4linux/video1/controls
> > audio_crc                    chroma_gain                   spatial_chroma_filter_type  video_bitrate_mode
> > audio_emphasis               contrast                      spatial_filter              video_encoding
> > audio_encoding               hue                           spatial_filter_mode         video_gop_closure
> > audio_layer_ii_bitrate       insert_navigation_packets     spatial_luma_filter_type    video_gop_size
> > audio_mute                   median_chroma_filter_maximum  stream_type                 video_mute
> > audio_sampling_frequency     median_chroma_filter_minimum  stream_vbi_format           video_mute_yuv
> > audio_stereo_mode            median_filter_type            temporal_filter             video_peak_bitrate
> > audio_stereo_mode_extension  median_luma_filter_maximum    temporal_filter_mode        video_temporal_decimation
> > balance                      median_luma_filter_minimum    video_aspect                volume
> > brightness                   mute                          video_b_frames
> > chroma_agc                   saturation                    video_bitrate
> > 
> > 
> > The question is, is this sufficient?
> 
> One thing that might be useful is to prefix the name with the control class
> name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
> groups them better. Or one could make a controls/user and controls/mpeg
> directory. That might not be such a bad idea actually.

Replying to your own mails is probably a bad sign, but I can't help myself :-)

I've changed the code to add a control class prefix for all but the user controls.
It looks much better now:

$ ls /sys/class/video4linux/video1/controls
balance                           mpeg_insert_navigation_packets     mpeg_video_aspect
brightness                        mpeg_median_chroma_filter_maximum  mpeg_video_b_frames
chroma_agc                        mpeg_median_chroma_filter_minimum  mpeg_video_bitrate
chroma_gain                       mpeg_median_filter_type            mpeg_video_bitrate_mode
contrast                          mpeg_median_luma_filter_maximum    mpeg_video_encoding
hue                               mpeg_median_luma_filter_minimum    mpeg_video_gop_closure
mpeg_audio_crc                    mpeg_spatial_chroma_filter_type    mpeg_video_gop_size
mpeg_audio_emphasis               mpeg_spatial_filter                mpeg_video_mute
mpeg_audio_encoding               mpeg_spatial_filter_mode           mpeg_video_mute_yuv
mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type      mpeg_video_peak_bitrate
mpeg_audio_mute                   mpeg_stream_type                   mpeg_video_temporal_decimation
mpeg_audio_sampling_frequency     mpeg_stream_vbi_format             mute
mpeg_audio_stereo_mode            mpeg_temporal_filter               saturation
mpeg_audio_stereo_mode_extension  mpeg_temporal_filter_mode          volume

> > One of the few drivers that exposes controls in sysfs is pvrusb2. As far as
> > I can tell from the source it will create subdirectories under the device
> > node for each control. Those subdirs have the name ctl_<control-name> (e.g.
> > ctl_volume), and below that are files exposing all the attributes of that
> > control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val
> > and bit_val. Most are clear, but some are a bit more obscure. enum_val is
> > basically a QUERYMENU and returns all menu options. bit_val seems to be used
> > for some non-control values like the TV standard that pvrusb2 also exposes
> > and where bit_val is a bit mask of all the valid bits that can be used.
> > 
> > Mike, if you have any additional information, just let us know. My pvrusb2
> > is in another country at the moment so I can't do any testing.
> > 
> > Personally I think that it is overkill to basically expose the whole
> > QUERYCTRL information to sysfs. I see it as an easy and quick way to read and
> > modify controls via a command line.
> 
> An in between solution would be to add _type files. So you would have 'hue' and
> 'hue_type'. 'cat hue_type' would give something like:

If we go for something like this, then I think it would be better to make a
new subdirectory. So 'controls' just has the controls, and 'ctrl_info' or
something similar would have read-only files containing this information.

Again, I still don't know whether we should do this. It is dangerously
seductive because it would be so trivial to implement.

Regards,

	Hans

> 
> int 0 255 1 128 0x0000 Hue
> 
> In other words 'type min max step flags name'.
> 
> And for menu controls like stream_type (hmm, that would become stream_type_type...)
> you would get:
> 
> menu 0 5 1 0 Stream Type
> MPEG-2 Program Stream
> 
> MPEG-1 System Stream
> MPEG-2 DVD-compatible Stream
> MPEG-1 VCD-compatible Stream
> MPEG-2 SVCD-compatible Stream
> 
> Note the empty line to denote the unsupported menu item (transport stream).
> 
> This would give the same information with just a single extra file. Still not
> sure whether it is worth it though.
> 
> Regards,
> 
> 	Hans
> 
> > 
> > Mike, do you know of anyone actively using that additional information?
> > 
> > And which non-control values do you at the moment expose in pvrusb2 through
> > sysfs? Can you perhaps give an ls -R of all the files you create in sysfs
> > for pvrusb2?
> > 
> > I am wondering whether some of those should get a place in the framework as
> > well. While I don't think e.g. cropping parameters are useful, things like the
> > current input or tuner frequency can be handy. However, for those to be useful
> > they would have to be wired up internally through the framework. For example,
> > VIDIOC_S_FREQUENCY would have to be hooked up internally to a control. That
> > would ensure that however you access it (ioctl or sysfs) it will both end up
> > in the same s_ctrl function.
> > 
> > It will be nice to hear from you what your experience is.
> > 
> > Comments? Ideas? Once we commit to something it is there for a long time to
> > come since sysfs is after all a public API (although it seems to be more
> > changable than ioctls).
> > 
> > Regards,
> > 
> > 	Hans
> > 
> > 
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

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

* Re: RFC: exposing controls in sysfs
  2010-04-06  6:37   ` Hans Verkuil
@ 2010-04-06 11:06     ` Andy Walls
  2010-04-06 11:27       ` Laurent Pinchart
  2010-04-06 13:16     ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 28+ messages in thread
From: Andy Walls @ 2010-04-06 11:06 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Mike Isely

On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:
> On Tuesday 06 April 2010 00:12:48 Hans Verkuil wrote:
> > On Monday 05 April 2010 23:47:10 Hans Verkuil wrote:

 
> > One thing that might be useful is to prefix the name with the control class
> > name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
> > groups them better. Or one could make a controls/user and controls/mpeg
> > directory. That might not be such a bad idea actually.
> 
> Replying to your own mails is probably a bad sign, but I can't help myself :-)

I had an old InfoCom text adventure game that would respond to querying
oneself with: "Talking to oneself is a sign of impending mental
collapse."

:D


> I've changed the code to add a control class prefix for all but the user controls.
> It looks much better now:
> 
> $ ls /sys/class/video4linux/video1/controls
> balance                           mpeg_insert_navigation_packets     mpeg_video_aspect
> brightness                        mpeg_median_chroma_filter_maximum  mpeg_video_b_frames
> chroma_agc                        mpeg_median_chroma_filter_minimum  mpeg_video_bitrate
> chroma_gain                       mpeg_median_filter_type            mpeg_video_bitrate_mode
> contrast                          mpeg_median_luma_filter_maximum    mpeg_video_encoding
> hue                               mpeg_median_luma_filter_minimum    mpeg_video_gop_closure
> mpeg_audio_crc                    mpeg_spatial_chroma_filter_type    mpeg_video_gop_size
> mpeg_audio_emphasis               mpeg_spatial_filter                mpeg_video_mute
> mpeg_audio_encoding               mpeg_spatial_filter_mode           mpeg_video_mute_yuv
> mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type      mpeg_video_peak_bitrate
> mpeg_audio_mute                   mpeg_stream_type                   mpeg_video_temporal_decimation
> mpeg_audio_sampling_frequency     mpeg_stream_vbi_format             mute
> mpeg_audio_stereo_mode            mpeg_temporal_filter               saturation
> mpeg_audio_stereo_mode_extension  mpeg_temporal_filter_mode          volume

So this is beginning to look OK.  You'll have longer names when a class
name is longer than 4 characters (e.g. "technician_" ).  However, I
suppose it is better than another directory which creates a deeper
hierarchy while still not avoiding the longer pathname.



> > > One of the few drivers that exposes controls in sysfs is pvrusb2. As far as
> > > I can tell from the source it will create subdirectories under the device
> > > node for each control. Those subdirs have the name ctl_<control-name> (e.g.
> > > ctl_volume), and below that are files exposing all the attributes of that
> > > control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val
> > > and bit_val. Most are clear, but some are a bit more obscure. enum_val is
> > > basically a QUERYMENU and returns all menu options. bit_val seems to be used
> > > for some non-control values like the TV standard that pvrusb2 also exposes
> > > and where bit_val is a bit mask of all the valid bits that can be used.
> > > 
> > > Mike, if you have any additional information, just let us know. My pvrusb2
> > > is in another country at the moment so I can't do any testing.
> > > 
> > > Personally I think that it is overkill to basically expose the whole
> > > QUERYCTRL information to sysfs. I see it as an easy and quick way to read and
> > > modify controls via a command line.


> > An in between solution would be to add _type files. So you would have 'hue' and
> > 'hue_type'. 'cat hue_type' would give something like:
> 
> If we go for something like this, then I think it would be better to make a
> new subdirectory. So 'controls' just has the controls, and 'ctrl_info' or
> something similar would have read-only files containing this information.

sysfs' major usability problem for humans is the insane directory depths
it can reach and the cross-links to everywhere.  Humans attempt to keep
a mental model of "where" they are in a logical "space", and sysfs is
like "maze of twisty little passages, all alike".

In the true sysfs spirit you should create a 'ctrl_info' directory full
of nodes with metadata *and* also create "foo_type" symlinks to all of
those metadata nodes.  Bonus points for having the 'ctrl_info' directory
and 'foo_type' symlinks in a different part of the sysfs tree but with a
similar directory name.


> Again, I still don't know whether we should do this. It is dangerously
> seductive because it would be so trivial to implement.

It's like watching ships run aground on a shallow sandbar that all the
locals know about.  The waters off of 'Point /sys' are full of usability
shipwrecks.  I don't know if it's some siren's song, the lack of a light
house, or just strange currents that deceive even seasoned
navigators....


Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control
metatdata.  It's not as easy as typing 'cat', but the user base using
sysfs in an interactive shell or shell script should also know how to
use v4l2-ctl.  In embedded systems, the final system deployment should
not need the control metadata available from sysfs in a command shell
anyway.


Regards,
Andy


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

* Re: RFC: exposing controls in sysfs
  2010-04-06 11:06     ` Andy Walls
@ 2010-04-06 11:27       ` Laurent Pinchart
  2010-04-06 11:44         ` Markus Rechberger
  2010-04-06 15:16         ` Mike Isely
  0 siblings, 2 replies; 28+ messages in thread
From: Laurent Pinchart @ 2010-04-06 11:27 UTC (permalink / raw)
  To: Andy Walls; +Cc: Hans Verkuil, linux-media, Mike Isely

Hi Andy,

On Tuesday 06 April 2010 13:06:18 Andy Walls wrote:
> On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:

[snip]

> > Again, I still don't know whether we should do this. It is dangerously
> > seductive because it would be so trivial to implement.
> 
> It's like watching ships run aground on a shallow sandbar that all the
> locals know about.  The waters off of 'Point /sys' are full of usability
> shipwrecks.  I don't know if it's some siren's song, the lack of a light
> house, or just strange currents that deceive even seasoned
> navigators....
> 
> Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control
> metatdata.  It's not as easy as typing 'cat', but the user base using
> sysfs in an interactive shell or shell script should also know how to
> use v4l2-ctl.  In embedded systems, the final system deployment should
> not need the control metadata available from sysfs in a command shell
> anyway.

I fully agree with this. If we push the idea one step further, why do we need 
to expose controls in sysfs at all ?

-- 
Regards,

Laurent Pinchart

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 11:27       ` Laurent Pinchart
@ 2010-04-06 11:44         ` Markus Rechberger
  2010-04-06 15:08           ` Mike Isely
  2010-04-06 15:16         ` Mike Isely
  1 sibling, 1 reply; 28+ messages in thread
From: Markus Rechberger @ 2010-04-06 11:44 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Andy Walls, Hans Verkuil, linux-media, Mike Isely

On Tue, Apr 6, 2010 at 1:27 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Andy,
>
> On Tuesday 06 April 2010 13:06:18 Andy Walls wrote:
>> On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:
>
> [snip]
>
>> > Again, I still don't know whether we should do this. It is dangerously
>> > seductive because it would be so trivial to implement.
>>
>> It's like watching ships run aground on a shallow sandbar that all the
>> locals know about.  The waters off of 'Point /sys' are full of usability
>> shipwrecks.  I don't know if it's some siren's song, the lack of a light
>> house, or just strange currents that deceive even seasoned
>> navigators....
>>
>> Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control
>> metatdata.  It's not as easy as typing 'cat', but the user base using
>> sysfs in an interactive shell or shell script should also know how to
>> use v4l2-ctl.  In embedded systems, the final system deployment should
>> not need the control metadata available from sysfs in a command shell
>> anyway.
>
> I fully agree with this. If we push the idea one step further, why do we need
> to expose controls in sysfs at all ?
>

how about security permissions? while you can easily change the
permission levels for nodes in /dev you can't do this so easily with
sysfs entries.
I don't really think this is needed at all some applications will
start to use ioctl some other apps might
go for sysfs.. this makes the API a little bit whacko

Markus

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

* Re: RFC: exposing controls in sysfs
  2010-04-06  6:37   ` Hans Verkuil
  2010-04-06 11:06     ` Andy Walls
@ 2010-04-06 13:16     ` Mauro Carvalho Chehab
  2010-04-06 13:44       ` Hans Verkuil
  1 sibling, 1 reply; 28+ messages in thread
From: Mauro Carvalho Chehab @ 2010-04-06 13:16 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Mike Isely

Hans Verkuil wrote:
> On Tuesday 06 April 2010 00:12:48 Hans Verkuil wrote:
>> On Monday 05 April 2010 23:47:10 Hans Verkuil wrote:
>>> Hi all,
>>>
>>> The new control framework makes it very easy to expose controls in sysfs.
>>> The way it is implemented now in the framework is that each device node
>>> will get a 'controls' subdirectory in sysfs. Below which are all the controls
>>> associated with that device node.
>>>
>>> So different device nodes can have different controls if so desired.
>>>
>>> The name of each sysfs file is derived from the control name, basically making
>>> it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non-
>>> alphanumerical characters. Seems to work well.
>>>
>>> For numerical controls you can write numbers in decimal, octal or hexadecimal.
>>>
>>> When you write to a button control it will ignore what you wrote, but still
>>> execute the action.
>>>
>>> It looks like this for ivtv:
>>>
>>> $ ls /sys/class/video4linux/video1
>>> controls  dev  device  index  name  power  subsystem  uevent
>>>
>>> $ ls /sys/class/video4linux/video1/controls
>>> audio_crc                    chroma_gain                   spatial_chroma_filter_type  video_bitrate_mode
>>> audio_emphasis               contrast                      spatial_filter              video_encoding
>>> audio_encoding               hue                           spatial_filter_mode         video_gop_closure
>>> audio_layer_ii_bitrate       insert_navigation_packets     spatial_luma_filter_type    video_gop_size
>>> audio_mute                   median_chroma_filter_maximum  stream_type                 video_mute
>>> audio_sampling_frequency     median_chroma_filter_minimum  stream_vbi_format           video_mute_yuv
>>> audio_stereo_mode            median_filter_type            temporal_filter             video_peak_bitrate
>>> audio_stereo_mode_extension  median_luma_filter_maximum    temporal_filter_mode        video_temporal_decimation
>>> balance                      median_luma_filter_minimum    video_aspect                volume
>>> brightness                   mute                          video_b_frames
>>> chroma_agc                   saturation                    video_bitrate
>>>
>>>
>>> The question is, is this sufficient?
>> One thing that might be useful is to prefix the name with the control class
>> name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
>> groups them better. Or one could make a controls/user and controls/mpeg
>> directory. That might not be such a bad idea actually.
> 
> Replying to your own mails is probably a bad sign, but I can't help myself :-)
> 
> I've changed the code to add a control class prefix for all but the user controls.
> It looks much better now:
> 
> $ ls /sys/class/video4linux/video1/controls
> balance                           mpeg_insert_navigation_packets     mpeg_video_aspect
> brightness                        mpeg_median_chroma_filter_maximum  mpeg_video_b_frames
> chroma_agc                        mpeg_median_chroma_filter_minimum  mpeg_video_bitrate
> chroma_gain                       mpeg_median_filter_type            mpeg_video_bitrate_mode
> contrast                          mpeg_median_luma_filter_maximum    mpeg_video_encoding
> hue                               mpeg_median_luma_filter_minimum    mpeg_video_gop_closure
> mpeg_audio_crc                    mpeg_spatial_chroma_filter_type    mpeg_video_gop_size
> mpeg_audio_emphasis               mpeg_spatial_filter                mpeg_video_mute
> mpeg_audio_encoding               mpeg_spatial_filter_mode           mpeg_video_mute_yuv
> mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type      mpeg_video_peak_bitrate
> mpeg_audio_mute                   mpeg_stream_type                   mpeg_video_temporal_decimation
> mpeg_audio_sampling_frequency     mpeg_stream_vbi_format             mute
> mpeg_audio_stereo_mode            mpeg_temporal_filter               saturation
> mpeg_audio_stereo_mode_extension  mpeg_temporal_filter_mode          volume


It would be more intuitive if you group the classes with a few subdirs:

/video/balance
/video/brightness
...
/mpeg_audio/crc
/mpeg_audio/mute
...
/audio/volume
/audio/bass
/audio/treble
..

-- 

Cheers,
Mauro

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 13:16     ` Mauro Carvalho Chehab
@ 2010-04-06 13:44       ` Hans Verkuil
  2010-04-06 13:59         ` Devin Heitmueller
  2010-04-06 14:32         ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 28+ messages in thread
From: Hans Verkuil @ 2010-04-06 13:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-media, Mike Isely


> Hans Verkuil wrote:
>> $ ls /sys/class/video4linux/video1/controls
>> balance                           mpeg_insert_navigation_packets
>> mpeg_video_aspect
>> brightness                        mpeg_median_chroma_filter_maximum
>> mpeg_video_b_frames
>> chroma_agc                        mpeg_median_chroma_filter_minimum
>> mpeg_video_bitrate
>> chroma_gain                       mpeg_median_filter_type
>> mpeg_video_bitrate_mode
>> contrast                          mpeg_median_luma_filter_maximum
>> mpeg_video_encoding
>> hue                               mpeg_median_luma_filter_minimum
>> mpeg_video_gop_closure
>> mpeg_audio_crc                    mpeg_spatial_chroma_filter_type
>> mpeg_video_gop_size
>> mpeg_audio_emphasis               mpeg_spatial_filter
>> mpeg_video_mute
>> mpeg_audio_encoding               mpeg_spatial_filter_mode
>> mpeg_video_mute_yuv
>> mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type
>> mpeg_video_peak_bitrate
>> mpeg_audio_mute                   mpeg_stream_type
>> mpeg_video_temporal_decimation
>> mpeg_audio_sampling_frequency     mpeg_stream_vbi_format
>> mute
>> mpeg_audio_stereo_mode            mpeg_temporal_filter
>> saturation
>> mpeg_audio_stereo_mode_extension  mpeg_temporal_filter_mode
>> volume
>
>
> It would be more intuitive if you group the classes with a few subdirs:
>
> /video/balance
> /video/brightness
> ...
> /mpeg_audio/crc
> /mpeg_audio/mute
> ...
> /audio/volume
> /audio/bass
> /audio/treble

1) We don't have that information.
2) It would make a simple scheme suddenly a lot more complicated (see
Andy's comments)
3) The main interface is always the application's GUI through ioctls, not
sysfs.
4) Remember that ivtv has an unusually large number of controls. Most
drivers will just have the usual audio and video controls, perhaps 10 at
most.

Strife for simplicity. I'm not sure whether we want to have this in sysfs
at all. While nice there is a danger that people suddenly see it as the
main API. And Markus' comment regarding permissions was a good one, I
thought.

I think we should just ditch this for the first implementation of the
control framework. It can always be added later, but once added it is
*much* harder to remove again. It's a nice proof-of-concept, though :-)

Regards,

        Hans

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


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

* Re: RFC: exposing controls in sysfs
  2010-04-06 13:44       ` Hans Verkuil
@ 2010-04-06 13:59         ` Devin Heitmueller
  2010-04-06 15:05           ` Mike Isely
  2010-04-06 14:32         ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 28+ messages in thread
From: Devin Heitmueller @ 2010-04-06 13:59 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Mauro Carvalho Chehab, linux-media, Mike Isely

On Tue, Apr 6, 2010 at 9:44 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> 1) We don't have that information.
> 2) It would make a simple scheme suddenly a lot more complicated (see
> Andy's comments)
> 3) The main interface is always the application's GUI through ioctls, not
> sysfs.
> 4) Remember that ivtv has an unusually large number of controls. Most
> drivers will just have the usual audio and video controls, perhaps 10 at
> most.
>
> Strife for simplicity. I'm not sure whether we want to have this in sysfs
> at all. While nice there is a danger that people suddenly see it as the
> main API. And Markus' comment regarding permissions was a good one, I
> thought.
>
> I think we should just ditch this for the first implementation of the
> control framework. It can always be added later, but once added it is
> *much* harder to remove again. It's a nice proof-of-concept, though :-)

I tend to agree with Hans.  We've already got *too many* interfaces
that do the same thing.  The testing matrix is already a nightmare -
V4L1 versus V4L2, mmap() versus read(), legacy controls versus
extended controls, and don't get even me started on VBI.

We should be working to make drivers and interfaces simpler, with
*fewer* ways of doing the same thing.  The flexibility of providing
yet another interface via sysfs compared to just calling v4l2-ctl just
isn't worth the extra testing overhead.  We've already got too much
stuff that needs to be fixed and not enough good developers to warrant
making the code more complicated with little tangible benefit.

And nobody I've talked to who writes applications that work with V4L
has been screaming "OMG, if only V4L had a sysfs interface to manage
controls!"

Devin

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

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 13:44       ` Hans Verkuil
  2010-04-06 13:59         ` Devin Heitmueller
@ 2010-04-06 14:32         ` Mauro Carvalho Chehab
  2010-04-06 16:06           ` Jonathan Cameron
  1 sibling, 1 reply; 28+ messages in thread
From: Mauro Carvalho Chehab @ 2010-04-06 14:32 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Mike Isely

Hans Verkuil wrote:
>> Hans Verkuil wrote:
>>> $ ls /sys/class/video4linux/video1/controls
>>> balance                           mpeg_insert_navigation_packets
>>> mpeg_video_aspect
>>> brightness                        mpeg_median_chroma_filter_maximum
>>> mpeg_video_b_frames
>>> chroma_agc                        mpeg_median_chroma_filter_minimum
>>> mpeg_video_bitrate
>>> chroma_gain                       mpeg_median_filter_type
>>> mpeg_video_bitrate_mode
>>> contrast                          mpeg_median_luma_filter_maximum
>>> mpeg_video_encoding
>>> hue                               mpeg_median_luma_filter_minimum
>>> mpeg_video_gop_closure
>>> mpeg_audio_crc                    mpeg_spatial_chroma_filter_type
>>> mpeg_video_gop_size
>>> mpeg_audio_emphasis               mpeg_spatial_filter
>>> mpeg_video_mute
>>> mpeg_audio_encoding               mpeg_spatial_filter_mode
>>> mpeg_video_mute_yuv
>>> mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type
>>> mpeg_video_peak_bitrate
>>> mpeg_audio_mute                   mpeg_stream_type
>>> mpeg_video_temporal_decimation
>>> mpeg_audio_sampling_frequency     mpeg_stream_vbi_format
>>> mute
>>> mpeg_audio_stereo_mode            mpeg_temporal_filter
>>> saturation
>>> mpeg_audio_stereo_mode_extension  mpeg_temporal_filter_mode
>>> volume
>>
>> It would be more intuitive if you group the classes with a few subdirs:
>>
>> /video/balance
>> /video/brightness
>> ...
>> /mpeg_audio/crc
>> /mpeg_audio/mute
>> ...
>> /audio/volume
>> /audio/bass
>> /audio/treble
> 
> 1) We don't have that information.
> 2) It would make a simple scheme suddenly a lot more complicated (see
> Andy's comments)
> 3) The main interface is always the application's GUI through ioctls, not
> sysfs.
> 4) Remember that ivtv has an unusually large number of controls. Most
> drivers will just have the usual audio and video controls, perhaps 10 at
> most.

Ok.

> I think we should just ditch this for the first implementation of the
> control framework. It can always be added later, but once added it is
> *much* harder to remove again. It's a nice proof-of-concept, though :-)

I like the concept, especially if we can get rid of other similar sysfs interfaces
that got added on a few drivers (pvrusb2 and some non-gspca drivers have
it, for sure). I think I saw some of the gspca patches also touching on sysfs.
Having this unified into a common interface is a bonus.

-- 

Cheers,
Mauro

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

* Re: RFC: exposing controls in sysfs
  2010-04-05 21:47 RFC: exposing controls in sysfs Hans Verkuil
  2010-04-05 22:12 ` Hans Verkuil
@ 2010-04-06 14:33 ` Mike Isely
  2010-04-07 18:50   ` Lars Hanisch
  1 sibling, 1 reply; 28+ messages in thread
From: Mike Isely @ 2010-04-06 14:33 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media


Comments below...

On Mon, 5 Apr 2010, Hans Verkuil wrote:

> Hi all,
> 
> The new control framework makes it very easy to expose controls in sysfs.
> The way it is implemented now in the framework is that each device node
> will get a 'controls' subdirectory in sysfs. Below which are all the controls
> associated with that device node.
> 
> So different device nodes can have different controls if so desired.
> 
> The name of each sysfs file is derived from the control name, basically making
> it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non-
> alphanumerical characters. Seems to work well.
> 
> For numerical controls you can write numbers in decimal, octal or hexadecimal.
> 
> When you write to a button control it will ignore what you wrote, but still
> execute the action.
> 
> It looks like this for ivtv:
> 
> $ ls /sys/class/video4linux/video1
> controls  dev  device  index  name  power  subsystem  uevent
> 
> $ ls /sys/class/video4linux/video1/controls
> audio_crc                    chroma_gain                   spatial_chroma_filter_type  video_bitrate_mode
> audio_emphasis               contrast                      spatial_filter              video_encoding
> audio_encoding               hue                           spatial_filter_mode         video_gop_closure
> audio_layer_ii_bitrate       insert_navigation_packets     spatial_luma_filter_type    video_gop_size
> audio_mute                   median_chroma_filter_maximum  stream_type                 video_mute
> audio_sampling_frequency     median_chroma_filter_minimum  stream_vbi_format           video_mute_yuv
> audio_stereo_mode            median_filter_type            temporal_filter             video_peak_bitrate
> audio_stereo_mode_extension  median_luma_filter_maximum    temporal_filter_mode        video_temporal_decimation
> balance                      median_luma_filter_minimum    video_aspect                volume
> brightness                   mute                          video_b_frames
> chroma_agc                   saturation                    video_bitrate
> 
> 
> The question is, is this sufficient?
> 
> One of the few drivers that exposes controls in sysfs is pvrusb2. As far as
> I can tell from the source it will create subdirectories under the device
> node for each control. Those subdirs have the name ctl_<control-name> (e.g.
> ctl_volume), and below that are files exposing all the attributes of that
> control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val
> and bit_val. Most are clear, but some are a bit more obscure. enum_val is
> basically a QUERYMENU and returns all menu options. bit_val seems to be used
> for some non-control values like the TV standard that pvrusb2 also exposes
> and where bit_val is a bit mask of all the valid bits that can be used.
> 
> Mike, if you have any additional information, just let us know. My pvrusb2
> is in another country at the moment so I can't do any testing.

Hans:

What you see in the pvrusb2 driver is the result of an idea I had back 
in 2005.  The pvrusb2 driver has an internal "control" API; my original 
idea back then was to then reimplement other interfaces on top of that 
API, in a manner that is as orthogonal as possible.  The reality today 
is still pretty close to that concept (except for DVB unfortunately 
since that framework's architecture effectively has to take over the RF 
tuner...); the V4L2 implementation in the driver certainly works this 
way.  The sysfs interface you see here is the result of implementing the 
same API through sysfs.  Right now with the pvrusb2 driver the only 
thing not exported through sysfs is the actual streaming of video 
itself.

The entire sysfs implementation in the driver can be found in 
pvrusb2-sysfs.c.  Notice that the file is generic; there is not anything 
in it that is specific to any particular control.  Rather, 
pvrusb2-sysfs.c is able to iterate through the driver's controls, 
picking up the control's name, its type, and accessors.  Based on what 
it finds, this module then synthesizes the interface that you see in 
/class/pvrusb2/* - it's actually possible to add new controls to the 
driver without changing anything in pvrusb2-sysfs.c.


> 
> Personally I think that it is overkill to basically expose the whole
> QUERYCTRL information to sysfs. I see it as an easy and quick way to read and
> modify controls via a command line.

Over time, I have ended up using pretty much every control in that 
interface.  Obviously not every control always gets touched, but I have 
found it extremely valuable to have such direct access to every "knob" 
in the driver this way.

Also, the original concept was that the interface was to be orthogonal; 
in theory any kind of control action in one interface should be just as 
valid in another.


> 
> Mike, do you know of anyone actively using that additional information?

Yes.

The VDR project at one time implemented a plugin to directly interface 
to the pvrusb2 driver in this manner.  I do not know if it is still 
being used since I don't maintain that plugin.

I have over the years seen feedback from many users who just love using 
the sysfs interface - by using sysfs for access to all the knobs & 
switches while just using "cat /dev/video0" (or whatever) to grab the 
video stream, it's possible to nearly completely operate the device 
without writing a single line of C code.  I have read of some people who 
have hacked up shell scripts for special purposes using this driver.  

When I say "nearly completely" above, the big asterisk there is DVB.  
The analog side is completely operable via sysfs.  However because of 
the way DVB works it is currently not possible to export the digital 
side of the driver through anything except DVB itself (a situation that 
I find to be "wrong" but probably will be very difficult to solve 
technically due to DVB's architecture and will likely be impossible 
anyway because the kinds of pvrusb2 driver changes that I think would be 
required probably won't be accepted by others anyway so I haven't been 
very motivated to attack the problem).


> 
> And which non-control values do you at the moment expose in pvrusb2 through
> sysfs? Can you perhaps give an ls -R of all the files you create in sysfs
> for pvrusb2?

*ALL* of them are exposed.  The exact set is synthesized at run-time by 
pvrusb2-sysfs.c via introspection of the control API defined in 
pvrusb2-hdw.h.  In the in-V4L version of the pvrusb2 driver this set of 
controls is likely a constant, but it has certainly changed over time as 
V4L and the pvrusb2 driver have evolved.  In the standalone pvrusb2 
driver, the control set can actually vary due to ifdef's in 
pvrusb2-hdw.c, which are affected by the kernel version and v4l-dvb 
snapshot against which the driver is compiled.


> 
> I am wondering whether some of those should get a place in the 
> framework as well. While I don't think e.g. cropping parameters are 
> useful, things like the current input or tuner frequency can be handy. 
> However, for those to be useful they would have to be wired up 
> internally through the framework. For example, VIDIOC_S_FREQUENCY 
> would have to be hooked up internally to a control. That would ensure 
> that however you access it (ioctl or sysfs) it will both end up in the 
> same s_ctrl function.

I think you're missing a critical point here.

There isn't any logic in the module within the pvrusb2 driver which 
"decides" which controls to expose.  The driver itself has an internal 
API used by everything else, and all that pvrusb2-sysfs.c does is 
enumerate that to expose all of the controls.  It would actually be 
extra work in the pvrusb2 driver to impose a policy on which controls 
are visible.  The cropping parameters are an example of this.  Another 
pvrusb2 driver user wanted to get cropping to work (which unfortunately 
*still* requires v4l-dvb changes that are not implemented).  Part of his 
implementation involved adding a few extra knobs within the driver to 
control the cropping behavior.  Once he did that, the knobs 
automatically became available via sysfs.


> 
> It will be nice to hear from you what your experience is.

Here's my experience:

I originally did the sysfs interface back in 2005 as a proof of concept.  
I wanted to prove that the internal API within the driver was 
functionally complete and relatively easy to use.  I had recently 
finished reading the LDDv3 and learned of the sysfs class interface.  
It occurred to me back then that it would be easy to define a sysfs 
class interface for the pvrusb2 driver in terms of that API.  So I did, 
and it was literally a 2 day hack.  It's worked fabulously well ever 
since.  In terms of popularity, I find that the user community loves it, 
while the developer community seems to tolerate it.

I routinely use the sysfs interface for my own testing.  It's just 
simply trivial to tweak things in the driver at the shell level using 
that interface.  I remember once using all the mpeg knobs to experiment 
with all the ways that filtering could be controlled, for example.  A 
common use-case for me during testing is to use mplayer as a dumb 
streaming app while tweaking the driver via sysfs.

I've had feedback from many users over the years who, upon discovering 
this interface, just latch onto it.  On more than one occassion I've had 
feedback to the effect of "I don't care about any V4L app; I just want 
to stream video and the sysfs interface makes this trivial to control".  
Probably the fact that only just a small handful of V4L apps (xawtv, 
mythtv and recently now mplayer?) even handle mpeg video has amplified 
this behavior: Many users have found it simpler to forget about V4L apps 
and just instead "cat /dev/video0" along with some shell hacking.

And long ago I know the VDR application gained a plugin script which 
itself used the sysfs interface to control the driver.

One aspect of that sysfs interface that I think has had a lot of value 
over time is that it is (within limits) self-describing.  For each 
control you get not only the ability to get (and usually set) its value, 
but you can inspect its limits, learn what the default value is, 
retrieve a description (though being english only that has limited 
utility), and discover its type.  The sysfs interface in the driver 
tries to use symbolic values where possible, including when setting / 
clearing bits in a bit mask.  The fact that such information is there 
helps when writing, say, a generic dialog-based wrapper to provide a TUI 
(Text User Interface) over the interface without having to encode too 
much specific information in the wrapper itself.  Thus if new controls 
get added (or changed) later then such things just automatically adapt.  
Of course this isn't a perfect thing, but it helps.

> 
> Comments? Ideas? Once we commit to something it is there for a long time to
> come since sysfs is after all a public API (although it seems to be more
> changable than ioctls).

I have found it useful over a long period of time.

I also suggest that if such an interface is defined in the general case 
that it should include some introspection capabilities, which will make 
it easier (or even possible) to evolve the interface over time while 
minimizing backwards compatibility issues.

If such a generic interface were made available, I could see an argument 
to remove the sysfs interface from the pvrusb2 driver.  HOWEVER there is 
a community using it, and also unless the generic interface were a 
complete replacement, the pvrusb2 piece will probably have to stay.  
(Note: the sysfs interface in the pvrusb2 driver is already a CONFIG_XX 
parameter so it's easy to deselect it when building the driver.)

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-05 22:12 ` Hans Verkuil
  2010-04-06  6:37   ` Hans Verkuil
@ 2010-04-06 14:41   ` Mike Isely
  2010-04-06 16:19     ` Jonathan Cameron
  1 sibling, 1 reply; 28+ messages in thread
From: Mike Isely @ 2010-04-06 14:41 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media

On Tue, 6 Apr 2010, Hans Verkuil wrote:

   [...]

> 
> One thing that might be useful is to prefix the name with the control class
> name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
> groups them better. Or one could make a controls/user and controls/mpeg
> directory. That might not be such a bad idea actually.

I agree with grouping in concept, and using subdirectories is not a bad 
thing.  Probably however you'd want to ensure that in the end all the 
controls end up logically at the same depth in the tree.


   [...]

> 
> An in between solution would be to add _type files. So you would have 
> 'hue' and 'hue_type'. 'cat hue_type' would give something like:
> 
> int 0 255 1 128 0x0000 Hue
> 
> In other words 'type min max step flags name'.

There was I thought at some point in the past a kernel policy that sysfs 
controls were supposed to limit themselves to one value per node.

> 
> And for menu controls like stream_type (hmm, that would become 
> stream_type_type...) you would get:
> 
> menu 0 5 1 0 Stream Type
> MPEG-2 Program Stream
> 
> MPEG-1 System Stream
> MPEG-2 DVD-compatible Stream
> MPEG-1 VCD-compatible Stream
> MPEG-2 SVCD-compatible Stream
> 
> Note the empty line to denote the unsupported menu item (transport stream).
> 
> This would give the same information with just a single extra file. Still not
> sure whether it is worth it though.

Just remember that the more complex / subtle you make the node contents, 
then the more parsing will be required for any program that tries to use 
it.  I also think it's probably a bad idea for example to define a 
format where the whitespace conveys additional information.  The case 
where I've seen whitespace as part of the syntax actually work cleanly 
is in Python.


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 13:59         ` Devin Heitmueller
@ 2010-04-06 15:05           ` Mike Isely
  0 siblings, 0 replies; 28+ messages in thread
From: Mike Isely @ 2010-04-06 15:05 UTC (permalink / raw)
  To: Devin Heitmueller; +Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-media

On Tue, 6 Apr 2010, Devin Heitmueller wrote:

   [...]

> 
> I tend to agree with Hans.  We've already got *too many* interfaces
> that do the same thing.  The testing matrix is already a nightmare -
> V4L1 versus V4L2, mmap() versus read(), legacy controls versus
> extended controls, and don't get even me started on VBI.
> 
> We should be working to make drivers and interfaces simpler, with
> *fewer* ways of doing the same thing.  The flexibility of providing
> yet another interface via sysfs compared to just calling v4l2-ctl just
> isn't worth the extra testing overhead.  We've already got too much
> stuff that needs to be fixed and not enough good developers to warrant
> making the code more complicated with little tangible benefit.

If another API (e.g. sysfs) is defined and it is specifically NOT 
permitted to be a complete set, then one can ultimately end up with 
situations where in order to effectively use a driver then multiple APIs 
*must* be used by the application.  That's even worse.

This situation already exists in the pvrusb2 driver and it's not because 
of sysfs - it's because of V4L and DVB.  When the pvrusb2 driver is used 
to handle a hybrid device (such as the HVR-1950) one has to use both the 
DVB and V4L APIs in order to effectively operate the device.  This is 
because both APIs provide something not available in the other.  And 
this really sucks if all the user wants to do is "stream mpeg, darn it! 
And I don't care if it is digital or analog".  I think that situation is 
very wrong; given that the HVR-1950 can spit out mpeg in either mode the 
user shouldn't be forced to make his application choice based on which 
mode he wants.  There's only ONE application out there that allows the 
user to operate an HVR-1950 without being forced to deal with this: 
MythTV, and that's because, well, MythTV implements both APIs: V4L and 
DVB.

I really, really dislike situations that arise where multiple APIs are 
*required* to operate a device, when really there should just be one 
API.  That said, if multiple APIs are to be exported by the driver 
interface, then such APIs really should be as complete as possible in 
order to avoid potential problems later where because of previous 
limiting choices of API design now multiple APIs become required.

I agree that testing against multiple APIs can be a pain and a drain on 
effort.  But that has not happened with the pvrusb2 driver.  It should 
be possible to implement the API in a way that minimizes further 
thrashing due to driver changes.  The pvrusb2 sysfs implementation there 
is programmatically created when the driver comes up.  The code which 
implements that interface really doesn't have any logic specific to 
particular API functions; it is just a reflection of what is internally 
in the driver.  If new "knobs" are added to the pvrusb2 driver, then the 
knob automatically appears in the sysfs interface.  If you were to go 
through the change history of the pvrusb2-sysfs.c module, all you're 
really going to find are changes caused by the sysfs class environment 
itself (i.e. when struct class was morphed into struct device), not the 
driver or its functionality.



> 
> And nobody I've talked to who writes applications that work with V4L
> has been screaming "OMG, if only V4L had a sysfs interface to manage
> controls!"

The experience I've seen with users and the pvrusb2 interface is that 
once they discover the sysfs API, the response is in fact very positive.  
Most users of the driver had no concept that such a thing was even 
possible until they were exposed to it.  Now that's not to say that we 
should all be screaming for this - but if people didn't really 
understand what was possible, then how could they ask for it?

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 11:44         ` Markus Rechberger
@ 2010-04-06 15:08           ` Mike Isely
  0 siblings, 0 replies; 28+ messages in thread
From: Mike Isely @ 2010-04-06 15:08 UTC (permalink / raw)
  To: Markus Rechberger; +Cc: Laurent Pinchart, Andy Walls, Hans Verkuil, linux-media

On Tue, 6 Apr 2010, Markus Rechberger wrote:

   [...]

> 
> how about security permissions? while you can easily change the
> permission levels for nodes in /dev you can't do this so easily with
> sysfs entries.
> I don't really think this is needed at all some applications will
> start to use ioctl some other apps might
> go for sysfs.. this makes the API a little bit whacko

This is an excellent point.  I should have brought this up sooner.

The driver has control over the modes of the nodes in sysfs.  The driver 
does NOT have control over the owner / group of those nodes.  It is 
possible to change the owner / group from userspace, and I *think* it's 
possible to create a udev rule to do this, but honestly I have not 
investigated this possibility so I don't fully know.

This is one serious potential drawback to using sysfs as a driver API.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 11:27       ` Laurent Pinchart
  2010-04-06 11:44         ` Markus Rechberger
@ 2010-04-06 15:16         ` Mike Isely
  2010-04-06 22:39           ` Hans Verkuil
  1 sibling, 1 reply; 28+ messages in thread
From: Mike Isely @ 2010-04-06 15:16 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Andy Walls, Hans Verkuil, linux-media

On Tue, 6 Apr 2010, Laurent Pinchart wrote:

> Hi Andy,
> 
> On Tuesday 06 April 2010 13:06:18 Andy Walls wrote:
> > On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:
> 
> [snip]
> 
> > > Again, I still don't know whether we should do this. It is dangerously
> > > seductive because it would be so trivial to implement.
> > 
> > It's like watching ships run aground on a shallow sandbar that all the
> > locals know about.  The waters off of 'Point /sys' are full of usability
> > shipwrecks.  I don't know if it's some siren's song, the lack of a light
> > house, or just strange currents that deceive even seasoned
> > navigators....
> > 
> > Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control
> > metatdata.  It's not as easy as typing 'cat', but the user base using
> > sysfs in an interactive shell or shell script should also know how to
> > use v4l2-ctl.  In embedded systems, the final system deployment should
> > not need the control metadata available from sysfs in a command shell
> > anyway.
> 
> I fully agree with this. If we push the idea one step further, why do we need 
> to expose controls in sysfs at all ?

I have found it useful to have the sysfs interface within the pvrusb2 
driver.

If it is going to take a lot of work to specifically craft a sysfs 
interface that exports the V4L API, then it will probably be a pain to 
maintain going forward.  By "a lot of work" I mean that each V4L API 
function would have to be explicitly coded for in this interface, thus 
as the V4L API evolves over time then extra work must be expended each 
time to keep the sysfs interface in step.  If that is to be the case 
then it may not be worth it.

In the pvrusb2 driver this has not been the case because the code I 
wrote which implements the sysfs interface for the driver does this 
programmatically.  That is, there is nothing in the pvrusb2-sysfs.c 
module which is specific to a particular function.  Instead, when the 
module initializes it is able to enumerate the API on its own and 
generate the appropriate interface for each control it finds.  Thus as 
the pvrusb2 driver's implementation has evolved over time, the sysfs 
implementation has simply continues to do its job, automatically 
reflecting internal changes without any extra work in that module's 
code.  I don't know if that same strategy could be done in the V4L core.  
If it could, then this would probably alleviate a lot of concerns about 
testing / maintenance going forward.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 14:32         ` Mauro Carvalho Chehab
@ 2010-04-06 16:06           ` Jonathan Cameron
  2010-04-06 16:36             ` Bjørn Forsman
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2010-04-06 16:06 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Hans Verkuil, linux-media, Mike Isely

On 04/06/10 15:32, Mauro Carvalho Chehab wrote:
> Hans Verkuil wrote:
>>> Hans Verkuil wrote:
>>>> $ ls /sys/class/video4linux/video1/controls
>>>> balance                           mpeg_insert_navigation_packets
>>>> mpeg_video_aspect
>>>> brightness                        mpeg_median_chroma_filter_maximum
>>>> mpeg_video_b_frames
>>>> chroma_agc                        mpeg_median_chroma_filter_minimum
>>>> mpeg_video_bitrate
>>>> chroma_gain                       mpeg_median_filter_type
>>>> mpeg_video_bitrate_mode
>>>> contrast                          mpeg_median_luma_filter_maximum
>>>> mpeg_video_encoding
>>>> hue                               mpeg_median_luma_filter_minimum
>>>> mpeg_video_gop_closure
>>>> mpeg_audio_crc                    mpeg_spatial_chroma_filter_type
>>>> mpeg_video_gop_size
>>>> mpeg_audio_emphasis               mpeg_spatial_filter
>>>> mpeg_video_mute
>>>> mpeg_audio_encoding               mpeg_spatial_filter_mode
>>>> mpeg_video_mute_yuv
>>>> mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type
>>>> mpeg_video_peak_bitrate
>>>> mpeg_audio_mute                   mpeg_stream_type
>>>> mpeg_video_temporal_decimation
>>>> mpeg_audio_sampling_frequency     mpeg_stream_vbi_format
>>>> mute
>>>> mpeg_audio_stereo_mode            mpeg_temporal_filter
>>>> saturation
>>>> mpeg_audio_stereo_mode_extension  mpeg_temporal_filter_mode
>>>> volume
>>>
>>> It would be more intuitive if you group the classes with a few subdirs:
>>>
>>> /video/balance
>>> /video/brightness
>>> ...
>>> /mpeg_audio/crc
>>> /mpeg_audio/mute
>>> ...
>>> /audio/volume
>>> /audio/bass
>>> /audio/treble
>>
>> 1) We don't have that information.
>> 2) It would make a simple scheme suddenly a lot more complicated (see
>> Andy's comments)
>> 3) The main interface is always the application's GUI through ioctls, not
>> sysfs.
>> 4) Remember that ivtv has an unusually large number of controls. Most
>> drivers will just have the usual audio and video controls, perhaps 10 at
>> most.
> 
> Ok.
> 
>> I think we should just ditch this for the first implementation of the
>> control framework. It can always be added later, but once added it is
>> *much* harder to remove again. It's a nice proof-of-concept, though :-)
> 
> I like the concept, especially if we can get rid of other similar sysfs interfaces
> that got added on a few drivers (pvrusb2 and some non-gspca drivers have
> it, for sure). I think I saw some of the gspca patches also touching on sysfs.
> Having this unified into a common interface is a bonus.

Obviously it adds to the review burden, but perhaps we can state that the sysfs
interface is only an additional option (and personally I think I'd find it pretty
helpful for debugging if nothing else) and that all functionality there MUST be
available through the normal routes?  If any functionality only supported via
sysfs is seen as a bug, then we can point it out in reviews etc.

I agree with Mauro that it would be really handy to unify any interfaces that are
going to turn up there anyway.

Generally I'm personally in favor with the convenience of sysfs interfaces for quick
and dirty debugging purposes but perhaps this isn't the time to do it here.

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 14:41   ` Mike Isely
@ 2010-04-06 16:19     ` Jonathan Cameron
  2010-04-06 22:47       ` Hans Verkuil
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Cameron @ 2010-04-06 16:19 UTC (permalink / raw)
  To: Mike Isely; +Cc: Hans Verkuil, linux-media

On 04/06/10 15:41, Mike Isely wrote:
> On Tue, 6 Apr 2010, Hans Verkuil wrote:
> 
>    [...]
> 
>>
>> One thing that might be useful is to prefix the name with the control class
>> name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
>> groups them better. Or one could make a controls/user and controls/mpeg
>> directory. That might not be such a bad idea actually.
> 
> I agree with grouping in concept, and using subdirectories is not a bad 
> thing.  Probably however you'd want to ensure that in the end all the 
> controls end up logically at the same depth in the tree.
> 
> 
>    [...]
> 
>>
>> An in between solution would be to add _type files. So you would have 
>> 'hue' and 'hue_type'. 'cat hue_type' would give something like:
>>
>> int 0 255 1 128 0x0000 Hue
>>
>> In other words 'type min max step flags name'.
> 
> There was I thought at some point in the past a kernel policy that sysfs 
> controls were supposed to limit themselves to one value per node.
It's usually considered to be one 'conceptual' value per node, though
this falls fowl of that rule too.  So you could have one file with a list
of possible values, or even one for say hue_range 0...255 but people are
going to through a wobbly about antyhing with as much data in it as above.

The debate on this was actually pretty well covered in an lwn article the
other week. http://lwn.net/Articles/378884/

So the above hue type would probably need:

hue_type (int)
hue_range (0...255)
hue_step (1)
hue_flags (128)
hue_name (Hue)

Of those, hue_name doesn't in this case tell us anything and hue_step could
be suppressed as an obvious default.  It could be argued that parts of the
above could be considered a single 'conceptual' value but I don't think the
whole can be.  The reasoning behind this  (and it is definitely true with
your above example) is that sysfs should be human readable without needing
to reach for the documentation.
> 
>>
>> And for menu controls like stream_type (hmm, that would become 
>> stream_type_type...) you would get:
>>
>> menu 0 5 1 0 Stream Type

>> MPEG-2 Program Stream
>>
>> MPEG-1 System Stream
>> MPEG-2 DVD-compatible Stream
>> MPEG-1 VCD-compatible Stream
>> MPEG-2 SVCD-compatible Stream
>>
>> Note the empty line to denote the unsupported menu item (transport stream).
>>
>> This would give the same information with just a single extra file. Still not
>> sure whether it is worth it though.
> 
> Just remember that the more complex / subtle you make the node contents, 
> then the more parsing will be required for any program that tries to use 
> it.  I also think it's probably a bad idea for example to define a 
> format where the whitespace conveys additional information.  The case 
> where I've seen whitespace as part of the syntax actually work cleanly 
> is in Python.
> 
> 


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

* Re: RFC: exposing controls in sysfs
  2010-04-06 16:06           ` Jonathan Cameron
@ 2010-04-06 16:36             ` Bjørn Forsman
  0 siblings, 0 replies; 28+ messages in thread
From: Bjørn Forsman @ 2010-04-06 16:36 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, Mike Isely

On 6 April 2010 18:06, Jonathan Cameron <jic23@cam.ac.uk> wrote:
> On 04/06/10 15:32, Mauro Carvalho Chehab wrote:
>> Hans Verkuil wrote:
>>>> Hans Verkuil wrote:
>>>>> $ ls /sys/class/video4linux/video1/controls
>>>>> balance                           mpeg_insert_navigation_packets
>>>>> mpeg_video_aspect
>>>>> brightness                        mpeg_median_chroma_filter_maximum
>>>>> mpeg_video_b_frames
>>>>> chroma_agc                        mpeg_median_chroma_filter_minimum
>>>>> mpeg_video_bitrate
>>>>> chroma_gain                       mpeg_median_filter_type
>>>>> mpeg_video_bitrate_mode
>>>>> contrast                          mpeg_median_luma_filter_maximum
>>>>> mpeg_video_encoding
>>>>> hue                               mpeg_median_luma_filter_minimum
>>>>> mpeg_video_gop_closure
>>>>> mpeg_audio_crc                    mpeg_spatial_chroma_filter_type
>>>>> mpeg_video_gop_size
>>>>> mpeg_audio_emphasis               mpeg_spatial_filter
>>>>> mpeg_video_mute
>>>>> mpeg_audio_encoding               mpeg_spatial_filter_mode
>>>>> mpeg_video_mute_yuv
>>>>> mpeg_audio_layer_ii_bitrate       mpeg_spatial_luma_filter_type
>>>>> mpeg_video_peak_bitrate
>>>>> mpeg_audio_mute                   mpeg_stream_type
>>>>> mpeg_video_temporal_decimation
>>>>> mpeg_audio_sampling_frequency     mpeg_stream_vbi_format
>>>>> mute
>>>>> mpeg_audio_stereo_mode            mpeg_temporal_filter
>>>>> saturation
>>>>> mpeg_audio_stereo_mode_extension  mpeg_temporal_filter_mode
>>>>> volume
>>>>
>>>> It would be more intuitive if you group the classes with a few subdirs:
>>>>
>>>> /video/balance
>>>> /video/brightness
>>>> ...
>>>> /mpeg_audio/crc
>>>> /mpeg_audio/mute
>>>> ...
>>>> /audio/volume
>>>> /audio/bass
>>>> /audio/treble
>>>
>>> 1) We don't have that information.
>>> 2) It would make a simple scheme suddenly a lot more complicated (see
>>> Andy's comments)
>>> 3) The main interface is always the application's GUI through ioctls, not
>>> sysfs.
>>> 4) Remember that ivtv has an unusually large number of controls. Most
>>> drivers will just have the usual audio and video controls, perhaps 10 at
>>> most.
>>
>> Ok.
>>
>>> I think we should just ditch this for the first implementation of the
>>> control framework. It can always be added later, but once added it is
>>> *much* harder to remove again. It's a nice proof-of-concept, though :-)
>>
>> I like the concept, especially if we can get rid of other similar sysfs interfaces
>> that got added on a few drivers (pvrusb2 and some non-gspca drivers have
>> it, for sure). I think I saw some of the gspca patches also touching on sysfs.
>> Having this unified into a common interface is a bonus.
>
> Obviously it adds to the review burden, but perhaps we can state that the sysfs
> interface is only an additional option (and personally I think I'd find it pretty
> helpful for debugging if nothing else) and that all functionality there MUST be
> available through the normal routes?  If any functionality only supported via
> sysfs is seen as a bug, then we can point it out in reviews etc.
>
> I agree with Mauro that it would be really handy to unify any interfaces that are
> going to turn up there anyway.
>
> Generally I'm personally in favor with the convenience of sysfs interfaces for quick
> and dirty debugging purposes but perhaps this isn't the time to do it here.

Hi all,

I'm a newbie but I have to ask: how about using debugfs instead of
sysfs? Then everyone will know that the interface is for debugging
only and not production code :-)

Best regards,
Bjørn Forsman

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 15:16         ` Mike Isely
@ 2010-04-06 22:39           ` Hans Verkuil
  2010-04-07  2:10             ` hermann pitton
  2010-04-07  6:56             ` Hans Verkuil
  0 siblings, 2 replies; 28+ messages in thread
From: Hans Verkuil @ 2010-04-06 22:39 UTC (permalink / raw)
  To: Mike Isely; +Cc: Laurent Pinchart, Andy Walls, linux-media

On Tuesday 06 April 2010 17:16:17 Mike Isely wrote:
> On Tue, 6 Apr 2010, Laurent Pinchart wrote:
> 
> > Hi Andy,
> > 
> > On Tuesday 06 April 2010 13:06:18 Andy Walls wrote:
> > > On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:
> > 
> > [snip]
> > 
> > > > Again, I still don't know whether we should do this. It is dangerously
> > > > seductive because it would be so trivial to implement.
> > > 
> > > It's like watching ships run aground on a shallow sandbar that all the
> > > locals know about.  The waters off of 'Point /sys' are full of usability
> > > shipwrecks.  I don't know if it's some siren's song, the lack of a light
> > > house, or just strange currents that deceive even seasoned
> > > navigators....
> > > 
> > > Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control
> > > metatdata.  It's not as easy as typing 'cat', but the user base using
> > > sysfs in an interactive shell or shell script should also know how to
> > > use v4l2-ctl.  In embedded systems, the final system deployment should
> > > not need the control metadata available from sysfs in a command shell
> > > anyway.
> > 
> > I fully agree with this. If we push the idea one step further, why do we need 
> > to expose controls in sysfs at all ?
> 
> I have found it useful to have the sysfs interface within the pvrusb2 
> driver.
> 
> If it is going to take a lot of work to specifically craft a sysfs 
> interface that exports the V4L API, then it will probably be a pain to 
> maintain going forward.  By "a lot of work" I mean that each V4L API 
> function would have to be explicitly coded for in this interface, thus 
> as the V4L API evolves over time then extra work must be expended each 
> time to keep the sysfs interface in step.  If that is to be the case 
> then it may not be worth it.

No, that is no work at all. For all practical purposes there are two objects
(OK, really three, but the node object is just internal). The first object
is the control handler, the second is the control object. Handlers have
controls. Handlers can also inherit controls from other handlers. If they
already had the same control, then they effectively override the inherited
control. Controls can also be private to a handler, i.e. they will never be
inherited by other handlers.

Sound familiar? It's your basic C++ class inheritance scheme with public
and private fields (or controls in this case).

The sysfs implementation is just bolted on top of this: each video_device
struct is associated with a control handler and sysfs will expose all controls
that that handler references.

You can take a look at the header of the control framework:

http://linuxtv.org/hg/~hverkuil/v4l-dvb-fw/raw-file/bf7cd2fb7a35/linux/include/media/v4l2-ctrls.h

<brainstorm mode on>

It would be trivial to add e.g. a V4L2 control class that could be used
to expose all sorts of V4L2 functionality to sysfs. It would be handled
differently in that you don't want to expose those through VIDIOC_QUERYCTRL
and friends, just to sysfs. Heck, it could be implemented almost completely
transparently from drivers. For example, an 'echo 1 >/sys/..../v4l2_input'
could be converted automatically to a VIDIOC_S_INPUT command that's issued
to the driver. Similar to what you did in pvrusb2, except you went the other
way around: ioctls are converted to controls. That is not feasible, though,
since you do not want to completely redo all drivers.

There are definitely some snags, but the basic premise is sound.

Of course, just the fact that you can easily do something does not mean that
you should. The first version of the framework will not contain any sysfs. It
is clear that the last word has not been said on this. It's not a big deal,
sysfs was just an add-on and not part of the core.

But having it in the kernel will make it a nice foundation on which to experiment.

Just a thought experiment: take VIDIOC_S_FREQUENCY. The struct has three
fields: tuner, type, frequency. So that's a cluster of three controls. So you
would need a 's_ctrl' function like this:

	switch (id) {
	/* handle the frequency cluster */
	case V4L2_CID_V4L_FREQ_TUNER:
		struct v4l2_frequency f;
		f.tuner = freq_tuner->cur.val;
		f.type = freq_type->cur.val;
		f.frequency = freq_freq->cur.val;
		return vdev->ioctl_ops->s_frequency(&f);
	}

Pseudo-code, of course, and there are some little things like the 'file' and 'fh'
args to s_frequency, but you could use the framework to make a very clean
implementation of this. Especially since the framework supports 'clustering' of
controls. Effectively creating a single composite control from the point of view
of the driver. Hmm, sounds awfully like a struct, doesn't it? :-)

> In the pvrusb2 driver this has not been the case because the code I 
> wrote which implements the sysfs interface for the driver does this 
> programmatically.  That is, there is nothing in the pvrusb2-sysfs.c 
> module which is specific to a particular function.  Instead, when the 
> module initializes it is able to enumerate the API on its own and 
> generate the appropriate interface for each control it finds.  Thus as 
> the pvrusb2 driver's implementation has evolved over time, the sysfs 
> implementation has simply continues to do its job, automatically 
> reflecting internal changes without any extra work in that module's 
> code.  I don't know if that same strategy could be done in the V4L core.  
> If it could, then this would probably alleviate a lot of concerns about 
> testing / maintenance going forward.

In other words, yes, it could do this. And with relatively little work and
completely transparent to all drivers.

But I have a big question mark whether we really want to go that way. The
good thing about it is that the ioctls remain the primary API, as they should
be. Anything like this will definitely be a phase 3 (or 4, or...), but it
is at least nice to realize how easy it would be. That's a good sign of the
quality of the code.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 16:19     ` Jonathan Cameron
@ 2010-04-06 22:47       ` Hans Verkuil
  0 siblings, 0 replies; 28+ messages in thread
From: Hans Verkuil @ 2010-04-06 22:47 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Mike Isely, linux-media

On Tuesday 06 April 2010 18:19:30 Jonathan Cameron wrote:
> On 04/06/10 15:41, Mike Isely wrote:
> > On Tue, 6 Apr 2010, Hans Verkuil wrote:
> > 
> >    [...]
> > 
> >>
> >> One thing that might be useful is to prefix the name with the control class
> >> name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would
> >> groups them better. Or one could make a controls/user and controls/mpeg
> >> directory. That might not be such a bad idea actually.
> > 
> > I agree with grouping in concept, and using subdirectories is not a bad 
> > thing.  Probably however you'd want to ensure that in the end all the 
> > controls end up logically at the same depth in the tree.
> > 
> > 
> >    [...]
> > 
> >>
> >> An in between solution would be to add _type files. So you would have 
> >> 'hue' and 'hue_type'. 'cat hue_type' would give something like:
> >>
> >> int 0 255 1 128 0x0000 Hue
> >>
> >> In other words 'type min max step flags name'.
> > 
> > There was I thought at some point in the past a kernel policy that sysfs 
> > controls were supposed to limit themselves to one value per node.
> It's usually considered to be one 'conceptual' value per node, though
> this falls fowl of that rule too.  So you could have one file with a list
> of possible values, or even one for say hue_range 0...255 but people are
> going to through a wobbly about antyhing with as much data in it as above.
> 
> The debate on this was actually pretty well covered in an lwn article the
> other week. http://lwn.net/Articles/378884/
> 
> So the above hue type would probably need:
> 
> hue_type (int)
> hue_range (0...255)
> hue_step (1)
> hue_flags (128)
> hue_name (Hue)
> 
> Of those, hue_name doesn't in this case tell us anything and hue_step could
> be suppressed as an obvious default.  It could be argued that parts of the
> above could be considered a single 'conceptual' value but I don't think the
> whole can be.  The reasoning behind this  (and it is definitely true with
> your above example) is that sysfs should be human readable without needing
> to reach for the documentation.
> > 
> >>
> >> And for menu controls like stream_type (hmm, that would become 
> >> stream_type_type...) you would get:
> >>
> >> menu 0 5 1 0 Stream Type
> 
> >> MPEG-2 Program Stream
> >>
> >> MPEG-1 System Stream
> >> MPEG-2 DVD-compatible Stream
> >> MPEG-1 VCD-compatible Stream
> >> MPEG-2 SVCD-compatible Stream
> >>
> >> Note the empty line to denote the unsupported menu item (transport stream).
> >>
> >> This would give the same information with just a single extra file. Still not
> >> sure whether it is worth it though.
> > 
> > Just remember that the more complex / subtle you make the node contents, 
> > then the more parsing will be required for any program that tries to use 
> > it.  I also think it's probably a bad idea for example to define a 
> > format where the whitespace conveys additional information.  The case 
> > where I've seen whitespace as part of the syntax actually work cleanly 
> > is in Python.

You are correct, it should be one value per item. It would become a really
big mess, though :-(

I don't see much of an advantage to doing this in sysfs. If you need to know
the type, then use v4l2-ctl. We could make a simple option for v4l2-ctl, or
write a new tool that does this in a format that's easy to handle by scripting
languages. Creating a zillion sysfs files strikes me as major overkill (not to
mention the additional resources it would claim).

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 22:39           ` Hans Verkuil
@ 2010-04-07  2:10             ` hermann pitton
  2010-04-07  6:56             ` Hans Verkuil
  1 sibling, 0 replies; 28+ messages in thread
From: hermann pitton @ 2010-04-07  2:10 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Mike Isely, Laurent Pinchart, Andy Walls, linux-media

Hi,

Am Mittwoch, den 07.04.2010, 00:39 +0200 schrieb Hans Verkuil:
> On Tuesday 06 April 2010 17:16:17 Mike Isely wrote:
> > On Tue, 6 Apr 2010, Laurent Pinchart wrote:
> > 
> > > Hi Andy,
> > > 
> > > On Tuesday 06 April 2010 13:06:18 Andy Walls wrote:
> > > > On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:
> > > 
> > > [snip]
[snip]
> > code.  I don't know if that same strategy could be done in the V4L core.  
> > If it could, then this would probably alleviate a lot of concerns about 
> > testing / maintenance going forward.
> 
> In other words, yes, it could do this. And with relatively little work and
> completely transparent to all drivers.
> 
> But I have a big question mark whether we really want to go that way. The
> good thing about it is that the ioctls remain the primary API, as they should
> be. Anything like this will definitely be a phase 3 (or 4, or...), but it
> is at least nice to realize how easy it would be. That's a good sign of the
> quality of the code.
> 
> Regards,
> 
> 	Hans

that all looks really good now and quite promising, also IR/remote.

Maybe too good?

About what might be upcoming problems nobody is talking about currently.

Never seen before ...

I would wonder a lot, if we really should have made it close to such
with a lot of buffering all around ;)

Well, I guess the time on that beach might be short.

Manu gave some pointers previously that HD+ is a serious issue for us,
also it seems, that some lower level of firmware starts to hide GPIO's
and i2c addresses on recent boards.

We likely should stay hackish and able for quick responses and not
over-engineer convenience we do not really have.

Just my two cents.

Cheers,
Hermann





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

* Re: RFC: exposing controls in sysfs
  2010-04-06 22:39           ` Hans Verkuil
  2010-04-07  2:10             ` hermann pitton
@ 2010-04-07  6:56             ` Hans Verkuil
  2010-04-08  0:52               ` Mike Isely
  1 sibling, 1 reply; 28+ messages in thread
From: Hans Verkuil @ 2010-04-07  6:56 UTC (permalink / raw)
  To: Mike Isely; +Cc: Laurent Pinchart, Andy Walls, linux-media

On Wednesday 07 April 2010 00:39:20 Hans Verkuil wrote:
> On Tuesday 06 April 2010 17:16:17 Mike Isely wrote:
> > On Tue, 6 Apr 2010, Laurent Pinchart wrote:
> > 
> > > Hi Andy,
> > > 
> > > On Tuesday 06 April 2010 13:06:18 Andy Walls wrote:
> > > > On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote:
> > > 
> > > [snip]
> > > 
> > > > > Again, I still don't know whether we should do this. It is dangerously
> > > > > seductive because it would be so trivial to implement.
> > > > 
> > > > It's like watching ships run aground on a shallow sandbar that all the
> > > > locals know about.  The waters off of 'Point /sys' are full of usability
> > > > shipwrecks.  I don't know if it's some siren's song, the lack of a light
> > > > house, or just strange currents that deceive even seasoned
> > > > navigators....
> > > > 
> > > > Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control
> > > > metatdata.  It's not as easy as typing 'cat', but the user base using
> > > > sysfs in an interactive shell or shell script should also know how to
> > > > use v4l2-ctl.  In embedded systems, the final system deployment should
> > > > not need the control metadata available from sysfs in a command shell
> > > > anyway.
> > > 
> > > I fully agree with this. If we push the idea one step further, why do we need 
> > > to expose controls in sysfs at all ?
> > 
> > I have found it useful to have the sysfs interface within the pvrusb2 
> > driver.
> > 
> > If it is going to take a lot of work to specifically craft a sysfs 
> > interface that exports the V4L API, then it will probably be a pain to 
> > maintain going forward.  By "a lot of work" I mean that each V4L API 
> > function would have to be explicitly coded for in this interface, thus 
> > as the V4L API evolves over time then extra work must be expended each 
> > time to keep the sysfs interface in step.  If that is to be the case 
> > then it may not be worth it.
> 
> No, that is no work at all. For all practical purposes there are two objects
> (OK, really three, but the node object is just internal). The first object
> is the control handler, the second is the control object. Handlers have
> controls. Handlers can also inherit controls from other handlers. If they
> already had the same control, then they effectively override the inherited
> control. Controls can also be private to a handler, i.e. they will never be
> inherited by other handlers.
> 
> Sound familiar? It's your basic C++ class inheritance scheme with public
> and private fields (or controls in this case).
> 
> The sysfs implementation is just bolted on top of this: each video_device
> struct is associated with a control handler and sysfs will expose all controls
> that that handler references.
> 
> You can take a look at the header of the control framework:
> 
> http://linuxtv.org/hg/~hverkuil/v4l-dvb-fw/raw-file/bf7cd2fb7a35/linux/include/media/v4l2-ctrls.h
> 
> <brainstorm mode on>
> 
> It would be trivial to add e.g. a V4L2 control class that could be used
> to expose all sorts of V4L2 functionality to sysfs. It would be handled
> differently in that you don't want to expose those through VIDIOC_QUERYCTRL
> and friends, just to sysfs. Heck, it could be implemented almost completely
> transparently from drivers. For example, an 'echo 1 >/sys/..../v4l2_input'
> could be converted automatically to a VIDIOC_S_INPUT command that's issued
> to the driver. Similar to what you did in pvrusb2, except you went the other
> way around: ioctls are converted to controls. That is not feasible, though,
> since you do not want to completely redo all drivers.
> 
> There are definitely some snags, but the basic premise is sound.
> 
> Of course, just the fact that you can easily do something does not mean that
> you should. The first version of the framework will not contain any sysfs. It
> is clear that the last word has not been said on this. It's not a big deal,
> sysfs was just an add-on and not part of the core.
> 
> But having it in the kernel will make it a nice foundation on which to experiment.
> 
> Just a thought experiment: take VIDIOC_S_FREQUENCY. The struct has three
> fields: tuner, type, frequency. So that's a cluster of three controls. So you
> would need a 's_ctrl' function like this:
> 
> 	switch (id) {
> 	/* handle the frequency cluster */
> 	case V4L2_CID_V4L_FREQ_TUNER:
> 		struct v4l2_frequency f;
> 		f.tuner = freq_tuner->cur.val;
> 		f.type = freq_type->cur.val;
> 		f.frequency = freq_freq->cur.val;
> 		return vdev->ioctl_ops->s_frequency(&f);
> 	}
> 
> Pseudo-code, of course, and there are some little things like the 'file' and 'fh'
> args to s_frequency, but you could use the framework to make a very clean
> implementation of this. Especially since the framework supports 'clustering' of
> controls. Effectively creating a single composite control from the point of view
> of the driver. Hmm, sounds awfully like a struct, doesn't it? :-)
> 
> > In the pvrusb2 driver this has not been the case because the code I 
> > wrote which implements the sysfs interface for the driver does this 
> > programmatically.  That is, there is nothing in the pvrusb2-sysfs.c 
> > module which is specific to a particular function.  Instead, when the 
> > module initializes it is able to enumerate the API on its own and 
> > generate the appropriate interface for each control it finds.  Thus as 
> > the pvrusb2 driver's implementation has evolved over time, the sysfs 
> > implementation has simply continues to do its job, automatically 
> > reflecting internal changes without any extra work in that module's 
> > code.  I don't know if that same strategy could be done in the V4L core.  
> > If it could, then this would probably alleviate a lot of concerns about 
> > testing / maintenance going forward.
> 
> In other words, yes, it could do this. And with relatively little work and
> completely transparent to all drivers.
> 
> But I have a big question mark whether we really want to go that way. The
> good thing about it is that the ioctls remain the primary API, as they should
> be. Anything like this will definitely be a phase 3 (or 4, or...), but it
> is at least nice to realize how easy it would be. That's a good sign of the
> quality of the code.

Perhaps we should just not do this in sysfs at all but in debugfs? We have a
lot more freedom there. No requirement of one-value-per-file, and if we need
to we can change things in the future. It would actually be easier to issue
ioctl commands to a driver from debugfs since we have a proper struct file
there.

It could be implemented as a separate module that can be loaded if debugfs is
enabled and suddenly you have all this extra debug functionality.

I admit, I would really enjoy writing something like this. I just don't want
to do this in sysfs as that makes it too 'official' so to speak. In other words,
mainline applications should not use sysfs, but home-grown scripts are free to
use it as far as I am concerned.

How much of a problem would that be for you, Mike? On the one hand users have
to mount debugfs, but on the other hand it will be consistent for all drivers
that use the control framework. And you should be able to ditch a substantial
amount of code :-)

A typical debugfs structure would be (for example):

/dbg/video4linux/video0/controls
/dbg/video4linux/video0/types
/dbg/video4linux/video0/v4l2

with controls still showing all controls, types contains type information for
all objects (both controls and v4l2) and v4l2 would contain the v4l2 'front-end
controls'.

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

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

* Re: RFC: exposing controls in sysfs
  2010-04-06 14:33 ` Mike Isely
@ 2010-04-07 18:50   ` Lars Hanisch
  2010-04-07 22:15     ` hermann pitton
  0 siblings, 1 reply; 28+ messages in thread
From: Lars Hanisch @ 2010-04-07 18:50 UTC (permalink / raw)
  To: Mike Isely; +Cc: Hans Verkuil, linux-media

Am 06.04.2010 16:33, schrieb Mike Isely:
>
> Comments below...
>
> On Mon, 5 Apr 2010, Hans Verkuil wrote:
>
>> Hi all,
>>
>> The new control framework makes it very easy to expose controls in sysfs.
>> The way it is implemented now in the framework is that each device node
>> will get a 'controls' subdirectory in sysfs. Below which are all the controls
>> associated with that device node.
>>
>> So different device nodes can have different controls if so desired.
>>
>> The name of each sysfs file is derived from the control name, basically making
>> it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non-
>> alphanumerical characters. Seems to work well.
>>
>> For numerical controls you can write numbers in decimal, octal or hexadecimal.
>>

>> When you write to a button control it will ignore what you wrote, but still
>> execute the action.
>>
>> It looks like this for ivtv:
>>
>> $ ls /sys/class/video4linux/video1
>> controls  dev  device  index  name  power  subsystem  uevent
>>
>> $ ls /sys/class/video4linux/video1/controls
>> audio_crc                    chroma_gain                   spatial_chroma_filter_type  video_bitrate_mode
>> audio_emphasis               contrast                      spatial_filter              video_encoding
>> audio_encoding               hue                           spatial_filter_mode         video_gop_closure
>> audio_layer_ii_bitrate       insert_navigation_packets     spatial_luma_filter_type    video_gop_size
>> audio_mute                   median_chroma_filter_maximum  stream_type                 video_mute
>> audio_sampling_frequency     median_chroma_filter_minimum  stream_vbi_format           video_mute_yuv
>> audio_stereo_mode            median_filter_type            temporal_filter             video_peak_bitrate
>> audio_stereo_mode_extension  median_luma_filter_maximum    temporal_filter_mode        video_temporal_decimation
>> balance                      median_luma_filter_minimum    video_aspect                volume
>> brightness                   mute                          video_b_frames
>> chroma_agc                   saturation                    video_bitrate
>>
>>
>> The question is, is this sufficient?
>>
>> One of the few drivers that exposes controls in sysfs is pvrusb2. As far as
>> I can tell from the source it will create subdirectories under the device
>> node for each control. Those subdirs have the name ctl_<control-name>  (e.g.
>> ctl_volume), and below that are files exposing all the attributes of that
>> control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val
>> and bit_val. Most are clear, but some are a bit more obscure. enum_val is
>> basically a QUERYMENU and returns all menu options. bit_val seems to be used
>> for some non-control values like the TV standard that pvrusb2 also exposes
>> and where bit_val is a bit mask of all the valid bits that can be used.
>>
>> Mike, if you have any additional information, just let us know. My pvrusb2
>> is in another country at the moment so I can't do any testing.
>
> Hans:
>
> What you see in the pvrusb2 driver is the result of an idea I had back
> in 2005.  The pvrusb2 driver has an internal "control" API; my original
> idea back then was to then reimplement other interfaces on top of that
> API, in a manner that is as orthogonal as possible.  The reality today
> is still pretty close to that concept (except for DVB unfortunately
> since that framework's architecture effectively has to take over the RF
> tuner...); the V4L2 implementation in the driver certainly works this
> way.  The sysfs interface you see here is the result of implementing the
> same API through sysfs.  Right now with the pvrusb2 driver the only
> thing not exported through sysfs is the actual streaming of video
> itself.
>
> The entire sysfs implementation in the driver can be found in
> pvrusb2-sysfs.c.  Notice that the file is generic; there is not anything
> in it that is specific to any particular control.  Rather,
> pvrusb2-sysfs.c is able to iterate through the driver's controls,
> picking up the control's name, its type, and accessors.  Based on what
> it finds, this module then synthesizes the interface that you see in
> /class/pvrusb2/* - it's actually possible to add new controls to the
> driver without changing anything in pvrusb2-sysfs.c.
>
>
>>
>> Personally I think that it is overkill to basically expose the whole
>> QUERYCTRL information to sysfs. I see it as an easy and quick way to read and
>> modify controls via a command line.
>
> Over time, I have ended up using pretty much every control in that
> interface.  Obviously not every control always gets touched, but I have
> found it extremely valuable to have such direct access to every "knob"
> in the driver this way.
>
> Also, the original concept was that the interface was to be orthogonal;
> in theory any kind of control action in one interface should be just as
> valid in another.
>
>
>>
>> Mike, do you know of anyone actively using that additional information?
>
> Yes.
>
> The VDR project at one time implemented a plugin to directly interface
> to the pvrusb2 driver in this manner.  I do not know if it is still
> being used since I don't maintain that plugin.

  Just FYI:
  The PVR USB2 device is now handled by the pvrinput-plugin, which uses only ioctls. The "old" pvrusb2-plugin is obsolete.

  http://projects.vdr-developer.org/projects/show/plg-pvrinput

Regards,
Lars.

>
> I have over the years seen feedback from many users who just love using
> the sysfs interface - by using sysfs for access to all the knobs&
> switches while just using "cat /dev/video0" (or whatever) to grab the
> video stream, it's possible to nearly completely operate the device
> without writing a single line of C code.  I have read of some people who
> have hacked up shell scripts for special purposes using this driver.
>
> When I say "nearly completely" above, the big asterisk there is DVB.
> The analog side is completely operable via sysfs.  However because of
> the way DVB works it is currently not possible to export the digital
> side of the driver through anything except DVB itself (a situation that
> I find to be "wrong" but probably will be very difficult to solve
> technically due to DVB's architecture and will likely be impossible
> anyway because the kinds of pvrusb2 driver changes that I think would be
> required probably won't be accepted by others anyway so I haven't been
> very motivated to attack the problem).
>
>
>>
>> And which non-control values do you at the moment expose in pvrusb2 through
>> sysfs? Can you perhaps give an ls -R of all the files you create in sysfs
>> for pvrusb2?
>
> *ALL* of them are exposed.  The exact set is synthesized at run-time by
> pvrusb2-sysfs.c via introspection of the control API defined in
> pvrusb2-hdw.h.  In the in-V4L version of the pvrusb2 driver this set of
> controls is likely a constant, but it has certainly changed over time as
> V4L and the pvrusb2 driver have evolved.  In the standalone pvrusb2
> driver, the control set can actually vary due to ifdef's in
> pvrusb2-hdw.c, which are affected by the kernel version and v4l-dvb
> snapshot against which the driver is compiled.
>
>
>>
>> I am wondering whether some of those should get a place in the
>> framework as well. While I don't think e.g. cropping parameters are
>> useful, things like the current input or tuner frequency can be handy.
>> However, for those to be useful they would have to be wired up
>> internally through the framework. For example, VIDIOC_S_FREQUENCY
>> would have to be hooked up internally to a control. That would ensure
>> that however you access it (ioctl or sysfs) it will both end up in the
>> same s_ctrl function.
>
> I think you're missing a critical point here.
>
> There isn't any logic in the module within the pvrusb2 driver which
> "decides" which controls to expose.  The driver itself has an internal
> API used by everything else, and all that pvrusb2-sysfs.c does is
> enumerate that to expose all of the controls.  It would actually be
> extra work in the pvrusb2 driver to impose a policy on which controls
> are visible.  The cropping parameters are an example of this.  Another
> pvrusb2 driver user wanted to get cropping to work (which unfortunately
> *still* requires v4l-dvb changes that are not implemented).  Part of his
> implementation involved adding a few extra knobs within the driver to
> control the cropping behavior.  Once he did that, the knobs
> automatically became available via sysfs.
>
>
>>
>> It will be nice to hear from you what your experience is.
>
> Here's my experience:
>
> I originally did the sysfs interface back in 2005 as a proof of concept.
> I wanted to prove that the internal API within the driver was
> functionally complete and relatively easy to use.  I had recently
> finished reading the LDDv3 and learned of the sysfs class interface.
> It occurred to me back then that it would be easy to define a sysfs
> class interface for the pvrusb2 driver in terms of that API.  So I did,
> and it was literally a 2 day hack.  It's worked fabulously well ever
> since.  In terms of popularity, I find that the user community loves it,
> while the developer community seems to tolerate it.
>
> I routinely use the sysfs interface for my own testing.  It's just
> simply trivial to tweak things in the driver at the shell level using
> that interface.  I remember once using all the mpeg knobs to experiment
> with all the ways that filtering could be controlled, for example.  A
> common use-case for me during testing is to use mplayer as a dumb
> streaming app while tweaking the driver via sysfs.
>
> I've had feedback from many users over the years who, upon discovering
> this interface, just latch onto it.  On more than one occassion I've had
> feedback to the effect of "I don't care about any V4L app; I just want
> to stream video and the sysfs interface makes this trivial to control".
> Probably the fact that only just a small handful of V4L apps (xawtv,
> mythtv and recently now mplayer?) even handle mpeg video has amplified
> this behavior: Many users have found it simpler to forget about V4L apps
> and just instead "cat /dev/video0" along with some shell hacking.
>
> And long ago I know the VDR application gained a plugin script which
> itself used the sysfs interface to control the driver.
>
> One aspect of that sysfs interface that I think has had a lot of value
> over time is that it is (within limits) self-describing.  For each
> control you get not only the ability to get (and usually set) its value,
> but you can inspect its limits, learn what the default value is,
> retrieve a description (though being english only that has limited
> utility), and discover its type.  The sysfs interface in the driver
> tries to use symbolic values where possible, including when setting /
> clearing bits in a bit mask.  The fact that such information is there
> helps when writing, say, a generic dialog-based wrapper to provide a TUI
> (Text User Interface) over the interface without having to encode too
> much specific information in the wrapper itself.  Thus if new controls
> get added (or changed) later then such things just automatically adapt.
> Of course this isn't a perfect thing, but it helps.
>
>>
>> Comments? Ideas? Once we commit to something it is there for a long time to
>> come since sysfs is after all a public API (although it seems to be more
>> changable than ioctls).
>
> I have found it useful over a long period of time.
>
> I also suggest that if such an interface is defined in the general case
> that it should include some introspection capabilities, which will make
> it easier (or even possible) to evolve the interface over time while
> minimizing backwards compatibility issues.
>
> If such a generic interface were made available, I could see an argument
> to remove the sysfs interface from the pvrusb2 driver.  HOWEVER there is
> a community using it, and also unless the generic interface were a
> complete replacement, the pvrusb2 piece will probably have to stay.
> (Note: the sysfs interface in the pvrusb2 driver is already a CONFIG_XX
> parameter so it's easy to deselect it when building the driver.)
>
>    -Mike
>
>

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

* Re: RFC: exposing controls in sysfs
  2010-04-07 18:50   ` Lars Hanisch
@ 2010-04-07 22:15     ` hermann pitton
  2010-04-08  0:47       ` Mike Isely
  0 siblings, 1 reply; 28+ messages in thread
From: hermann pitton @ 2010-04-07 22:15 UTC (permalink / raw)
  To: Lars Hanisch; +Cc: Mike Isely, Hans Verkuil, linux-media

Hi,

Am Mittwoch, den 07.04.2010, 20:50 +0200 schrieb Lars Hanisch:
> Am 06.04.2010 16:33, schrieb Mike Isely:
[snip]
> >>
> >> Mike, do you know of anyone actively using that additional information?
> >
> > Yes.
> >
> > The VDR project at one time implemented a plugin to directly interface
> > to the pvrusb2 driver in this manner.  I do not know if it is still
> > being used since I don't maintain that plugin.
> 
>   Just FYI:
>   The PVR USB2 device is now handled by the pvrinput-plugin, which uses only ioctls. The "old" pvrusb2-plugin is obsolete.
> 
>   http://projects.vdr-developer.org/projects/show/plg-pvrinput
> 
> Regards,
> Lars.

[snip]

thanks Lars.

Mike is really caring and went out for even any most obscure tuner bit
to help to improve such stuff in the past, when we have been without any
data sheets.

To open second, maybe third and even forth ways for apps to use a
device, likely going out of sync soon, does only load maintenance work
without real gain.

We should stay sharp to discover something others don't want to let us
know about. All other ideas about markets are illusions. Or?

So, debugfs sounds much better than sysfs for my taste.

Any app and any driver, going out of sync on the latter, will remind us
that backward compat _must always be guaranteed_  ...

Or did change anything on that and is sysfs excluded from that rule?

Cheers,
Hermann





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

* Re: RFC: exposing controls in sysfs
  2010-04-07 22:15     ` hermann pitton
@ 2010-04-08  0:47       ` Mike Isely
  2010-04-08  0:58         ` Mike Isely
  2010-04-08 17:57         ` Lars Hanisch
  0 siblings, 2 replies; 28+ messages in thread
From: Mike Isely @ 2010-04-08  0:47 UTC (permalink / raw)
  To: hermann pitton; +Cc: Lars Hanisch, Hans Verkuil, linux-media

On Thu, 8 Apr 2010, hermann pitton wrote:

> Hi,
> 
> Am Mittwoch, den 07.04.2010, 20:50 +0200 schrieb Lars Hanisch:
> > Am 06.04.2010 16:33, schrieb Mike Isely:
> [snip]
> > >>
> > >> Mike, do you know of anyone actively using that additional information?
> > >
> > > Yes.
> > >
> > > The VDR project at one time implemented a plugin to directly interface
> > > to the pvrusb2 driver in this manner.  I do not know if it is still
> > > being used since I don't maintain that plugin.
> > 
> >   Just FYI:
> >   The PVR USB2 device is now handled by the pvrinput-plugin, which uses only ioctls. The "old" pvrusb2-plugin is obsolete.
> > 
> >   http://projects.vdr-developer.org/projects/show/plg-pvrinput

Lars:

Thanks for letting me know about that - until this message I had no idea 
if VDR was still using that interface.


> > 
> > Regards,
> > Lars.
> 
> [snip]
> 
> thanks Lars.
> 
> Mike is really caring and went out for even any most obscure tuner bit
> to help to improve such stuff in the past, when we have been without any
> data sheets.

Hermann:

You might have me confused with Mike Krufky there - he's the one who did 
so much of the tuner driver overhauling in v4l-dvb in the past.


> 
> To open second, maybe third and even forth ways for apps to use a
> device, likely going out of sync soon, does only load maintenance work
> without real gain.

Well it was an experiment at the time to see how well such a concept 
would work.  I had done it in a way to minimize maintenance load going 
forward.  On both counts I feel the interface actually has done very 
well, nonstandard though it may be.

I still get the general impression that the user community really has 
liked the sysfs interface, but the developers never really got very fond 
of it :-(


> 
> We should stay sharp to discover something others don't want to let us
> know about. All other ideas about markets are illusions. Or?
> 
> So, debugfs sounds much better than sysfs for my taste.
> 
> Any app and any driver, going out of sync on the latter, will remind us
> that backward compat _must always be guaranteed_  ...
> 
> Or did change anything on that and is sysfs excluded from that rule?

Backwards compatibility is very important and thus any kind of new 
interface deserves a lot of forethought to ensure that choices are made 
in the present that people will regret in the future.  Making an 
interface self-describing is one way that helps with compatibility: if 
the app can discover on its own how to use the interface then it can 
adapt to interface changes in the future.  I think a lot of people get 
their brains so wrapped around the "ioctl-way" of doing things and then 
they try to map that concept into a sysfs-like (or debugfs-like) 
abstraction that they don't see how to naturally take advantage of what 
is possible there.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-07  6:56             ` Hans Verkuil
@ 2010-04-08  0:52               ` Mike Isely
  0 siblings, 0 replies; 28+ messages in thread
From: Mike Isely @ 2010-04-08  0:52 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Laurent Pinchart, Andy Walls, linux-media

On Wed, 7 Apr 2010, Hans Verkuil wrote:

   [...]

> 
> Perhaps we should just not do this in sysfs at all but in debugfs? We have a
> lot more freedom there. No requirement of one-value-per-file, and if we need
> to we can change things in the future. It would actually be easier to issue
> ioctl commands to a driver from debugfs since we have a proper struct file
> there.
> 
> It could be implemented as a separate module that can be loaded if debugfs is
> enabled and suddenly you have all this extra debug functionality.
> 
> I admit, I would really enjoy writing something like this. I just don't want
> to do this in sysfs as that makes it too 'official' so to speak. In other words,
> mainline applications should not use sysfs, but home-grown scripts are free to
> use it as far as I am concerned.
> 
> How much of a problem would that be for you, Mike? On the one hand users have
> to mount debugfs, but on the other hand it will be consistent for all drivers
> that use the control framework. And you should be able to ditch a substantial
> amount of code :-)

Adding a debugfs interface that can be used by all V4L drivers is 
obviously a concept I would not have any problem with.

However that does not necessarily mean that I would agree with eventual 
removal of the pvrusb2 driver's existing sysfs interface.  That would 
depend on whether or not doing such a thing loses functionality and what 
the driver's user community would think about it.

  -Mike


-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-08  0:47       ` Mike Isely
@ 2010-04-08  0:58         ` Mike Isely
  2010-04-08 17:57         ` Lars Hanisch
  1 sibling, 0 replies; 28+ messages in thread
From: Mike Isely @ 2010-04-08  0:58 UTC (permalink / raw)
  To: hermann pitton; +Cc: Lars Hanisch, Hans Verkuil, linux-media

On Wed, 7 Apr 2010, Mike Isely wrote:

> 
> Backwards compatibility is very important and thus any kind of new 
> interface deserves a lot of forethought to ensure that choices are made 
> in the present that people will regret in the future.  Making an 

"in the present that people will NOT (!!) regret in the future."

Holy cow, what a typo...  :-)

  -Mike

> interface self-describing is one way that helps with compatibility: if 
> the app can discover on its own how to use the interface then it can 
> adapt to interface changes in the future.  I think a lot of people get 
> their brains so wrapped around the "ioctl-way" of doing things and then 
> they try to map that concept into a sysfs-like (or debugfs-like) 
> abstraction that they don't see how to naturally take advantage of what 
> is possible there.
> 
>   -Mike
> 
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8

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

* Re: RFC: exposing controls in sysfs
  2010-04-08  0:47       ` Mike Isely
  2010-04-08  0:58         ` Mike Isely
@ 2010-04-08 17:57         ` Lars Hanisch
  1 sibling, 0 replies; 28+ messages in thread
From: Lars Hanisch @ 2010-04-08 17:57 UTC (permalink / raw)
  To: Mike Isely; +Cc: hermann pitton, Hans Verkuil, linux-media

Am 08.04.2010 02:47, schrieb Mike Isely:
> On Thu, 8 Apr 2010, hermann pitton wrote:
>
>> Hi,
>>
>> Am Mittwoch, den 07.04.2010, 20:50 +0200 schrieb Lars Hanisch:
>>> Am 06.04.2010 16:33, schrieb Mike Isely:
>> [snip]
>>>>>
>>>>> Mike, do you know of anyone actively using that additional information?
>>>>
>>>> Yes.
>>>>
>>>> The VDR project at one time implemented a plugin to directly interface
>>>> to the pvrusb2 driver in this manner.  I do not know if it is still
>>>> being used since I don't maintain that plugin.
>>>
>>>    Just FYI:
>>>    The PVR USB2 device is now handled by the pvrinput-plugin, which uses only ioctls. The "old" pvrusb2-plugin is obsolete.
>>>
>>>    http://projects.vdr-developer.org/projects/show/plg-pvrinput
>
> Lars:
>
> Thanks for letting me know about that - until this message I had no idea
> if VDR was still using that interface.
>
>
>>>
>>> Regards,
>>> Lars.
>>
>> [snip]
>>
>> thanks Lars.
>>
>> Mike is really caring and went out for even any most obscure tuner bit
>> to help to improve such stuff in the past, when we have been without any
>> data sheets.
>
> Hermann:
>
> You might have me confused with Mike Krufky there - he's the one who did
> so much of the tuner driver overhauling in v4l-dvb in the past.
>
>
>>
>> To open second, maybe third and even forth ways for apps to use a
>> device, likely going out of sync soon, does only load maintenance work
>> without real gain.
>
> Well it was an experiment at the time to see how well such a concept
> would work.  I had done it in a way to minimize maintenance load going
> forward.  On both counts I feel the interface actually has done very
> well, nonstandard though it may be.
>
> I still get the general impression that the user community really has
> liked the sysfs interface, but the developers never really got very fond
> of it :-(

  From my point of view as an application developer I never tried to use
sysfs at all. I admit that it's nice to use from a shell script in "known
environments" (like setting up a card for recording with cat etc.) but what
about error handling? How will I (the script) know, if setting a control is
successful or not? Currently I don't know if v4l2-ctl returns some useful
exit code, but with ioctls it's a lot easier to track errors.
  I never liked to handle with directories and files, reading and writing if
there's a function which is doing the same thing in a much easier way. :-)

  But all this might be related to my not-really-present knowledge of using
sysfs in the right way.

  And reading other posts debugfs seems to be the better choice (just read
some articles on it to get a general survey of it).

Regards,
Lars.

>
>
>>
>> We should stay sharp to discover something others don't want to let us
>> know about. All other ideas about markets are illusions. Or?
>>
>> So, debugfs sounds much better than sysfs for my taste.
>>
>> Any app and any driver, going out of sync on the latter, will remind us
>> that backward compat _must always be guaranteed_  ...
>>
>> Or did change anything on that and is sysfs excluded from that rule?
>
> Backwards compatibility is very important and thus any kind of new
> interface deserves a lot of forethought to ensure that choices are made
> in the present that people will regret in the future.  Making an
> interface self-describing is one way that helps with compatibility: if
> the app can discover on its own how to use the interface then it can
> adapt to interface changes in the future.  I think a lot of people get
> their brains so wrapped around the "ioctl-way" of doing things and then
> they try to map that concept into a sysfs-like (or debugfs-like)
> abstraction that they don't see how to naturally take advantage of what
> is possible there.
>
>    -Mike
>

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

end of thread, other threads:[~2010-04-08 17:58 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-05 21:47 RFC: exposing controls in sysfs Hans Verkuil
2010-04-05 22:12 ` Hans Verkuil
2010-04-06  6:37   ` Hans Verkuil
2010-04-06 11:06     ` Andy Walls
2010-04-06 11:27       ` Laurent Pinchart
2010-04-06 11:44         ` Markus Rechberger
2010-04-06 15:08           ` Mike Isely
2010-04-06 15:16         ` Mike Isely
2010-04-06 22:39           ` Hans Verkuil
2010-04-07  2:10             ` hermann pitton
2010-04-07  6:56             ` Hans Verkuil
2010-04-08  0:52               ` Mike Isely
2010-04-06 13:16     ` Mauro Carvalho Chehab
2010-04-06 13:44       ` Hans Verkuil
2010-04-06 13:59         ` Devin Heitmueller
2010-04-06 15:05           ` Mike Isely
2010-04-06 14:32         ` Mauro Carvalho Chehab
2010-04-06 16:06           ` Jonathan Cameron
2010-04-06 16:36             ` Bjørn Forsman
2010-04-06 14:41   ` Mike Isely
2010-04-06 16:19     ` Jonathan Cameron
2010-04-06 22:47       ` Hans Verkuil
2010-04-06 14:33 ` Mike Isely
2010-04-07 18:50   ` Lars Hanisch
2010-04-07 22:15     ` hermann pitton
2010-04-08  0:47       ` Mike Isely
2010-04-08  0:58         ` Mike Isely
2010-04-08 17:57         ` Lars Hanisch

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.