Re: RFC: exposing controls in sysfs
Am 08.04.2010 02:47, schrieb Mike Isely: On Thu, 8 Apr 2010, hermann pitton wrote: Hi, Am Mittwoch, den 07.04.2010, 20:50 +0200 schrieb Lars Hanisch: Am 06.04.2010 16:33, schrieb Mike Isely: [snip] Mike, do you know of anyone actively using that additional information? Yes. The VDR project at one time implemented a plugin to directly interface to the pvrusb2 driver in this manner. I do not know if it is still being used since I don't maintain that plugin. Just FYI: The PVR USB2 device is now handled by the pvrinput-plugin, which uses only ioctls. The old pvrusb2-plugin is obsolete. http://projects.vdr-developer.org/projects/show/plg-pvrinput Lars: Thanks for letting me know about that - until this message I had no idea if VDR was still using that interface. Regards, Lars. [snip] thanks Lars. Mike is really caring and went out for even any most obscure tuner bit to help to improve such stuff in the past, when we have been without any data sheets. Hermann: You might have me confused with Mike Krufky there - he's the one who did so much of the tuner driver overhauling in v4l-dvb in the past. To open second, maybe third and even forth ways for apps to use a device, likely going out of sync soon, does only load maintenance work without real gain. Well it was an experiment at the time to see how well such a concept would work. I had done it in a way to minimize maintenance load going forward. On both counts I feel the interface actually has done very well, nonstandard though it may be. I still get the general impression that the user community really has liked the sysfs interface, but the developers never really got very fond of it :-( From my point of view as an application developer I never tried to use sysfs at all. I admit that it's nice to use from a shell script in known environments (like setting up a card for recording with cat etc.) but what about error handling? How will I (the script) know, if setting a control is successful or not? Currently I don't know if v4l2-ctl returns some useful exit code, but with ioctls it's a lot easier to track errors. I never liked to handle with directories and files, reading and writing if there's a function which is doing the same thing in a much easier way. :-) But all this might be related to my not-really-present knowledge of using sysfs in the right way. And reading other posts debugfs seems to be the better choice (just read some articles on it to get a general survey of it). Regards, Lars. We should stay sharp to discover something others don't want to let us know about. All other ideas about markets are illusions. Or? So, debugfs sounds much better than sysfs for my taste. Any app and any driver, going out of sync on the latter, will remind us that backward compat _must always be guaranteed_ ... Or did change anything on that and is sysfs excluded from that rule? Backwards compatibility is very important and thus any kind of new interface deserves a lot of forethought to ensure that choices are made in the present that people will regret in the future. Making an interface self-describing is one way that helps with compatibility: if the app can discover on its own how to use the interface then it can adapt to interface changes in the future. I think a lot of people get their brains so wrapped around the ioctl-way of doing things and then they try to map that concept into a sysfs-like (or debugfs-like) abstraction that they don't see how to naturally take advantage of what is possible there. -Mike -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Wednesday 07 April 2010 00:39:20 Hans Verkuil wrote: On Tuesday 06 April 2010 17:16:17 Mike Isely wrote: On Tue, 6 Apr 2010, Laurent Pinchart wrote: Hi Andy, On Tuesday 06 April 2010 13:06:18 Andy Walls wrote: On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote: [snip] Again, I still don't know whether we should do this. It is dangerously seductive because it would be so trivial to implement. It's like watching ships run aground on a shallow sandbar that all the locals know about. The waters off of 'Point /sys' are full of usability shipwrecks. I don't know if it's some siren's song, the lack of a light house, or just strange currents that deceive even seasoned navigators Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control metatdata. It's not as easy as typing 'cat', but the user base using sysfs in an interactive shell or shell script should also know how to use v4l2-ctl. In embedded systems, the final system deployment should not need the control metadata available from sysfs in a command shell anyway. I fully agree with this. If we push the idea one step further, why do we need to expose controls in sysfs at all ? I have found it useful to have the sysfs interface within the pvrusb2 driver. If it is going to take a lot of work to specifically craft a sysfs interface that exports the V4L API, then it will probably be a pain to maintain going forward. By a lot of work I mean that each V4L API function would have to be explicitly coded for in this interface, thus as the V4L API evolves over time then extra work must be expended each time to keep the sysfs interface in step. If that is to be the case then it may not be worth it. No, that is no work at all. For all practical purposes there are two objects (OK, really three, but the node object is just internal). The first object is the control handler, the second is the control object. Handlers have controls. Handlers can also inherit controls from other handlers. If they already had the same control, then they effectively override the inherited control. Controls can also be private to a handler, i.e. they will never be inherited by other handlers. Sound familiar? It's your basic C++ class inheritance scheme with public and private fields (or controls in this case). The sysfs implementation is just bolted on top of this: each video_device struct is associated with a control handler and sysfs will expose all controls that that handler references. You can take a look at the header of the control framework: http://linuxtv.org/hg/~hverkuil/v4l-dvb-fw/raw-file/bf7cd2fb7a35/linux/include/media/v4l2-ctrls.h brainstorm mode on It would be trivial to add e.g. a V4L2 control class that could be used to expose all sorts of V4L2 functionality to sysfs. It would be handled differently in that you don't want to expose those through VIDIOC_QUERYCTRL and friends, just to sysfs. Heck, it could be implemented almost completely transparently from drivers. For example, an 'echo 1 /sys//v4l2_input' could be converted automatically to a VIDIOC_S_INPUT command that's issued to the driver. Similar to what you did in pvrusb2, except you went the other way around: ioctls are converted to controls. That is not feasible, though, since you do not want to completely redo all drivers. There are definitely some snags, but the basic premise is sound. Of course, just the fact that you can easily do something does not mean that you should. The first version of the framework will not contain any sysfs. It is clear that the last word has not been said on this. It's not a big deal, sysfs was just an add-on and not part of the core. But having it in the kernel will make it a nice foundation on which to experiment. Just a thought experiment: take VIDIOC_S_FREQUENCY. The struct has three fields: tuner, type, frequency. So that's a cluster of three controls. So you would need a 's_ctrl' function like this: switch (id) { /* handle the frequency cluster */ case V4L2_CID_V4L_FREQ_TUNER: struct v4l2_frequency f; f.tuner = freq_tuner-cur.val; f.type = freq_type-cur.val; f.frequency = freq_freq-cur.val; return vdev-ioctl_ops-s_frequency(f); } Pseudo-code, of course, and there are some little things like the 'file' and 'fh' args to s_frequency, but you could use the framework to make a very clean implementation of this. Especially since the framework supports 'clustering' of controls. Effectively creating a single composite control from the point of view of the driver. Hmm, sounds awfully like a struct, doesn't it? :-) In the pvrusb2 driver this has not been the case because the code I wrote which implements the sysfs interface for the driver does
Re: RFC: exposing controls in sysfs
Am 06.04.2010 16:33, schrieb Mike Isely: Comments below... On Mon, 5 Apr 2010, Hans Verkuil wrote: Hi all, The new control framework makes it very easy to expose controls in sysfs. The way it is implemented now in the framework is that each device node will get a 'controls' subdirectory in sysfs. Below which are all the controls associated with that device node. So different device nodes can have different controls if so desired. The name of each sysfs file is derived from the control name, basically making it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non- alphanumerical characters. Seems to work well. For numerical controls you can write numbers in decimal, octal or hexadecimal. When you write to a button control it will ignore what you wrote, but still execute the action. It looks like this for ivtv: $ ls /sys/class/video4linux/video1 controls dev device index name power subsystem uevent $ ls /sys/class/video4linux/video1/controls audio_crcchroma_gain spatial_chroma_filter_type video_bitrate_mode audio_emphasis contrast spatial_filter video_encoding audio_encoding hue spatial_filter_mode video_gop_closure audio_layer_ii_bitrate insert_navigation_packets spatial_luma_filter_typevideo_gop_size audio_mute median_chroma_filter_maximum stream_type video_mute audio_sampling_frequency median_chroma_filter_minimum stream_vbi_format video_mute_yuv audio_stereo_modemedian_filter_typetemporal_filter video_peak_bitrate audio_stereo_mode_extension median_luma_filter_maximumtemporal_filter_mode video_temporal_decimation balance median_luma_filter_minimumvideo_aspect volume brightness mute video_b_frames chroma_agc saturationvideo_bitrate The question is, is this sufficient? One of the few drivers that exposes controls in sysfs is pvrusb2. As far as I can tell from the source it will create subdirectories under the device node for each control. Those subdirs have the name ctl_control-name (e.g. ctl_volume), and below that are files exposing all the attributes of that control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val and bit_val. Most are clear, but some are a bit more obscure. enum_val is basically a QUERYMENU and returns all menu options. bit_val seems to be used for some non-control values like the TV standard that pvrusb2 also exposes and where bit_val is a bit mask of all the valid bits that can be used. Mike, if you have any additional information, just let us know. My pvrusb2 is in another country at the moment so I can't do any testing. Hans: What you see in the pvrusb2 driver is the result of an idea I had back in 2005. The pvrusb2 driver has an internal control API; my original idea back then was to then reimplement other interfaces on top of that API, in a manner that is as orthogonal as possible. The reality today is still pretty close to that concept (except for DVB unfortunately since that framework's architecture effectively has to take over the RF tuner...); the V4L2 implementation in the driver certainly works this way. The sysfs interface you see here is the result of implementing the same API through sysfs. Right now with the pvrusb2 driver the only thing not exported through sysfs is the actual streaming of video itself. The entire sysfs implementation in the driver can be found in pvrusb2-sysfs.c. Notice that the file is generic; there is not anything in it that is specific to any particular control. Rather, pvrusb2-sysfs.c is able to iterate through the driver's controls, picking up the control's name, its type, and accessors. Based on what it finds, this module then synthesizes the interface that you see in /class/pvrusb2/* - it's actually possible to add new controls to the driver without changing anything in pvrusb2-sysfs.c. Personally I think that it is overkill to basically expose the whole QUERYCTRL information to sysfs. I see it as an easy and quick way to read and modify controls via a command line. Over time, I have ended up using pretty much every control in that interface. Obviously not every control always gets touched, but I have found it extremely valuable to have such direct access to every knob in the driver this way. Also, the original concept was that the interface was to be orthogonal; in theory any kind of control action in one interface should be just as valid in another. Mike, do you know of anyone actively using that additional information? Yes. The VDR project at one time implemented a plugin to directly interface to the pvrusb2 driver in this manner. I do not know if it is still being used
Re: RFC: exposing controls in sysfs
Hi, Am Mittwoch, den 07.04.2010, 20:50 +0200 schrieb Lars Hanisch: Am 06.04.2010 16:33, schrieb Mike Isely: [snip] Mike, do you know of anyone actively using that additional information? Yes. The VDR project at one time implemented a plugin to directly interface to the pvrusb2 driver in this manner. I do not know if it is still being used since I don't maintain that plugin. Just FYI: The PVR USB2 device is now handled by the pvrinput-plugin, which uses only ioctls. The old pvrusb2-plugin is obsolete. http://projects.vdr-developer.org/projects/show/plg-pvrinput Regards, Lars. [snip] thanks Lars. Mike is really caring and went out for even any most obscure tuner bit to help to improve such stuff in the past, when we have been without any data sheets. To open second, maybe third and even forth ways for apps to use a device, likely going out of sync soon, does only load maintenance work without real gain. We should stay sharp to discover something others don't want to let us know about. All other ideas about markets are illusions. Or? So, debugfs sounds much better than sysfs for my taste. Any app and any driver, going out of sync on the latter, will remind us that backward compat _must always be guaranteed_ ... Or did change anything on that and is sysfs excluded from that rule? Cheers, Hermann -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Thu, 8 Apr 2010, hermann pitton wrote: Hi, Am Mittwoch, den 07.04.2010, 20:50 +0200 schrieb Lars Hanisch: Am 06.04.2010 16:33, schrieb Mike Isely: [snip] Mike, do you know of anyone actively using that additional information? Yes. The VDR project at one time implemented a plugin to directly interface to the pvrusb2 driver in this manner. I do not know if it is still being used since I don't maintain that plugin. Just FYI: The PVR USB2 device is now handled by the pvrinput-plugin, which uses only ioctls. The old pvrusb2-plugin is obsolete. http://projects.vdr-developer.org/projects/show/plg-pvrinput Lars: Thanks for letting me know about that - until this message I had no idea if VDR was still using that interface. Regards, Lars. [snip] thanks Lars. Mike is really caring and went out for even any most obscure tuner bit to help to improve such stuff in the past, when we have been without any data sheets. Hermann: You might have me confused with Mike Krufky there - he's the one who did so much of the tuner driver overhauling in v4l-dvb in the past. To open second, maybe third and even forth ways for apps to use a device, likely going out of sync soon, does only load maintenance work without real gain. Well it was an experiment at the time to see how well such a concept would work. I had done it in a way to minimize maintenance load going forward. On both counts I feel the interface actually has done very well, nonstandard though it may be. I still get the general impression that the user community really has liked the sysfs interface, but the developers never really got very fond of it :-( We should stay sharp to discover something others don't want to let us know about. All other ideas about markets are illusions. Or? So, debugfs sounds much better than sysfs for my taste. Any app and any driver, going out of sync on the latter, will remind us that backward compat _must always be guaranteed_ ... Or did change anything on that and is sysfs excluded from that rule? Backwards compatibility is very important and thus any kind of new interface deserves a lot of forethought to ensure that choices are made in the present that people will regret in the future. Making an interface self-describing is one way that helps with compatibility: if the app can discover on its own how to use the interface then it can adapt to interface changes in the future. I think a lot of people get their brains so wrapped around the ioctl-way of doing things and then they try to map that concept into a sysfs-like (or debugfs-like) abstraction that they don't see how to naturally take advantage of what is possible there. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Wed, 7 Apr 2010, Hans Verkuil wrote: [...] Perhaps we should just not do this in sysfs at all but in debugfs? We have a lot more freedom there. No requirement of one-value-per-file, and if we need to we can change things in the future. It would actually be easier to issue ioctl commands to a driver from debugfs since we have a proper struct file there. It could be implemented as a separate module that can be loaded if debugfs is enabled and suddenly you have all this extra debug functionality. I admit, I would really enjoy writing something like this. I just don't want to do this in sysfs as that makes it too 'official' so to speak. In other words, mainline applications should not use sysfs, but home-grown scripts are free to use it as far as I am concerned. How much of a problem would that be for you, Mike? On the one hand users have to mount debugfs, but on the other hand it will be consistent for all drivers that use the control framework. And you should be able to ditch a substantial amount of code :-) Adding a debugfs interface that can be used by all V4L drivers is obviously a concept I would not have any problem with. However that does not necessarily mean that I would agree with eventual removal of the pvrusb2 driver's existing sysfs interface. That would depend on whether or not doing such a thing loses functionality and what the driver's user community would think about it. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Tuesday 06 April 2010 00:12:48 Hans Verkuil wrote: On Monday 05 April 2010 23:47:10 Hans Verkuil wrote: Hi all, The new control framework makes it very easy to expose controls in sysfs. The way it is implemented now in the framework is that each device node will get a 'controls' subdirectory in sysfs. Below which are all the controls associated with that device node. So different device nodes can have different controls if so desired. The name of each sysfs file is derived from the control name, basically making it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non- alphanumerical characters. Seems to work well. For numerical controls you can write numbers in decimal, octal or hexadecimal. When you write to a button control it will ignore what you wrote, but still execute the action. It looks like this for ivtv: $ ls /sys/class/video4linux/video1 controls dev device index name power subsystem uevent $ ls /sys/class/video4linux/video1/controls audio_crcchroma_gain spatial_chroma_filter_type video_bitrate_mode audio_emphasis contrast spatial_filter video_encoding audio_encoding hue spatial_filter_mode video_gop_closure audio_layer_ii_bitrate insert_navigation_packets spatial_luma_filter_typevideo_gop_size audio_mute median_chroma_filter_maximum stream_type video_mute audio_sampling_frequency median_chroma_filter_minimum stream_vbi_format video_mute_yuv audio_stereo_modemedian_filter_typetemporal_filter video_peak_bitrate audio_stereo_mode_extension median_luma_filter_maximum temporal_filter_modevideo_temporal_decimation balance median_luma_filter_minimumvideo_aspect volume brightness mute video_b_frames chroma_agc saturationvideo_bitrate The question is, is this sufficient? One thing that might be useful is to prefix the name with the control class name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would groups them better. Or one could make a controls/user and controls/mpeg directory. That might not be such a bad idea actually. Replying to your own mails is probably a bad sign, but I can't help myself :-) I've changed the code to add a control class prefix for all but the user controls. It looks much better now: $ ls /sys/class/video4linux/video1/controls balance mpeg_insert_navigation_packets mpeg_video_aspect brightnessmpeg_median_chroma_filter_maximum mpeg_video_b_frames chroma_agcmpeg_median_chroma_filter_minimum mpeg_video_bitrate chroma_gain mpeg_median_filter_type mpeg_video_bitrate_mode contrast mpeg_median_luma_filter_maximum mpeg_video_encoding hue mpeg_median_luma_filter_minimum mpeg_video_gop_closure mpeg_audio_crcmpeg_spatial_chroma_filter_type mpeg_video_gop_size mpeg_audio_emphasis mpeg_spatial_filter mpeg_video_mute mpeg_audio_encoding mpeg_spatial_filter_mode mpeg_video_mute_yuv mpeg_audio_layer_ii_bitrate mpeg_spatial_luma_filter_type mpeg_video_peak_bitrate mpeg_audio_mute mpeg_stream_type mpeg_video_temporal_decimation mpeg_audio_sampling_frequency mpeg_stream_vbi_format mute mpeg_audio_stereo_modempeg_temporal_filter saturation mpeg_audio_stereo_mode_extension mpeg_temporal_filter_mode volume One of the few drivers that exposes controls in sysfs is pvrusb2. As far as I can tell from the source it will create subdirectories under the device node for each control. Those subdirs have the name ctl_control-name (e.g. ctl_volume), and below that are files exposing all the attributes of that control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val and bit_val. Most are clear, but some are a bit more obscure. enum_val is basically a QUERYMENU and returns all menu options. bit_val seems to be used for some non-control values like the TV standard that pvrusb2 also exposes and where bit_val is a bit mask of all the valid bits that can be used. Mike, if you have any additional information, just let us know. My pvrusb2 is in another country at the moment so I can't do any testing. Personally I think that it is overkill to basically expose the whole QUERYCTRL information to sysfs. I see it as an easy and quick way to read and modify
Re: RFC: exposing controls in sysfs
On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote: On Tuesday 06 April 2010 00:12:48 Hans Verkuil wrote: On Monday 05 April 2010 23:47:10 Hans Verkuil wrote: One thing that might be useful is to prefix the name with the control class name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would groups them better. Or one could make a controls/user and controls/mpeg directory. That might not be such a bad idea actually. Replying to your own mails is probably a bad sign, but I can't help myself :-) I had an old InfoCom text adventure game that would respond to querying oneself with: Talking to oneself is a sign of impending mental collapse. :D I've changed the code to add a control class prefix for all but the user controls. It looks much better now: $ ls /sys/class/video4linux/video1/controls balance mpeg_insert_navigation_packets mpeg_video_aspect brightnessmpeg_median_chroma_filter_maximum mpeg_video_b_frames chroma_agcmpeg_median_chroma_filter_minimum mpeg_video_bitrate chroma_gain mpeg_median_filter_type mpeg_video_bitrate_mode contrast mpeg_median_luma_filter_maximum mpeg_video_encoding hue mpeg_median_luma_filter_minimum mpeg_video_gop_closure mpeg_audio_crcmpeg_spatial_chroma_filter_type mpeg_video_gop_size mpeg_audio_emphasis mpeg_spatial_filter mpeg_video_mute mpeg_audio_encoding mpeg_spatial_filter_mode mpeg_video_mute_yuv mpeg_audio_layer_ii_bitrate mpeg_spatial_luma_filter_type mpeg_video_peak_bitrate mpeg_audio_mute mpeg_stream_type mpeg_video_temporal_decimation mpeg_audio_sampling_frequency mpeg_stream_vbi_format mute mpeg_audio_stereo_modempeg_temporal_filter saturation mpeg_audio_stereo_mode_extension mpeg_temporal_filter_mode volume So this is beginning to look OK. You'll have longer names when a class name is longer than 4 characters (e.g. technician_ ). However, I suppose it is better than another directory which creates a deeper hierarchy while still not avoiding the longer pathname. One of the few drivers that exposes controls in sysfs is pvrusb2. As far as I can tell from the source it will create subdirectories under the device node for each control. Those subdirs have the name ctl_control-name (e.g. ctl_volume), and below that are files exposing all the attributes of that control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val and bit_val. Most are clear, but some are a bit more obscure. enum_val is basically a QUERYMENU and returns all menu options. bit_val seems to be used for some non-control values like the TV standard that pvrusb2 also exposes and where bit_val is a bit mask of all the valid bits that can be used. Mike, if you have any additional information, just let us know. My pvrusb2 is in another country at the moment so I can't do any testing. Personally I think that it is overkill to basically expose the whole QUERYCTRL information to sysfs. I see it as an easy and quick way to read and modify controls via a command line. An in between solution would be to add _type files. So you would have 'hue' and 'hue_type'. 'cat hue_type' would give something like: If we go for something like this, then I think it would be better to make a new subdirectory. So 'controls' just has the controls, and 'ctrl_info' or something similar would have read-only files containing this information. sysfs' major usability problem for humans is the insane directory depths it can reach and the cross-links to everywhere. Humans attempt to keep a mental model of where they are in a logical space, and sysfs is like maze of twisty little passages, all alike. In the true sysfs spirit you should create a 'ctrl_info' directory full of nodes with metadata *and* also create foo_type symlinks to all of those metadata nodes. Bonus points for having the 'ctrl_info' directory and 'foo_type' symlinks in a different part of the sysfs tree but with a similar directory name. Again, I still don't know whether we should do this. It is dangerously seductive because it would be so trivial to implement. It's like watching ships run aground on a shallow sandbar that all the locals know about. The waters off of 'Point /sys' are full of usability shipwrecks. I don't know if it's some siren's song, the lack of a light house, or just strange currents that deceive even seasoned navigators Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control metatdata. It's not as easy as typing 'cat', but the user base using sysfs in an interactive shell or shell
Re: RFC: exposing controls in sysfs
Hi Andy, On Tuesday 06 April 2010 13:06:18 Andy Walls wrote: On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote: [snip] Again, I still don't know whether we should do this. It is dangerously seductive because it would be so trivial to implement. It's like watching ships run aground on a shallow sandbar that all the locals know about. The waters off of 'Point /sys' are full of usability shipwrecks. I don't know if it's some siren's song, the lack of a light house, or just strange currents that deceive even seasoned navigators Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control metatdata. It's not as easy as typing 'cat', but the user base using sysfs in an interactive shell or shell script should also know how to use v4l2-ctl. In embedded systems, the final system deployment should not need the control metadata available from sysfs in a command shell anyway. I fully agree with this. If we push the idea one step further, why do we need to expose controls in sysfs at all ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Tue, Apr 6, 2010 at 1:27 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: Hi Andy, On Tuesday 06 April 2010 13:06:18 Andy Walls wrote: On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote: [snip] Again, I still don't know whether we should do this. It is dangerously seductive because it would be so trivial to implement. It's like watching ships run aground on a shallow sandbar that all the locals know about. The waters off of 'Point /sys' are full of usability shipwrecks. I don't know if it's some siren's song, the lack of a light house, or just strange currents that deceive even seasoned navigators Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control metatdata. It's not as easy as typing 'cat', but the user base using sysfs in an interactive shell or shell script should also know how to use v4l2-ctl. In embedded systems, the final system deployment should not need the control metadata available from sysfs in a command shell anyway. I fully agree with this. If we push the idea one step further, why do we need to expose controls in sysfs at all ? how about security permissions? while you can easily change the permission levels for nodes in /dev you can't do this so easily with sysfs entries. I don't really think this is needed at all some applications will start to use ioctl some other apps might go for sysfs.. this makes the API a little bit whacko Markus -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
Hans Verkuil wrote: On Tuesday 06 April 2010 00:12:48 Hans Verkuil wrote: On Monday 05 April 2010 23:47:10 Hans Verkuil wrote: Hi all, The new control framework makes it very easy to expose controls in sysfs. The way it is implemented now in the framework is that each device node will get a 'controls' subdirectory in sysfs. Below which are all the controls associated with that device node. So different device nodes can have different controls if so desired. The name of each sysfs file is derived from the control name, basically making it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non- alphanumerical characters. Seems to work well. For numerical controls you can write numbers in decimal, octal or hexadecimal. When you write to a button control it will ignore what you wrote, but still execute the action. It looks like this for ivtv: $ ls /sys/class/video4linux/video1 controls dev device index name power subsystem uevent $ ls /sys/class/video4linux/video1/controls audio_crcchroma_gain spatial_chroma_filter_type video_bitrate_mode audio_emphasis contrast spatial_filter video_encoding audio_encoding hue spatial_filter_mode video_gop_closure audio_layer_ii_bitrate insert_navigation_packets spatial_luma_filter_typevideo_gop_size audio_mute median_chroma_filter_maximum stream_type video_mute audio_sampling_frequency median_chroma_filter_minimum stream_vbi_format video_mute_yuv audio_stereo_modemedian_filter_typetemporal_filter video_peak_bitrate audio_stereo_mode_extension median_luma_filter_maximum temporal_filter_modevideo_temporal_decimation balance median_luma_filter_minimumvideo_aspect volume brightness mute video_b_frames chroma_agc saturationvideo_bitrate The question is, is this sufficient? One thing that might be useful is to prefix the name with the control class name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would groups them better. Or one could make a controls/user and controls/mpeg directory. That might not be such a bad idea actually. Replying to your own mails is probably a bad sign, but I can't help myself :-) I've changed the code to add a control class prefix for all but the user controls. It looks much better now: $ ls /sys/class/video4linux/video1/controls balance mpeg_insert_navigation_packets mpeg_video_aspect brightnessmpeg_median_chroma_filter_maximum mpeg_video_b_frames chroma_agcmpeg_median_chroma_filter_minimum mpeg_video_bitrate chroma_gain mpeg_median_filter_type mpeg_video_bitrate_mode contrast mpeg_median_luma_filter_maximum mpeg_video_encoding hue mpeg_median_luma_filter_minimum mpeg_video_gop_closure mpeg_audio_crcmpeg_spatial_chroma_filter_type mpeg_video_gop_size mpeg_audio_emphasis mpeg_spatial_filter mpeg_video_mute mpeg_audio_encoding mpeg_spatial_filter_mode mpeg_video_mute_yuv mpeg_audio_layer_ii_bitrate mpeg_spatial_luma_filter_type mpeg_video_peak_bitrate mpeg_audio_mute mpeg_stream_type mpeg_video_temporal_decimation mpeg_audio_sampling_frequency mpeg_stream_vbi_format mute mpeg_audio_stereo_modempeg_temporal_filter saturation mpeg_audio_stereo_mode_extension mpeg_temporal_filter_mode volume It would be more intuitive if you group the classes with a few subdirs: /video/balance /video/brightness ... /mpeg_audio/crc /mpeg_audio/mute ... /audio/volume /audio/bass /audio/treble .. -- Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
Hans Verkuil wrote: $ ls /sys/class/video4linux/video1/controls balance mpeg_insert_navigation_packets mpeg_video_aspect brightnessmpeg_median_chroma_filter_maximum mpeg_video_b_frames chroma_agcmpeg_median_chroma_filter_minimum mpeg_video_bitrate chroma_gain mpeg_median_filter_type mpeg_video_bitrate_mode contrast mpeg_median_luma_filter_maximum mpeg_video_encoding hue mpeg_median_luma_filter_minimum mpeg_video_gop_closure mpeg_audio_crcmpeg_spatial_chroma_filter_type mpeg_video_gop_size mpeg_audio_emphasis mpeg_spatial_filter mpeg_video_mute mpeg_audio_encoding mpeg_spatial_filter_mode mpeg_video_mute_yuv mpeg_audio_layer_ii_bitrate mpeg_spatial_luma_filter_type mpeg_video_peak_bitrate mpeg_audio_mute mpeg_stream_type mpeg_video_temporal_decimation mpeg_audio_sampling_frequency mpeg_stream_vbi_format mute mpeg_audio_stereo_modempeg_temporal_filter saturation mpeg_audio_stereo_mode_extension mpeg_temporal_filter_mode volume It would be more intuitive if you group the classes with a few subdirs: /video/balance /video/brightness ... /mpeg_audio/crc /mpeg_audio/mute ... /audio/volume /audio/bass /audio/treble 1) We don't have that information. 2) It would make a simple scheme suddenly a lot more complicated (see Andy's comments) 3) The main interface is always the application's GUI through ioctls, not sysfs. 4) Remember that ivtv has an unusually large number of controls. Most drivers will just have the usual audio and video controls, perhaps 10 at most. Strife for simplicity. I'm not sure whether we want to have this in sysfs at all. While nice there is a danger that people suddenly see it as the main API. And Markus' comment regarding permissions was a good one, I thought. I think we should just ditch this for the first implementation of the control framework. It can always be added later, but once added it is *much* harder to remove again. It's a nice proof-of-concept, though :-) Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Tue, Apr 6, 2010 at 9:44 AM, Hans Verkuil hverk...@xs4all.nl wrote: 1) We don't have that information. 2) It would make a simple scheme suddenly a lot more complicated (see Andy's comments) 3) The main interface is always the application's GUI through ioctls, not sysfs. 4) Remember that ivtv has an unusually large number of controls. Most drivers will just have the usual audio and video controls, perhaps 10 at most. Strife for simplicity. I'm not sure whether we want to have this in sysfs at all. While nice there is a danger that people suddenly see it as the main API. And Markus' comment regarding permissions was a good one, I thought. I think we should just ditch this for the first implementation of the control framework. It can always be added later, but once added it is *much* harder to remove again. It's a nice proof-of-concept, though :-) I tend to agree with Hans. We've already got *too many* interfaces that do the same thing. The testing matrix is already a nightmare - V4L1 versus V4L2, mmap() versus read(), legacy controls versus extended controls, and don't get even me started on VBI. We should be working to make drivers and interfaces simpler, with *fewer* ways of doing the same thing. The flexibility of providing yet another interface via sysfs compared to just calling v4l2-ctl just isn't worth the extra testing overhead. We've already got too much stuff that needs to be fixed and not enough good developers to warrant making the code more complicated with little tangible benefit. And nobody I've talked to who writes applications that work with V4L has been screaming OMG, if only V4L had a sysfs interface to manage controls! Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
Hans Verkuil wrote: Hans Verkuil wrote: $ ls /sys/class/video4linux/video1/controls balance mpeg_insert_navigation_packets mpeg_video_aspect brightnessmpeg_median_chroma_filter_maximum mpeg_video_b_frames chroma_agcmpeg_median_chroma_filter_minimum mpeg_video_bitrate chroma_gain mpeg_median_filter_type mpeg_video_bitrate_mode contrast mpeg_median_luma_filter_maximum mpeg_video_encoding hue mpeg_median_luma_filter_minimum mpeg_video_gop_closure mpeg_audio_crcmpeg_spatial_chroma_filter_type mpeg_video_gop_size mpeg_audio_emphasis mpeg_spatial_filter mpeg_video_mute mpeg_audio_encoding mpeg_spatial_filter_mode mpeg_video_mute_yuv mpeg_audio_layer_ii_bitrate mpeg_spatial_luma_filter_type mpeg_video_peak_bitrate mpeg_audio_mute mpeg_stream_type mpeg_video_temporal_decimation mpeg_audio_sampling_frequency mpeg_stream_vbi_format mute mpeg_audio_stereo_modempeg_temporal_filter saturation mpeg_audio_stereo_mode_extension mpeg_temporal_filter_mode volume It would be more intuitive if you group the classes with a few subdirs: /video/balance /video/brightness ... /mpeg_audio/crc /mpeg_audio/mute ... /audio/volume /audio/bass /audio/treble 1) We don't have that information. 2) It would make a simple scheme suddenly a lot more complicated (see Andy's comments) 3) The main interface is always the application's GUI through ioctls, not sysfs. 4) Remember that ivtv has an unusually large number of controls. Most drivers will just have the usual audio and video controls, perhaps 10 at most. Ok. I think we should just ditch this for the first implementation of the control framework. It can always be added later, but once added it is *much* harder to remove again. It's a nice proof-of-concept, though :-) I like the concept, especially if we can get rid of other similar sysfs interfaces that got added on a few drivers (pvrusb2 and some non-gspca drivers have it, for sure). I think I saw some of the gspca patches also touching on sysfs. Having this unified into a common interface is a bonus. -- Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
Comments below... On Mon, 5 Apr 2010, Hans Verkuil wrote: Hi all, The new control framework makes it very easy to expose controls in sysfs. The way it is implemented now in the framework is that each device node will get a 'controls' subdirectory in sysfs. Below which are all the controls associated with that device node. So different device nodes can have different controls if so desired. The name of each sysfs file is derived from the control name, basically making it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non- alphanumerical characters. Seems to work well. For numerical controls you can write numbers in decimal, octal or hexadecimal. When you write to a button control it will ignore what you wrote, but still execute the action. It looks like this for ivtv: $ ls /sys/class/video4linux/video1 controls dev device index name power subsystem uevent $ ls /sys/class/video4linux/video1/controls audio_crcchroma_gain spatial_chroma_filter_type video_bitrate_mode audio_emphasis contrast spatial_filter video_encoding audio_encoding hue spatial_filter_mode video_gop_closure audio_layer_ii_bitrate insert_navigation_packets spatial_luma_filter_typevideo_gop_size audio_mute median_chroma_filter_maximum stream_type video_mute audio_sampling_frequency median_chroma_filter_minimum stream_vbi_format video_mute_yuv audio_stereo_modemedian_filter_typetemporal_filter video_peak_bitrate audio_stereo_mode_extension median_luma_filter_maximum temporal_filter_modevideo_temporal_decimation balance median_luma_filter_minimumvideo_aspect volume brightness mute video_b_frames chroma_agc saturationvideo_bitrate The question is, is this sufficient? One of the few drivers that exposes controls in sysfs is pvrusb2. As far as I can tell from the source it will create subdirectories under the device node for each control. Those subdirs have the name ctl_control-name (e.g. ctl_volume), and below that are files exposing all the attributes of that control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val and bit_val. Most are clear, but some are a bit more obscure. enum_val is basically a QUERYMENU and returns all menu options. bit_val seems to be used for some non-control values like the TV standard that pvrusb2 also exposes and where bit_val is a bit mask of all the valid bits that can be used. Mike, if you have any additional information, just let us know. My pvrusb2 is in another country at the moment so I can't do any testing. Hans: What you see in the pvrusb2 driver is the result of an idea I had back in 2005. The pvrusb2 driver has an internal control API; my original idea back then was to then reimplement other interfaces on top of that API, in a manner that is as orthogonal as possible. The reality today is still pretty close to that concept (except for DVB unfortunately since that framework's architecture effectively has to take over the RF tuner...); the V4L2 implementation in the driver certainly works this way. The sysfs interface you see here is the result of implementing the same API through sysfs. Right now with the pvrusb2 driver the only thing not exported through sysfs is the actual streaming of video itself. The entire sysfs implementation in the driver can be found in pvrusb2-sysfs.c. Notice that the file is generic; there is not anything in it that is specific to any particular control. Rather, pvrusb2-sysfs.c is able to iterate through the driver's controls, picking up the control's name, its type, and accessors. Based on what it finds, this module then synthesizes the interface that you see in /class/pvrusb2/* - it's actually possible to add new controls to the driver without changing anything in pvrusb2-sysfs.c. Personally I think that it is overkill to basically expose the whole QUERYCTRL information to sysfs. I see it as an easy and quick way to read and modify controls via a command line. Over time, I have ended up using pretty much every control in that interface. Obviously not every control always gets touched, but I have found it extremely valuable to have such direct access to every knob in the driver this way. Also, the original concept was that the interface was to be orthogonal; in theory any kind of control action in one interface should be just as valid in another. Mike, do you know of anyone actively using that additional information? Yes. The VDR project at one time implemented a plugin to directly interface to the pvrusb2 driver in this manner. I do
Re: RFC: exposing controls in sysfs
On Tue, 6 Apr 2010, Hans Verkuil wrote: [...] One thing that might be useful is to prefix the name with the control class name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would groups them better. Or one could make a controls/user and controls/mpeg directory. That might not be such a bad idea actually. I agree with grouping in concept, and using subdirectories is not a bad thing. Probably however you'd want to ensure that in the end all the controls end up logically at the same depth in the tree. [...] An in between solution would be to add _type files. So you would have 'hue' and 'hue_type'. 'cat hue_type' would give something like: int 0 255 1 128 0x Hue In other words 'type min max step flags name'. There was I thought at some point in the past a kernel policy that sysfs controls were supposed to limit themselves to one value per node. And for menu controls like stream_type (hmm, that would become stream_type_type...) you would get: menu 0 5 1 0 Stream Type MPEG-2 Program Stream MPEG-1 System Stream MPEG-2 DVD-compatible Stream MPEG-1 VCD-compatible Stream MPEG-2 SVCD-compatible Stream Note the empty line to denote the unsupported menu item (transport stream). This would give the same information with just a single extra file. Still not sure whether it is worth it though. Just remember that the more complex / subtle you make the node contents, then the more parsing will be required for any program that tries to use it. I also think it's probably a bad idea for example to define a format where the whitespace conveys additional information. The case where I've seen whitespace as part of the syntax actually work cleanly is in Python. -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Tue, 6 Apr 2010, Devin Heitmueller wrote: [...] I tend to agree with Hans. We've already got *too many* interfaces that do the same thing. The testing matrix is already a nightmare - V4L1 versus V4L2, mmap() versus read(), legacy controls versus extended controls, and don't get even me started on VBI. We should be working to make drivers and interfaces simpler, with *fewer* ways of doing the same thing. The flexibility of providing yet another interface via sysfs compared to just calling v4l2-ctl just isn't worth the extra testing overhead. We've already got too much stuff that needs to be fixed and not enough good developers to warrant making the code more complicated with little tangible benefit. If another API (e.g. sysfs) is defined and it is specifically NOT permitted to be a complete set, then one can ultimately end up with situations where in order to effectively use a driver then multiple APIs *must* be used by the application. That's even worse. This situation already exists in the pvrusb2 driver and it's not because of sysfs - it's because of V4L and DVB. When the pvrusb2 driver is used to handle a hybrid device (such as the HVR-1950) one has to use both the DVB and V4L APIs in order to effectively operate the device. This is because both APIs provide something not available in the other. And this really sucks if all the user wants to do is stream mpeg, darn it! And I don't care if it is digital or analog. I think that situation is very wrong; given that the HVR-1950 can spit out mpeg in either mode the user shouldn't be forced to make his application choice based on which mode he wants. There's only ONE application out there that allows the user to operate an HVR-1950 without being forced to deal with this: MythTV, and that's because, well, MythTV implements both APIs: V4L and DVB. I really, really dislike situations that arise where multiple APIs are *required* to operate a device, when really there should just be one API. That said, if multiple APIs are to be exported by the driver interface, then such APIs really should be as complete as possible in order to avoid potential problems later where because of previous limiting choices of API design now multiple APIs become required. I agree that testing against multiple APIs can be a pain and a drain on effort. But that has not happened with the pvrusb2 driver. It should be possible to implement the API in a way that minimizes further thrashing due to driver changes. The pvrusb2 sysfs implementation there is programmatically created when the driver comes up. The code which implements that interface really doesn't have any logic specific to particular API functions; it is just a reflection of what is internally in the driver. If new knobs are added to the pvrusb2 driver, then the knob automatically appears in the sysfs interface. If you were to go through the change history of the pvrusb2-sysfs.c module, all you're really going to find are changes caused by the sysfs class environment itself (i.e. when struct class was morphed into struct device), not the driver or its functionality. And nobody I've talked to who writes applications that work with V4L has been screaming OMG, if only V4L had a sysfs interface to manage controls! The experience I've seen with users and the pvrusb2 interface is that once they discover the sysfs API, the response is in fact very positive. Most users of the driver had no concept that such a thing was even possible until they were exposed to it. Now that's not to say that we should all be screaming for this - but if people didn't really understand what was possible, then how could they ask for it? -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Tue, 6 Apr 2010, Markus Rechberger wrote: [...] how about security permissions? while you can easily change the permission levels for nodes in /dev you can't do this so easily with sysfs entries. I don't really think this is needed at all some applications will start to use ioctl some other apps might go for sysfs.. this makes the API a little bit whacko This is an excellent point. I should have brought this up sooner. The driver has control over the modes of the nodes in sysfs. The driver does NOT have control over the owner / group of those nodes. It is possible to change the owner / group from userspace, and I *think* it's possible to create a udev rule to do this, but honestly I have not investigated this possibility so I don't fully know. This is one serious potential drawback to using sysfs as a driver API. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Tue, 6 Apr 2010, Laurent Pinchart wrote: Hi Andy, On Tuesday 06 April 2010 13:06:18 Andy Walls wrote: On Tue, 2010-04-06 at 08:37 +0200, Hans Verkuil wrote: [snip] Again, I still don't know whether we should do this. It is dangerously seductive because it would be so trivial to implement. It's like watching ships run aground on a shallow sandbar that all the locals know about. The waters off of 'Point /sys' are full of usability shipwrecks. I don't know if it's some siren's song, the lack of a light house, or just strange currents that deceive even seasoned navigators Let the user run 'v4l2-ctl -d /dev/videoN -L' to learn about the control metatdata. It's not as easy as typing 'cat', but the user base using sysfs in an interactive shell or shell script should also know how to use v4l2-ctl. In embedded systems, the final system deployment should not need the control metadata available from sysfs in a command shell anyway. I fully agree with this. If we push the idea one step further, why do we need to expose controls in sysfs at all ? I have found it useful to have the sysfs interface within the pvrusb2 driver. If it is going to take a lot of work to specifically craft a sysfs interface that exports the V4L API, then it will probably be a pain to maintain going forward. By a lot of work I mean that each V4L API function would have to be explicitly coded for in this interface, thus as the V4L API evolves over time then extra work must be expended each time to keep the sysfs interface in step. If that is to be the case then it may not be worth it. In the pvrusb2 driver this has not been the case because the code I wrote which implements the sysfs interface for the driver does this programmatically. That is, there is nothing in the pvrusb2-sysfs.c module which is specific to a particular function. Instead, when the module initializes it is able to enumerate the API on its own and generate the appropriate interface for each control it finds. Thus as the pvrusb2 driver's implementation has evolved over time, the sysfs implementation has simply continues to do its job, automatically reflecting internal changes without any extra work in that module's code. I don't know if that same strategy could be done in the V4L core. If it could, then this would probably alleviate a lot of concerns about testing / maintenance going forward. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On 04/06/10 15:32, Mauro Carvalho Chehab wrote: Hans Verkuil wrote: Hans Verkuil wrote: $ ls /sys/class/video4linux/video1/controls balance mpeg_insert_navigation_packets mpeg_video_aspect brightnessmpeg_median_chroma_filter_maximum mpeg_video_b_frames chroma_agcmpeg_median_chroma_filter_minimum mpeg_video_bitrate chroma_gain mpeg_median_filter_type mpeg_video_bitrate_mode contrast mpeg_median_luma_filter_maximum mpeg_video_encoding hue mpeg_median_luma_filter_minimum mpeg_video_gop_closure mpeg_audio_crcmpeg_spatial_chroma_filter_type mpeg_video_gop_size mpeg_audio_emphasis mpeg_spatial_filter mpeg_video_mute mpeg_audio_encoding mpeg_spatial_filter_mode mpeg_video_mute_yuv mpeg_audio_layer_ii_bitrate mpeg_spatial_luma_filter_type mpeg_video_peak_bitrate mpeg_audio_mute mpeg_stream_type mpeg_video_temporal_decimation mpeg_audio_sampling_frequency mpeg_stream_vbi_format mute mpeg_audio_stereo_modempeg_temporal_filter saturation mpeg_audio_stereo_mode_extension mpeg_temporal_filter_mode volume It would be more intuitive if you group the classes with a few subdirs: /video/balance /video/brightness ... /mpeg_audio/crc /mpeg_audio/mute ... /audio/volume /audio/bass /audio/treble 1) We don't have that information. 2) It would make a simple scheme suddenly a lot more complicated (see Andy's comments) 3) The main interface is always the application's GUI through ioctls, not sysfs. 4) Remember that ivtv has an unusually large number of controls. Most drivers will just have the usual audio and video controls, perhaps 10 at most. Ok. I think we should just ditch this for the first implementation of the control framework. It can always be added later, but once added it is *much* harder to remove again. It's a nice proof-of-concept, though :-) I like the concept, especially if we can get rid of other similar sysfs interfaces that got added on a few drivers (pvrusb2 and some non-gspca drivers have it, for sure). I think I saw some of the gspca patches also touching on sysfs. Having this unified into a common interface is a bonus. Obviously it adds to the review burden, but perhaps we can state that the sysfs interface is only an additional option (and personally I think I'd find it pretty helpful for debugging if nothing else) and that all functionality there MUST be available through the normal routes? If any functionality only supported via sysfs is seen as a bug, then we can point it out in reviews etc. I agree with Mauro that it would be really handy to unify any interfaces that are going to turn up there anyway. Generally I'm personally in favor with the convenience of sysfs interfaces for quick and dirty debugging purposes but perhaps this isn't the time to do it here. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On 04/06/10 15:41, Mike Isely wrote: On Tue, 6 Apr 2010, Hans Verkuil wrote: [...] One thing that might be useful is to prefix the name with the control class name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would groups them better. Or one could make a controls/user and controls/mpeg directory. That might not be such a bad idea actually. I agree with grouping in concept, and using subdirectories is not a bad thing. Probably however you'd want to ensure that in the end all the controls end up logically at the same depth in the tree. [...] An in between solution would be to add _type files. So you would have 'hue' and 'hue_type'. 'cat hue_type' would give something like: int 0 255 1 128 0x Hue In other words 'type min max step flags name'. There was I thought at some point in the past a kernel policy that sysfs controls were supposed to limit themselves to one value per node. It's usually considered to be one 'conceptual' value per node, though this falls fowl of that rule too. So you could have one file with a list of possible values, or even one for say hue_range 0...255 but people are going to through a wobbly about antyhing with as much data in it as above. The debate on this was actually pretty well covered in an lwn article the other week. http://lwn.net/Articles/378884/ So the above hue type would probably need: hue_type (int) hue_range (0...255) hue_step (1) hue_flags (128) hue_name (Hue) Of those, hue_name doesn't in this case tell us anything and hue_step could be suppressed as an obvious default. It could be argued that parts of the above could be considered a single 'conceptual' value but I don't think the whole can be. The reasoning behind this (and it is definitely true with your above example) is that sysfs should be human readable without needing to reach for the documentation. And for menu controls like stream_type (hmm, that would become stream_type_type...) you would get: menu 0 5 1 0 Stream Type MPEG-2 Program Stream MPEG-1 System Stream MPEG-2 DVD-compatible Stream MPEG-1 VCD-compatible Stream MPEG-2 SVCD-compatible Stream Note the empty line to denote the unsupported menu item (transport stream). This would give the same information with just a single extra file. Still not sure whether it is worth it though. Just remember that the more complex / subtle you make the node contents, then the more parsing will be required for any program that tries to use it. I also think it's probably a bad idea for example to define a format where the whitespace conveys additional information. The case where I've seen whitespace as part of the syntax actually work cleanly is in Python. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On 6 April 2010 18:06, Jonathan Cameron ji...@cam.ac.uk wrote: On 04/06/10 15:32, Mauro Carvalho Chehab wrote: Hans Verkuil wrote: Hans Verkuil wrote: $ ls /sys/class/video4linux/video1/controls balance mpeg_insert_navigation_packets mpeg_video_aspect brightness mpeg_median_chroma_filter_maximum mpeg_video_b_frames chroma_agc mpeg_median_chroma_filter_minimum mpeg_video_bitrate chroma_gain mpeg_median_filter_type mpeg_video_bitrate_mode contrast mpeg_median_luma_filter_maximum mpeg_video_encoding hue mpeg_median_luma_filter_minimum mpeg_video_gop_closure mpeg_audio_crc mpeg_spatial_chroma_filter_type mpeg_video_gop_size mpeg_audio_emphasis mpeg_spatial_filter mpeg_video_mute mpeg_audio_encoding mpeg_spatial_filter_mode mpeg_video_mute_yuv mpeg_audio_layer_ii_bitrate mpeg_spatial_luma_filter_type mpeg_video_peak_bitrate mpeg_audio_mute mpeg_stream_type mpeg_video_temporal_decimation mpeg_audio_sampling_frequency mpeg_stream_vbi_format mute mpeg_audio_stereo_mode mpeg_temporal_filter saturation mpeg_audio_stereo_mode_extension mpeg_temporal_filter_mode volume It would be more intuitive if you group the classes with a few subdirs: /video/balance /video/brightness ... /mpeg_audio/crc /mpeg_audio/mute ... /audio/volume /audio/bass /audio/treble 1) We don't have that information. 2) It would make a simple scheme suddenly a lot more complicated (see Andy's comments) 3) The main interface is always the application's GUI through ioctls, not sysfs. 4) Remember that ivtv has an unusually large number of controls. Most drivers will just have the usual audio and video controls, perhaps 10 at most. Ok. I think we should just ditch this for the first implementation of the control framework. It can always be added later, but once added it is *much* harder to remove again. It's a nice proof-of-concept, though :-) I like the concept, especially if we can get rid of other similar sysfs interfaces that got added on a few drivers (pvrusb2 and some non-gspca drivers have it, for sure). I think I saw some of the gspca patches also touching on sysfs. Having this unified into a common interface is a bonus. Obviously it adds to the review burden, but perhaps we can state that the sysfs interface is only an additional option (and personally I think I'd find it pretty helpful for debugging if nothing else) and that all functionality there MUST be available through the normal routes? If any functionality only supported via sysfs is seen as a bug, then we can point it out in reviews etc. I agree with Mauro that it would be really handy to unify any interfaces that are going to turn up there anyway. Generally I'm personally in favor with the convenience of sysfs interfaces for quick and dirty debugging purposes but perhaps this isn't the time to do it here. Hi all, I'm a newbie but I have to ask: how about using debugfs instead of sysfs? Then everyone will know that the interface is for debugging only and not production code :-) Best regards, Bjørn Forsman -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: exposing controls in sysfs
On Monday 05 April 2010 23:47:10 Hans Verkuil wrote: Hi all, The new control framework makes it very easy to expose controls in sysfs. The way it is implemented now in the framework is that each device node will get a 'controls' subdirectory in sysfs. Below which are all the controls associated with that device node. So different device nodes can have different controls if so desired. The name of each sysfs file is derived from the control name, basically making it lowercase, replacing ' ', '-' and '_' with '_' and skipping any other non- alphanumerical characters. Seems to work well. For numerical controls you can write numbers in decimal, octal or hexadecimal. When you write to a button control it will ignore what you wrote, but still execute the action. It looks like this for ivtv: $ ls /sys/class/video4linux/video1 controls dev device index name power subsystem uevent $ ls /sys/class/video4linux/video1/controls audio_crcchroma_gain spatial_chroma_filter_type video_bitrate_mode audio_emphasis contrast spatial_filter video_encoding audio_encoding hue spatial_filter_mode video_gop_closure audio_layer_ii_bitrate insert_navigation_packets spatial_luma_filter_typevideo_gop_size audio_mute median_chroma_filter_maximum stream_type video_mute audio_sampling_frequency median_chroma_filter_minimum stream_vbi_format video_mute_yuv audio_stereo_modemedian_filter_typetemporal_filter video_peak_bitrate audio_stereo_mode_extension median_luma_filter_maximum temporal_filter_modevideo_temporal_decimation balance median_luma_filter_minimumvideo_aspect volume brightness mute video_b_frames chroma_agc saturationvideo_bitrate The question is, is this sufficient? One thing that might be useful is to prefix the name with the control class name. E.g. hue becomes user_hue and audio_crc becomes mpeg_audio_crc. It would groups them better. Or one could make a controls/user and controls/mpeg directory. That might not be such a bad idea actually. One of the few drivers that exposes controls in sysfs is pvrusb2. As far as I can tell from the source it will create subdirectories under the device node for each control. Those subdirs have the name ctl_control-name (e.g. ctl_volume), and below that are files exposing all the attributes of that control: name, type, min_val, max_val, def_val, cur_val, custom_val, enum_val and bit_val. Most are clear, but some are a bit more obscure. enum_val is basically a QUERYMENU and returns all menu options. bit_val seems to be used for some non-control values like the TV standard that pvrusb2 also exposes and where bit_val is a bit mask of all the valid bits that can be used. Mike, if you have any additional information, just let us know. My pvrusb2 is in another country at the moment so I can't do any testing. Personally I think that it is overkill to basically expose the whole QUERYCTRL information to sysfs. I see it as an easy and quick way to read and modify controls via a command line. An in between solution would be to add _type files. So you would have 'hue' and 'hue_type'. 'cat hue_type' would give something like: int 0 255 1 128 0x Hue In other words 'type min max step flags name'. And for menu controls like stream_type (hmm, that would become stream_type_type...) you would get: menu 0 5 1 0 Stream Type MPEG-2 Program Stream MPEG-1 System Stream MPEG-2 DVD-compatible Stream MPEG-1 VCD-compatible Stream MPEG-2 SVCD-compatible Stream Note the empty line to denote the unsupported menu item (transport stream). This would give the same information with just a single extra file. Still not sure whether it is worth it though. Regards, Hans Mike, do you know of anyone actively using that additional information? And which non-control values do you at the moment expose in pvrusb2 through sysfs? Can you perhaps give an ls -R of all the files you create in sysfs for pvrusb2? I am wondering whether some of those should get a place in the framework as well. While I don't think e.g. cropping parameters are useful, things like the current input or tuner frequency can be handy. However, for those to be useful they would have to be wired up internally through the framework. For example, VIDIOC_S_FREQUENCY would have to be hooked up internally to a control. That would ensure that however you access it (ioctl or sysfs) it will both end up in the same s_ctrl function. It will be nice to hear from you what your experience is. Comments? Ideas? Once we commit to something it is there for a long time to come since