On date Thursday 2024-06-13 16:45:48 +0100, Andrew Sayers wrote:
> Some documentation nitpicks.  Nothing jumped out about the code, but I don't
> know the algorithm well enough to spot anything deep.
> 
> > From 9932cfc19500acbd0685eb2cc8fd88e9af3f5dbd Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab@gmail.com>
> > Date: Mon, 27 May 2024 11:19:08 +0200
> > Subject: [PATCH] lavfi: add Perlin noise generator
> > 
> > ---
> >  Changelog                 |   1 +
> >  doc/filters.texi          | 100 +++++++++++++++++
> >  libavfilter/Makefile      |   1 +
> >  libavfilter/allfilters.c  |   1 +
> >  libavfilter/perlin.c      | 224 ++++++++++++++++++++++++++++++++++++++
> >  libavfilter/perlin.h      | 101 +++++++++++++++++
> >  libavfilter/vsrc_perlin.c | 169 ++++++++++++++++++++++++++++
> >  7 files changed, 597 insertions(+)
> >  create mode 100644 libavfilter/perlin.c
> >  create mode 100644 libavfilter/perlin.h
> >  create mode 100644 libavfilter/vsrc_perlin.c
[...] 
> > +@item tscale
> > +Define a scale factor used to multiple the time coordinate. This can
> > +be useful to change the time variation speed.
> > +
> > +@item random_mode
> > +Set random mode used to compute initial pattern.
> > +
> > +Supported values are:
> > +@table @option
> > +@item random
> > +Compute and use random seed.
> > +
> > +@item ken
> > +Use the predefined initial pattern defined by Ken Perlin in the
> > +original article, can be useful to compare the output with other
> > +sources.
> > +
> > +@item seed
> > +Use the value specified by @option{random_seed} option.
> > +@end table
> 
> Nit: "Define a...", "Use the..." etc. is redundant - remove them to
> optimise for reading time.

kept current form since the following is a complete sentence

> > +@item
> > +Chain Perlin noise with the @ref{lutyuv} to generate a black&white
> > +effect:
> > +@example
> > +perlin:octaves=7:tscale=0.3,lutyuv=y='if(lt(val\,128)\,255\,0)'
> > +@end example
> > +
> > +@item
> > +Stretch noise along the y axis, and convert gray level to red-only
> 

> I initially thought this was a typo for "read-only" - maybe s/red/blue/?

I prefer to keep red, this is a fire approximation

[...]
> 
> > +    { "random_mode", "set random mode used to compute initial pattern", OFFSET(random_mode), AV_OPT_TYPE_INT, {.i64=FF_PERLIN_RANDOM_MODE_RANDOM}, 0, FF_PERLIN_RANDOM_MODE_NB-1, FLAGS, .unit = "random_mode" },
> > +    { "random", "compute and use random seed", 0, AV_OPT_TYPE_CONST, {.i64=FF_PERLIN_RANDOM_MODE_RANDOM},   0, 0, FLAGS, .unit = "random_mode" },
> > +    { "ken", "use the predefined initial pattern defined by Ken Perlin in the original article", 0, AV_OPT_TYPE_CONST, {.i64=FF_PERLIN_RANDOM_MODE_KEN}, 0, 0, FLAGS, .unit = "random_mode" },
> > +    { "seed", "use the value specified by random_seed", 0, AV_OPT_TYPE_CONST, {.i64=FF_PERLIN_RANDOM_MODE_SEED}, 0, 0, FLAGS, .unit = "random_mode" },
> > +
> > +    { "random_seed", "set the seed for filling the initial grid randomly", OFFSET(random_seed), AV_OPT_TYPE_UINT, {.i64=0}, 0, UINT_MAX, FLAGS },
> > +    { "seed",        "set the seed for filling the initial grid randomly", OFFSET(random_seed), AV_OPT_TYPE_UINT, {.i64=0}, 0, UINT_MAX, FLAGS },
> 
> Nit: "set the seed for filling the initial grid randomly" is ambiguous.
> Do you mean:
> 
> a) Set the seed for the RNG to a specified value (value is the seed number)
> b) Pick a random number to fill the grid with (value is a boolean yes/no)
> 
> I settled on the former after several reads, but would appreciate clearer wording.

Changed to:
set the seed for filling the initial pattern

[...]

Applied other fixes as well, thanks.