[FFmpeg-devel] [PATCH] FFV1 specification: Reduce redundancy in the description of xxPlane() and xxLine()

Michael Niedermayer michaelni at gmx.at
Sun May 3 13:55:07 CEST 2015


On Sun, May 03, 2015 at 12:31:05PM +0200, Jerome Martinez wrote:
> Le 03/05/2015 04:57, Michael Niedermayer a écrit :
> >i dont think its a good idea to replace a named variable in a
> >for () statement by a construct so long that it needs a linebreak
> >in the text output [...]
> 
> I hesitated and this is a very good point, so I replaced by 2 named
> variables:

> - plane_count which is the count of planes

thats a bad name for packed formats


> - quant_table_index_count which is the count of quant_table indexes

any reason not to call it quant_table_count ? (its shorter and seems
not to contain less information)


[...]
> >
> >also a more critical problem is that this patch makes the spec
> >less well defined.
> 
> It does not.
> As written in the patch description:
> "the order of planes is already defined in the General section and
> has no impact on the bitstream."

please keep the line length of commit messages below 80
but yes ive missed that part of the commit message


[...]

> It is not good to have redundancy in a specification.

Its more important that a spec is readable than being non redundant
its after all a english text that has to be understood by a human

but of course you have a point by arguing that the luma/cb/cr
function names are "limiting" and we should indeed chnage that as
you suggested


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150503/956c4889/attachment.asc>


More information about the ffmpeg-devel mailing list