* 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.