[FFmpeg-devel] [PATCH] add dumpwave filter

[Moritz Barsnick]

> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -680,13 +680,13 @@ select RIAA.
>  @item cd
>  select Compact Disc (CD).
>  @item 50fm
> -select 50??s (FM).
> +select 50??s (FM).
>  @item 75fm
> -select 75??s (FM).
> +select 75??s (FM).
>  @item 50kf
> -select 50??s (FM-KF).
> +select 50??s (FM-KF).
>  @item 75kf
> -select 75??s (FM-KF).
> +select 75??s (FM-KF).
>  @end table
>  @end table

Please review your own patches carefully. As you can see here, your
editor is changing other sections due to some charset options or so.
Please make sure that doesn't happen.

> +The default value is @var{1800}

You probably don't need to add markup to "1800" (and is it really a
@val? not @code?), especially:

> +Samples count per value per channel, default 128

as you didn't do that here either.

> + at item f, file
> +Path to a file ``-'' is a shorthand
> +for standard output.

There needs to be a comma or perios after "Path to a file".
Furthermore, you don't need to wrap your lines so short.


>  more details but also more artifacts, while higher values make the image smoother
> -but also blurrier. Default value is @code{0} ??? PSNR optimal.
> +but also blurrier. Default value is @code{0} ??? PSNR optimal.

Your editor also changed this.

> +static const AVOption dumpwave_options[] = {
> +    { "w", "set number of data values",  OFFSET(width), AV_OPT_TYPE_INT,  {.i64 = 1800}, 1, INT_MAX, FLAGS },
> +    { "width", "set number of data values",  OFFSET(width), AV_OPT_TYPE_INT,  {.i64 = 1800}, 1, INT_MAX, FLAGS },
> +    { "n", "set number of samples per value per channel",  OFFSET(nb_samples), AV_OPT_TYPE_INT64,  {.i64 = 128}, 1, INT64_MAX, FLAGS },
> +    { "nb_samples", "set number of samples per value per channel",  OFFSET(nb_samples), AV_OPT_TYPE_INT64,  {.i64 = 128}, 1, INT64_MAX, FLAGS },
> +    { "f", "set dump file", OFFSET(filename), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, FLAGS },
> +    { "file", "set dump file", OFFSET(filename), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, FLAGS },

Cosmetic: I suggest aligning these with some whitespace, which makes it
easier to recognize the duplicated options.

> +    dumpwave->values = av_malloc(dumpwave->width * sizeof(float));
> +    if (!dumpwave->values)
> +        return AVERROR(ENOMEM);
> +    memset(dumpwave->values, 0, dumpwave->width * sizeof(float));

av_mallocz()?

> +static inline void RMS(DumpWaveContext *dumpwave, const float sample)
                      ^^^ I believe capitals are not desired here (but may be wrong)

> +    if (sample != 0)

0.f

Moritz