[FFmpeg-devel] [PATCH] add dumpwave filter

Dmytro Humeniuk dmitry.gumenyuk at gmail.com
Thu Jan 18 18:23:55 EET 2018


> On 18 Jan 2018, at 00:13, Moritz Barsnick <barsnick at 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.
> 
>> + 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 },
> 
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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avfilter-add-dumpwave-filter.patch
Type: application/octet-stream
Size: 68968 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180118/92f9aeb5/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 3872 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180118/92f9aeb5/attachment.bin>


More information about the ffmpeg-devel mailing list