Re: RFC: exposing controls in sysfs

2010-04-08 Thread Lars Hanisch

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


--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-07 Thread Hans Verkuil
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 

Re: RFC: exposing controls in sysfs

2010-04-07 Thread Lars Hanisch

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_crcchroma_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_typevideo_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_modemedian_filter_typetemporal_filter  
   video_peak_bitrate
audio_stereo_mode_extension  median_luma_filter_maximumtemporal_filter_mode 
   video_temporal_decimation
balance  median_luma_filter_minimumvideo_aspect 
   volume
brightness   mute  video_b_frames
chroma_agc   saturationvideo_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 

Re: RFC: exposing controls in sysfs

2010-04-07 Thread hermann pitton
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




--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-07 Thread 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 :-(


 
 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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-07 Thread Mike Isely
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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Hans Verkuil
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_crcchroma_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_typevideo_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_modemedian_filter_typetemporal_filter  
 video_peak_bitrate
  audio_stereo_mode_extension  median_luma_filter_maximum
  temporal_filter_modevideo_temporal_decimation
  balance  median_luma_filter_minimumvideo_aspect 
 volume
  brightness   mute  video_b_frames
  chroma_agc   saturationvideo_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
brightnessmpeg_median_chroma_filter_maximum  
mpeg_video_b_frames
chroma_agcmpeg_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_crcmpeg_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_modempeg_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 

Re: RFC: exposing controls in sysfs

2010-04-06 Thread Andy Walls
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
 brightnessmpeg_median_chroma_filter_maximum  
 mpeg_video_b_frames
 chroma_agcmpeg_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_crcmpeg_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_modempeg_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 

Re: RFC: exposing controls in sysfs

2010-04-06 Thread Laurent Pinchart
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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Markus Rechberger
On Tue, Apr 6, 2010 at 1:27 PM, Laurent Pinchart
laurent.pinch...@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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mauro Carvalho Chehab
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_crcchroma_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_typevideo_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_modemedian_filter_typetemporal_filter  
video_peak_bitrate
 audio_stereo_mode_extension  median_luma_filter_maximum
 temporal_filter_modevideo_temporal_decimation
 balance  median_luma_filter_minimumvideo_aspect 
volume
 brightness   mute  video_b_frames
 chroma_agc   saturationvideo_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
 brightnessmpeg_median_chroma_filter_maximum  
 mpeg_video_b_frames
 chroma_agcmpeg_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_crcmpeg_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_modempeg_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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Hans Verkuil

 Hans Verkuil wrote:
 $ ls /sys/class/video4linux/video1/controls
 balance   mpeg_insert_navigation_packets
 mpeg_video_aspect
 brightnessmpeg_median_chroma_filter_maximum
 mpeg_video_b_frames
 chroma_agcmpeg_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_crcmpeg_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_modempeg_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

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Devin Heitmueller
On Tue, Apr 6, 2010 at 9:44 AM, Hans Verkuil hverk...@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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mauro Carvalho Chehab
Hans Verkuil wrote:
 Hans Verkuil wrote:
 $ ls /sys/class/video4linux/video1/controls
 balance   mpeg_insert_navigation_packets
 mpeg_video_aspect
 brightnessmpeg_median_chroma_filter_maximum
 mpeg_video_b_frames
 chroma_agcmpeg_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_crcmpeg_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_modempeg_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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread 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_crcchroma_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_typevideo_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_modemedian_filter_typetemporal_filter
  video_peak_bitrate
 audio_stereo_mode_extension  median_luma_filter_maximum
 temporal_filter_modevideo_temporal_decimation
 balance  median_luma_filter_minimumvideo_aspect   
  volume
 brightness   mute  video_b_frames
 chroma_agc   saturationvideo_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 

Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
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 0x 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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Mike Isely
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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Jonathan Cameron
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
 brightnessmpeg_median_chroma_filter_maximum
 mpeg_video_b_frames
 chroma_agcmpeg_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_crcmpeg_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_modempeg_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.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Jonathan Cameron
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 0x 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.
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-06 Thread Bjørn Forsman
On 6 April 2010 18:06, Jonathan Cameron ji...@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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RFC: exposing controls in sysfs

2010-04-05 Thread Hans Verkuil
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_crcchroma_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_typevideo_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_modemedian_filter_typetemporal_filter
  video_peak_bitrate
 audio_stereo_mode_extension  median_luma_filter_maximum
 temporal_filter_modevideo_temporal_decimation
 balance  median_luma_filter_minimumvideo_aspect   
  volume
 brightness   mute  video_b_frames
 chroma_agc   saturationvideo_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 0x 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