Re: [Patch v2 11/11] Documention: v4l: Documentation for HEVC CIDs

From: Andrzej Hajda
Date: Tue Mar 07 2017 - 07:20:34 EST



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