On Tue, 2017-03-07 at 13:08 +0100, Andrzej Hajda wrote: > On 03.03.2017 10:07, Smitha T Murthy wrote: > > Added V4l2 controls for HEVC encoder > > It should be rather "Document controls for HEVC encoder" or sth similar. > > In general most of comments are in previous patch. > Few additional comments: > - please be careful about control names - they are exported to userspace > and becomes ABI, so it will be difficult to change them later (this > comment is rather to previous patch), > - please provide good documentation as for most users this documentation > will be the only available source of information, > - in short: bugs in the driver can be easily fixed(usually), wrong > control names will be hard to fix, weak documentation will prevent using it. > > And regarding this patch: > - please expand all acronyms (pb, tmv, BIT,...), > - please consider using menu instead of numbers for profile, level, > tier, types, generally everywhere where control value enumerates > 'things' and is not a pure number (coefficient, counter,...), > - if control is per-frame please drop it, V4L2 does not support it at > the moment ( I suppose ), > > Regards > Andrzej > > Ok I will change the patch description. I will try to document each control more elaborately and check the control names again. I do understand your concern regarding the wrong documentation, I will try to make more understandable and helpful. I will expand the macro names in the next version. I will create a menu for controls where it is applicable. Thank you so much for your review. Regards, Smitha >
