> On 18 Jan 2018, at 00:13, Moritz Barsnick <barsn...@gmx.net> wrote:
>
>> --- 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.
>
Sorry, I will pay more attention reviewing patches
>> +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.
>
>> +@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 },
>
I’m not sure what is maximum line length limit
> 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
Thanks
>
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
0001-avfilter-add-dumpwave-filter.patch
Description: Binary data
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
