Re: [Patch v2 11/11] Documention: v4l: Documentation for HEVC CIDs
From: Smitha T Murthy
Date: Tue Mar 14 2017 - 07:40:22 EST
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
>