Re: RFC: exposing controls in sysfs

[Jonathan Cameron <jic23@cam.ac.uk>]

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.