Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Tobias Rapp]

On 20.01.2018 20:17, Dmytro Humeniuk wrote:



On 18 Jan 2018, at 17:32, Dmytro Humeniuk  wrote:



On 18 Jan 2018, at 08:56, Tobias Rapp  wrote:

On 15.01.2018 13:48, Dmytro Humeniuk wrote:

On 15 Jan 2018, at 09:14, Tobias Rapp  wrote:

On 13.01.2018 23:52, Дмитрий Гуменюк wrote:

Hi,

On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  wrote:

Hi


On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  wrote:


On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:

On 12.01.2018 12:16, Дмитрий Гуменюк wrote:

Hi

On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,
For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.


I agree. If the BWF Peak Envelope output which was suggested in the other 
thread does not match your demands and filter implementation is actually 
necessary I would prefer if the filter would attach the RMS value(s) as frame 
metadata instead of directly dumping to file. Frame metadata can then be re-

RMS values may be counted for several frames or only for a half of a frame

used by other filters or dumped into file by using the existing "ametadata" 
filter.

This would be similar to:

ffmpeg -i input-file -f null -filter:a 
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
/dev/null

I like this idea, but won’t asetnsamples affect performance by creating fifo 
queue? And it may require some effort to parse long output


I added asetnsamples to define the audio frame size (interval of values from astats). You 
can reduce the number of lines printed by ametadata by using the 
"key=lavfi.astats.foo" option.

I used asetnsamples as well, and I measured performance while transcoding - it 
appears to be slight slower

I think output is now more generic and I got rid of long switch/case, thanks 
for support

Here is most recent patch, seems like all comments are addressed, did I miss 
something?


I still would prefer to have the value attached as frame metadata, then dumped into file via the 
existing "ametadata" filter. Even better would be to integrate the statistic value (if 
missing) into the "astats" filter.

If your concern is the output format of "ametadata" then some output format 
extension (CSV/JSON) needs to be discussed for ametadata/metadata.

If your concern is performance then please add some numbers. In my tests using an approx. 
5 minutes input WAV file (48kHz, stereo) the run with "asetnsamples" was 
considerably faster than the run without (1.7s vs. 13.9s)

Hi
As I mentioned previously adding metadata to each frame is not possible
as value may be counted for several frames or only for a half of a frame
I used 2 hours long 48kHz mp3 
https://s3-eu-west-1.amazonaws.com/balamii/SynthSystemSystersJAN2018.mp3
For this purposes I set up CentOS AWS EC2 nano instance
Then I transcoded it while filtering like following (just to recreate real 
situation):
1. -filter:a 
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat" 
out.mp3
2. -filter:a "dumpwave=n=192197:f=-" out.mp3
Results:
1. 244810550046 nanoseconds
2. 87494286740 nanoseconds
One of the possible use cases - to set up 2 chains of asetnsamples->metadata - 
for example:
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat,asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file1.dat”
 for sure it will affect performance
Comparing with "dumpwave=n=192197:f=out1,dumpwave=n= 22050:f=out2"


Sorry, I misunderstood your concerns regarding asetnsamples filter performance. 
The numbers I provided have been for

"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"

versus

"astats=metadata=on,ametadata=print:file=stats-file.dat"

When comparing astats+ametadata versus dumpwave it is obvious that a 
specialized filter which only calculates one statistic value is faster than a 
filter that calculates multiple statistics. But still my opinion is that if the 
dumpwave filter is to be added to the codebase it should be more generic (i.e. 
output frame metadata similar to the psnr/ssim filters for video).


Actually current output(normalised float values in range 0...1) was proposed by 
Kyle as more generic.

Ping


What I wrote is my personal opinion. I acknowledge that you have put 
good efforts in implementing the patch and even added FATE tests -- so 
my words must sound disappointing to you. Rest assured that almost all 
non-trivial patches need multiple iterations.


From my side improving the existing astats+ametadata code would be the 
preferred way to continue. If that is 

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Dmytro Humeniuk]

> On 18 Jan 2018, at 17:32, Dmytro Humeniuk  wrote:
> 
>> 
>> On 18 Jan 2018, at 08:56, Tobias Rapp  wrote:
>> 
>> On 15.01.2018 13:48, Dmytro Humeniuk wrote:
 On 15 Jan 2018, at 09:14, Tobias Rapp  wrote:
 
 On 13.01.2018 23:52, Дмитрий Гуменюк wrote:
> Hi,
>> On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  
>> wrote:
>> 
>> Hi
>> 
>>> On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  
>>> wrote:
>>> 
 On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:
 
 On 12.01.2018 12:16, Дмитрий Гуменюк wrote:
> Hi
>> On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:
>> 
>> On 10.01.2018 18:18, Kyle Swanson wrote:
>>> Hi,
>>> For this to be a part of libavfilter the output needs to be more 
>>> generic
>>> than the just the Soundcloud format. If we want this to be 
>>> generally useful
>>> it should probably just output an array of floats between 0.0 and 
>>> 1.0. The
>>> consumer of this data (JS library, or whatever) can use this in 
>>> whatever
>>> way it wants.
>> 
>> I agree. If the BWF Peak Envelope output which was suggested in the 
>> other thread does not match your demands and filter implementation 
>> is actually necessary I would prefer if the filter would attach the 
>> RMS value(s) as frame metadata instead of directly dumping to file. 
>> Frame metadata can then be re-
> RMS values may be counted for several frames or only for a half of a 
> frame
>> used by other filters or dumped into file by using the existing 
>> "ametadata" filter.
>> 
>> This would be similar to:
>> 
>> ffmpeg -i input-file -f null -filter:a 
>> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
>>  /dev/null
> I like this idea, but won’t asetnsamples affect performance by 
> creating fifo queue? And it may require some effort to parse long 
> output
 
 I added asetnsamples to define the audio frame size (interval of 
 values from astats). You can reduce the number of lines printed by 
 ametadata by using the "key=lavfi.astats.foo" option.
>>> I used asetnsamples as well, and I measured performance while 
>>> transcoding - it appears to be slight slower
>> I think output is now more generic and I got rid of long switch/case, 
>> thanks for support
> Here is most recent patch, seems like all comments are addressed, did I 
> miss something?
 
 I still would prefer to have the value attached as frame metadata, then 
 dumped into file via the existing "ametadata" filter. Even better would be 
 to integrate the statistic value (if missing) into the "astats" filter.
 
 If your concern is the output format of "ametadata" then some output 
 format extension (CSV/JSON) needs to be discussed for ametadata/metadata.
 
 If your concern is performance then please add some numbers. In my tests 
 using an approx. 5 minutes input WAV file (48kHz, stereo) the run with 
 "asetnsamples" was considerably faster than the run without (1.7s vs. 
 13.9s)
>>> Hi
>>> As I mentioned previously adding metadata to each frame is not possible
>>> as value may be counted for several frames or only for a half of a frame
>>> I used 2 hours long 48kHz mp3 
>>> https://s3-eu-west-1.amazonaws.com/balamii/SynthSystemSystersJAN2018.mp3
>>> For this purposes I set up CentOS AWS EC2 nano instance
>>> Then I transcoded it while filtering like following (just to recreate real 
>>> situation):
>>> 1. -filter:a 
>>> "asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat"
>>>  out.mp3
>>> 2. -filter:a "dumpwave=n=192197:f=-" out.mp3
>>> Results:
>>> 1. 244810550046 nanoseconds
>>> 2. 87494286740 nanoseconds
>>> One of the possible use cases - to set up 2 chains of 
>>> asetnsamples->metadata - for example:
>>> "asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat,asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file1.dat”
>>>  for sure it will affect performance
>>> Comparing with "dumpwave=n=192197:f=out1,dumpwave=n= 22050:f=out2"
>> 
>> Sorry, I misunderstood your concerns regarding asetnsamples filter 
>> performance. The numbers I provided have been for
>> 
>> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
>> 
>> versus
>> 
>> "astats=metadata=on,ametadata=print:file=stats-file.dat"
>> 
>> When comparing astats+ametadata versus dumpwave it is obvious that a 
>> specialized filter which only calculates one statistic value is faster than 
>> a filter that calculates multiple 

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Dmytro Humeniuk]

> On 18 Jan 2018, at 08:56, Tobias Rapp  wrote:
> 
> On 15.01.2018 13:48, Dmytro Humeniuk wrote:
>>> On 15 Jan 2018, at 09:14, Tobias Rapp  wrote:
>>> 
>>> On 13.01.2018 23:52, Дмитрий Гуменюк wrote:
 Hi,
> On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  
> wrote:
> 
> Hi
> 
>> On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  
>> wrote:
>> 
>>> On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:
>>> 
>>> On 12.01.2018 12:16, Дмитрий Гуменюк wrote:
 Hi
> On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:
> 
> On 10.01.2018 18:18, Kyle Swanson wrote:
>> Hi,
>> For this to be a part of libavfilter the output needs to be more 
>> generic
>> than the just the Soundcloud format. If we want this to be generally 
>> useful
>> it should probably just output an array of floats between 0.0 and 
>> 1.0. The
>> consumer of this data (JS library, or whatever) can use this in 
>> whatever
>> way it wants.
> 
> I agree. If the BWF Peak Envelope output which was suggested in the 
> other thread does not match your demands and filter implementation is 
> actually necessary I would prefer if the filter would attach the RMS 
> value(s) as frame metadata instead of directly dumping to file. Frame 
> metadata can then be re-
 RMS values may be counted for several frames or only for a half of a 
 frame
> used by other filters or dumped into file by using the existing 
> "ametadata" filter.
> 
> This would be similar to:
> 
> ffmpeg -i input-file -f null -filter:a 
> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
>  /dev/null
 I like this idea, but won’t asetnsamples affect performance by 
 creating fifo queue? And it may require some effort to parse long 
 output
>>> 
>>> I added asetnsamples to define the audio frame size (interval of values 
>>> from astats). You can reduce the number of lines printed by ametadata 
>>> by using the "key=lavfi.astats.foo" option.
>> I used asetnsamples as well, and I measured performance while 
>> transcoding - it appears to be slight slower
> I think output is now more generic and I got rid of long switch/case, 
> thanks for support
 Here is most recent patch, seems like all comments are addressed, did I 
 miss something?
>>> 
>>> I still would prefer to have the value attached as frame metadata, then 
>>> dumped into file via the existing "ametadata" filter. Even better would be 
>>> to integrate the statistic value (if missing) into the "astats" filter.
>>> 
>>> If your concern is the output format of "ametadata" then some output format 
>>> extension (CSV/JSON) needs to be discussed for ametadata/metadata.
>>> 
>>> If your concern is performance then please add some numbers. In my tests 
>>> using an approx. 5 minutes input WAV file (48kHz, stereo) the run with 
>>> "asetnsamples" was considerably faster than the run without (1.7s vs. 13.9s)
>> Hi
>> As I mentioned previously adding metadata to each frame is not possible
>> as value may be counted for several frames or only for a half of a frame
>> I used 2 hours long 48kHz mp3 
>> https://s3-eu-west-1.amazonaws.com/balamii/SynthSystemSystersJAN2018.mp3
>> For this purposes I set up CentOS AWS EC2 nano instance
>> Then I transcoded it while filtering like following (just to recreate real 
>> situation):
>> 1. -filter:a 
>> "asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat" 
>> out.mp3
>> 2. -filter:a "dumpwave=n=192197:f=-" out.mp3
>> Results:
>> 1. 244810550046 nanoseconds
>> 2. 87494286740 nanoseconds
>> One of the possible use cases - to set up 2 chains of asetnsamples->metadata 
>> - for example:
>> "asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat,asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file1.dat”
>>  for sure it will affect performance
>> Comparing with "dumpwave=n=192197:f=out1,dumpwave=n= 22050:f=out2"
> 
> Sorry, I misunderstood your concerns regarding asetnsamples filter 
> performance. The numbers I provided have been for
> 
> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
> 
> versus
> 
> "astats=metadata=on,ametadata=print:file=stats-file.dat"
> 
> When comparing astats+ametadata versus dumpwave it is obvious that a 
> specialized filter which only calculates one statistic value is faster than a 
> filter that calculates multiple statistics. But still my opinion is that if 
> the dumpwave filter is to be added to the codebase it should be more generic 
> (i.e. output frame metadata similar to the psnr/ssim filters for 

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Dmytro Humeniuk]

> On 18 Jan 2018, at 00:13, Moritz Barsnick  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

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Tobias Rapp]

On 15.01.2018 13:48, Dmytro Humeniuk wrote:



On 15 Jan 2018, at 09:14, Tobias Rapp  wrote:

On 13.01.2018 23:52, Дмитрий Гуменюк wrote:

Hi,

On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  wrote:

Hi


On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  wrote:


On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:

On 12.01.2018 12:16, Дмитрий Гуменюк wrote:

Hi

On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,
For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.


I agree. If the BWF Peak Envelope output which was suggested in the other 
thread does not match your demands and filter implementation is actually 
necessary I would prefer if the filter would attach the RMS value(s) as frame 
metadata instead of directly dumping to file. Frame metadata can then be re-

RMS values may be counted for several frames or only for a half of a frame

used by other filters or dumped into file by using the existing "ametadata" 
filter.

This would be similar to:

ffmpeg -i input-file -f null -filter:a 
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
/dev/null

I like this idea, but won’t asetnsamples affect performance by creating fifo 
queue? And it may require some effort to parse long output


I added asetnsamples to define the audio frame size (interval of values from astats). You 
can reduce the number of lines printed by ametadata by using the 
"key=lavfi.astats.foo" option.

I used asetnsamples as well, and I measured performance while transcoding - it 
appears to be slight slower

I think output is now more generic and I got rid of long switch/case, thanks 
for support

Here is most recent patch, seems like all comments are addressed, did I miss 
something?


I still would prefer to have the value attached as frame metadata, then dumped into file via the 
existing "ametadata" filter. Even better would be to integrate the statistic value (if 
missing) into the "astats" filter.

If your concern is the output format of "ametadata" then some output format 
extension (CSV/JSON) needs to be discussed for ametadata/metadata.

If your concern is performance then please add some numbers. In my tests using an approx. 
5 minutes input WAV file (48kHz, stereo) the run with "asetnsamples" was 
considerably faster than the run without (1.7s vs. 13.9s)

Hi
As I mentioned previously adding metadata to each frame is not possible
as value may be counted for several frames or only for a half of a frame

I used 2 hours long 48kHz mp3 
https://s3-eu-west-1.amazonaws.com/balamii/SynthSystemSystersJAN2018.mp3
For this purposes I set up CentOS AWS EC2 nano instance
Then I transcoded it while filtering like following (just to recreate real 
situation):
1. -filter:a 
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat" 
out.mp3
2. -filter:a "dumpwave=n=192197:f=-" out.mp3
Results:
1. 244810550046 nanoseconds
2. 87494286740 nanoseconds

One of the possible use cases - to set up 2 chains of asetnsamples->metadata - 
for example:
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat,asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file1.dat”
 for sure it will affect performance
Comparing with "dumpwave=n=192197:f=out1,dumpwave=n= 22050:f=out2"


Sorry, I misunderstood your concerns regarding asetnsamples filter 
performance. The numbers I provided have been for


"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"

versus

"astats=metadata=on,ametadata=print:file=stats-file.dat"

When comparing astats+ametadata versus dumpwave it is obvious that a 
specialized filter which only calculates one statistic value is faster 
than a filter that calculates multiple statistics. But still my opinion 
is that if the dumpwave filter is to be added to the codebase it should 
be more generic (i.e. output frame metadata similar to the psnr/ssim 
filters for video).


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [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.

> +@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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Dmytro Humeniuk]

> On 15 Jan 2018, at 13:48, Dmytro Humeniuk  wrote:
> 
>> 
>> On 15 Jan 2018, at 09:14, Tobias Rapp  wrote:
>> 
>> On 13.01.2018 23:52, Дмитрий Гуменюк wrote:
>>> Hi,
 On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  
 wrote:
 
 Hi
 
> On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  
> wrote:
> 
>> On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:
>> 
>> On 12.01.2018 12:16, Дмитрий Гуменюк wrote:
>>> Hi
 On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:
 
 On 10.01.2018 18:18, Kyle Swanson wrote:
> Hi,
> For this to be a part of libavfilter the output needs to be more 
> generic
> than the just the Soundcloud format. If we want this to be generally 
> useful
> it should probably just output an array of floats between 0.0 and 
> 1.0. The
> consumer of this data (JS library, or whatever) can use this in 
> whatever
> way it wants.
 
 I agree. If the BWF Peak Envelope output which was suggested in the 
 other thread does not match your demands and filter implementation is 
 actually necessary I would prefer if the filter would attach the RMS 
 value(s) as frame metadata instead of directly dumping to file. Frame 
 metadata can then be re-
>>> RMS values may be counted for several frames or only for a half of a 
>>> frame
 used by other filters or dumped into file by using the existing 
 "ametadata" filter.
 
 This would be similar to:
 
 ffmpeg -i input-file -f null -filter:a 
 "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
  /dev/null
>>> I like this idea, but won’t asetnsamples affect performance by creating 
>>> fifo queue? And it may require some effort to parse long output
>> 
>> I added asetnsamples to define the audio frame size (interval of values 
>> from astats). You can reduce the number of lines printed by ametadata by 
>> using the "key=lavfi.astats.foo" option.
> I used asetnsamples as well, and I measured performance while transcoding 
> - it appears to be slight slower
 I think output is now more generic and I got rid of long switch/case, 
 thanks for support
>>> Here is most recent patch, seems like all comments are addressed, did I 
>>> miss something?
>> 
>> I still would prefer to have the value attached as frame metadata, then 
>> dumped into file via the existing "ametadata" filter. Even better would be 
>> to integrate the statistic value (if missing) into the "astats" filter.
>> 
>> If your concern is the output format of "ametadata" then some output format 
>> extension (CSV/JSON) needs to be discussed for ametadata/metadata.
>> 
>> If your concern is performance then please add some numbers. In my tests 
>> using an approx. 5 minutes input WAV file (48kHz, stereo) the run with 
>> "asetnsamples" was considerably faster than the run without (1.7s vs. 13.9s)
> Hi
> As I mentioned previously adding metadata to each frame is not possible
> as value may be counted for several frames or only for a half of a frame 
> 
> I used 2 hours long 48kHz mp3 
> https://s3-eu-west-1.amazonaws.com/balamii/SynthSystemSystersJAN2018.mp3
> For this purposes I set up CentOS AWS EC2 nano instance
> Then I transcoded it while filtering like following (just to recreate real 
> situation):
> 1. -filter:a 
> "asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat" 
> out.mp3
> 2. -filter:a "dumpwave=n=192197:f=-" out.mp3
> Results:
> 1. 244810550046 nanoseconds
> 2. 87494286740 nanoseconds
> 
> One of the possible use cases - to set up 2 chains of asetnsamples->metadata 
> - for example:
> "asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat,asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file1.dat”
>  for sure it will affect performance
> Comparing with "dumpwave=n=192197:f=out1,dumpwave=n= 22050:f=out2”
> 
Ping
>> Regards,
>> Tobias
>> 
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
—
Best regards,
Dmytro
From 64e953923a0c9f882afcced1d00ce572e0fafaeb Mon Sep 17 00:00:00 2001
From: Dmytro Humeniuk 
Date: Mon, 15 Jan 2018 00:01:25 +0100
Subject: [PATCH] avfilter: add dumpwave filter.

Signed-off-by: Dmytro Humeniuk 
---
 Changelog|1 +
 doc/filters.texi |   33 +-
 libavfilter/Makefile |1 +
 libavfilter/af_dumpwave.c|  234 +
 libavfilter/allfilters.c |1 +
 libavfilter/version.h|4 +-
 tests/fate-run.sh 

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Dmytro Humeniuk]

> On 15 Jan 2018, at 09:14, Tobias Rapp  wrote:
> 
> On 13.01.2018 23:52, Дмитрий Гуменюк wrote:
>> Hi,
>>> On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  wrote:
>>> 
>>> Hi
>>> 
 On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  
 wrote:
 
> On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:
> 
> On 12.01.2018 12:16, Дмитрий Гуменюк wrote:
>> Hi
>>> On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:
>>> 
>>> On 10.01.2018 18:18, Kyle Swanson wrote:
 Hi,
 For this to be a part of libavfilter the output needs to be more 
 generic
 than the just the Soundcloud format. If we want this to be generally 
 useful
 it should probably just output an array of floats between 0.0 and 1.0. 
 The
 consumer of this data (JS library, or whatever) can use this in 
 whatever
 way it wants.
>>> 
>>> I agree. If the BWF Peak Envelope output which was suggested in the 
>>> other thread does not match your demands and filter implementation is 
>>> actually necessary I would prefer if the filter would attach the RMS 
>>> value(s) as frame metadata instead of directly dumping to file. Frame 
>>> metadata can then be re-
>> RMS values may be counted for several frames or only for a half of a 
>> frame
>>> used by other filters or dumped into file by using the existing 
>>> "ametadata" filter.
>>> 
>>> This would be similar to:
>>> 
>>> ffmpeg -i input-file -f null -filter:a 
>>> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
>>>  /dev/null
>> I like this idea, but won’t asetnsamples affect performance by creating 
>> fifo queue? And it may require some effort to parse long output
> 
> I added asetnsamples to define the audio frame size (interval of values 
> from astats). You can reduce the number of lines printed by ametadata by 
> using the "key=lavfi.astats.foo" option.
 I used asetnsamples as well, and I measured performance while transcoding 
 - it appears to be slight slower
>>> I think output is now more generic and I got rid of long switch/case, 
>>> thanks for support
>> Here is most recent patch, seems like all comments are addressed, did I miss 
>> something?
> 
> I still would prefer to have the value attached as frame metadata, then 
> dumped into file via the existing "ametadata" filter. Even better would be to 
> integrate the statistic value (if missing) into the "astats" filter.
> 
> If your concern is the output format of "ametadata" then some output format 
> extension (CSV/JSON) needs to be discussed for ametadata/metadata.
> 
> If your concern is performance then please add some numbers. In my tests 
> using an approx. 5 minutes input WAV file (48kHz, stereo) the run with 
> "asetnsamples" was considerably faster than the run without (1.7s vs. 13.9s)
Hi
As I mentioned previously adding metadata to each frame is not possible
as value may be counted for several frames or only for a half of a frame 

I used 2 hours long 48kHz mp3 
https://s3-eu-west-1.amazonaws.com/balamii/SynthSystemSystersJAN2018.mp3
For this purposes I set up CentOS AWS EC2 nano instance
Then I transcoded it while filtering like following (just to recreate real 
situation):
1. -filter:a 
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat" 
out.mp3
2. -filter:a "dumpwave=n=192197:f=-" out.mp3
Results:
1. 244810550046 nanoseconds
2. 87494286740 nanoseconds

One of the possible use cases - to set up 2 chains of asetnsamples->metadata - 
for example:
"asetnsamples=192197,astats=metadata=on,ametadata=print:file=stats-file.dat,asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file1.dat”
 for sure it will affect performance
Comparing with "dumpwave=n=192197:f=out1,dumpwave=n= 22050:f=out2"

> Regards,
> Tobias
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Tobias Rapp]

On 13.01.2018 23:52, Дмитрий Гуменюк wrote:

Hi,

On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  wrote:

Hi


On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  wrote:


On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:

On 12.01.2018 12:16, Дмитрий Гуменюк wrote:

Hi

On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,
For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.


I agree. If the BWF Peak Envelope output which was suggested in the other 
thread does not match your demands and filter implementation is actually 
necessary I would prefer if the filter would attach the RMS value(s) as frame 
metadata instead of directly dumping to file. Frame metadata can then be re-

RMS values may be counted for several frames or only for a half of a frame

used by other filters or dumped into file by using the existing "ametadata" 
filter.

This would be similar to:

ffmpeg -i input-file -f null -filter:a 
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
/dev/null

I like this idea, but won’t asetnsamples affect performance by creating fifo 
queue? And it may require some effort to parse long output


I added asetnsamples to define the audio frame size (interval of values from astats). You 
can reduce the number of lines printed by ametadata by using the 
"key=lavfi.astats.foo" option.

I used asetnsamples as well, and I measured performance while transcoding - it 
appears to be slight slower

I think output is now more generic and I got rid of long switch/case, thanks 
for support

Here is most recent patch, seems like all comments are addressed, did I miss 
something?


I still would prefer to have the value attached as frame metadata, then 
dumped into file via the existing "ametadata" filter. Even better would 
be to integrate the statistic value (if missing) into the "astats" filter.


If your concern is the output format of "ametadata" then some output 
format extension (CSV/JSON) needs to be discussed for ametadata/metadata.


If your concern is performance then please add some numbers. In my tests 
using an approx. 5 minutes input WAV file (48kHz, stereo) the run with 
"asetnsamples" was considerably faster than the run without (1.7s vs. 13.9s)


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

Hi, 
> On 13 Jan 2018, at 01:37, Дмитрий Гуменюк  wrote:
> 
> Hi
> 
>> On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  wrote:
>> 
>>> On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:
>>> 
>>> On 12.01.2018 12:16, Дмитрий Гуменюк wrote:
 Hi
> On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:
> 
> On 10.01.2018 18:18, Kyle Swanson wrote:
>> Hi,
>> For this to be a part of libavfilter the output needs to be more generic
>> than the just the Soundcloud format. If we want this to be generally 
>> useful
>> it should probably just output an array of floats between 0.0 and 1.0. 
>> The
>> consumer of this data (JS library, or whatever) can use this in whatever
>> way it wants.
> 
> I agree. If the BWF Peak Envelope output which was suggested in the other 
> thread does not match your demands and filter implementation is actually 
> necessary I would prefer if the filter would attach the RMS value(s) as 
> frame metadata instead of directly dumping to file. Frame metadata can 
> then be re-
 RMS values may be counted for several frames or only for a half of a frame
> used by other filters or dumped into file by using the existing 
> "ametadata" filter.
> 
> This would be similar to:
> 
> ffmpeg -i input-file -f null -filter:a 
> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
>  /dev/null
 I like this idea, but won’t asetnsamples affect performance by creating 
 fifo queue? And it may require some effort to parse long output
>>> 
>>> I added asetnsamples to define the audio frame size (interval of values 
>>> from astats). You can reduce the number of lines printed by ametadata by 
>>> using the "key=lavfi.astats.foo" option.
>> I used asetnsamples as well, and I measured performance while transcoding - 
>> it appears to be slight slower
> I think output is now more generic and I got rid of long switch/case, thanks 
> for support 
Here is most recent patch, seems like all comments are addressed, did I miss 
something?
>>> 
>>> Regards,
>>> Tobias
>>> 
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> <0001-avfilter-add-dumpwave-filter.patch.txt>


FFmpeg-devel-v2-avfilter-add-dumpwave-filter..patch
Description: Binary data





___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

Hi

> On 12 Jan 2018, at 13:32, Дмитрий Гуменюк  wrote:
> 
>> On 12 Jan 2018, at 13:17, Tobias Rapp > > wrote:
>> 
>> On 12.01.2018 12:16, Дмитрий Гуменюк wrote:
>>> Hi
 On 11 Jan 2018, at 09:20, Tobias Rapp > wrote:
 
 On 10.01.2018 18:18, Kyle Swanson wrote:
> Hi,
> For this to be a part of libavfilter the output needs to be more generic
> than the just the Soundcloud format. If we want this to be generally 
> useful
> it should probably just output an array of floats between 0.0 and 1.0. The
> consumer of this data (JS library, or whatever) can use this in whatever
> way it wants.
 
 I agree. If the BWF Peak Envelope output which was suggested in the other 
 thread does not match your demands and filter implementation is actually 
 necessary I would prefer if the filter would attach the RMS value(s) as 
 frame metadata instead of directly dumping to file. Frame metadata can 
 then be re-
>>> RMS values may be counted for several frames or only for a half of a frame
 used by other filters or dumped into file by using the existing 
 "ametadata" filter.
 
 This would be similar to:
 
 ffmpeg -i input-file -f null -filter:a 
 "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat"
  /dev/null
>>> I like this idea, but won’t asetnsamples affect performance by creating 
>>> fifo queue? And it may require some effort to parse long output
>> 
>> I added asetnsamples to define the audio frame size (interval of values from 
>> astats). You can reduce the number of lines printed by ametadata by using 
>> the "key=lavfi.astats.foo" option.
> I used asetnsamples as well, and I measured performance while transcoding - 
> it appears to be slight slower
I think output is now more generic and I got rid of long switch/case, thanks 
for support 

>> 
>> Regards,
>> Tobias
>> 
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org 
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
>> 

smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

> On 12 Jan 2018, at 13:17, Tobias Rapp  wrote:
> 
> On 12.01.2018 12:16, Дмитрий Гуменюк wrote:
>> Hi
>>> On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:
>>> 
>>> On 10.01.2018 18:18, Kyle Swanson wrote:
 Hi,
 For this to be a part of libavfilter the output needs to be more generic
 than the just the Soundcloud format. If we want this to be generally useful
 it should probably just output an array of floats between 0.0 and 1.0. The
 consumer of this data (JS library, or whatever) can use this in whatever
 way it wants.
>>> 
>>> I agree. If the BWF Peak Envelope output which was suggested in the other 
>>> thread does not match your demands and filter implementation is actually 
>>> necessary I would prefer if the filter would attach the RMS value(s) as 
>>> frame metadata instead of directly dumping to file. Frame metadata can then 
>>> be re-
>> RMS values may be counted for several frames or only for a half of a frame
>>> used by other filters or dumped into file by using the existing "ametadata" 
>>> filter.
>>> 
>>> This would be similar to:
>>> 
>>> ffmpeg -i input-file -f null -filter:a 
>>> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
>>> /dev/null
>> I like this idea, but won’t asetnsamples affect performance by creating fifo 
>> queue? And it may require some effort to parse long output
> 
> I added asetnsamples to define the audio frame size (interval of values from 
> astats). You can reduce the number of lines printed by ametadata by using the 
> "key=lavfi.astats.foo" option.
I used asetnsamples as well, and I measured performance while transcoding - it 
appears to be slight slower
> 
> 
> Regards,
> Tobias
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org 
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
> 


smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Tobias Rapp]

On 12.01.2018 12:16, Дмитрий Гуменюк wrote:

Hi

On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,
For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.


I agree. If the BWF Peak Envelope output which was suggested in the other 
thread does not match your demands and filter implementation is actually 
necessary I would prefer if the filter would attach the RMS value(s) as frame 
metadata instead of directly dumping to file. Frame metadata can then be re-

RMS values may be counted for several frames or only for a half of a frame

used by other filters or dumped into file by using the existing "ametadata" 
filter.

This would be similar to:

ffmpeg -i input-file -f null -filter:a 
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
/dev/null

I like this idea, but won’t asetnsamples affect performance by creating fifo 
queue? And it may require some effort to parse long output


I added asetnsamples to define the audio frame size (interval of values 
from astats). You can reduce the number of lines printed by ametadata by 
using the "key=lavfi.astats.foo" option.


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

Hi
> On 11 Jan 2018, at 09:20, Tobias Rapp  wrote:
> 
> On 10.01.2018 18:18, Kyle Swanson wrote:
>> Hi,
>> For this to be a part of libavfilter the output needs to be more generic
>> than the just the Soundcloud format. If we want this to be generally useful
>> it should probably just output an array of floats between 0.0 and 1.0. The
>> consumer of this data (JS library, or whatever) can use this in whatever
>> way it wants.
> 
> I agree. If the BWF Peak Envelope output which was suggested in the other 
> thread does not match your demands and filter implementation is actually 
> necessary I would prefer if the filter would attach the RMS value(s) as frame 
> metadata instead of directly dumping to file. Frame metadata can then be re-
RMS values may be counted for several frames or only for a half of a frame 
> used by other filters or dumped into file by using the existing "ametadata" 
> filter.
> 
> This would be similar to:
> 
> ffmpeg -i input-file -f null -filter:a 
> "asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
> /dev/null
I like this idea, but won’t asetnsamples affect performance by creating fifo 
queue? And it may require some effort to parse long output
> 
> 
> BTW: The "astats" filter already provides some RMS values.
> 
>> If you send another patch, just reply to this thread because
>> that makes it easier to follow (sending a patch as an attachment is OK).
>> Here are some critiques:
>> [...]
> 
> Also when sending patches adding an increased version number helps sorting 
> out which is the latest one (git format-patch -v2 ...).
> 
> Regards,
> Tobias
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Tobias Rapp]

On 10.01.2018 18:18, Kyle Swanson wrote:

Hi,

For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants.


I agree. If the BWF Peak Envelope output which was suggested in the 
other thread does not match your demands and filter implementation is 
actually necessary I would prefer if the filter would attach the RMS 
value(s) as frame metadata instead of directly dumping to file. Frame 
metadata can then be re-used by other filters or dumped into file by 
using the existing "ametadata" filter.


This would be similar to:

ffmpeg -i input-file -f null -filter:a 
"asetnsamples=22050,astats=metadata=on,ametadata=print:file=stats-file.dat" 
/dev/null


BTW: The "astats" filter already provides some RMS values.


If you send another patch, just reply to this thread because
that makes it easier to follow (sending a patch as an attachment is OK).
Here are some critiques:

[...]


Also when sending patches adding an increased version number helps 
sorting out which is the latest one (git format-patch -v2 ...).


Regards,
Tobias

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Kyle Swanson]

Hi,

For this to be a part of libavfilter the output needs to be more generic
than the just the Soundcloud format. If we want this to be generally useful
it should probably just output an array of floats between 0.0 and 1.0. The
consumer of this data (JS library, or whatever) can use this in whatever
way it wants. If you send another patch, just reply to this thread because
that makes it easier to follow (sending a patch as an attachment is OK).
Here are some critiques:

+typedef struct DumpWaveContext {
> +const AVClass *class;   /**< class for AVOptions */
> +int w;  /**< number of data values in json */
> +int h;  /**< values will be scaled according to
> provided */
> +int is_disabled;/**< disable filter in case it's
> misconfigured */
> +int i;  /**< index of value */
> +char *json; /**< path to json */
> +char *str;  /**< comma separated values */, wha
> +double *values; /**< scaling factors */
> +int64_t s;  /**< samples per value per channel */
> +int64_t n;  /**< current number of samples counted */
> +int64_t max_samples;/**< samples per value */
> +double sum; /**< sum of the squared samples per value */
> +} DumpWaveContext;

Use more descriptive variable names for your struct members. They don't
have to be just one letter.


> +{ "d", "set width and height", OFFSET(w), AV_OPT_TYPE_IMAGE_SIZE,
> {.str = "640x480"}, 0, 0, FLAGS },

Get rid of this. We shouldn't care how this data is used/rendered. Our only
job should be to output an array of floats.


> +{ "s", "set number of samples per value per channel",  OFFSET(s),
> AV_OPT_TYPE_INT64,  {.i64 = 0}, 0, INT64_MAX, FLAGS },

Maybe you can call this frame_size? 0 is not a useful value here, it
shouldn't be the lower bound or the default value.



> +static av_cold int init(AVFilterContext *ctx)
> +{
> +DumpWaveContext *dumpwave = ctx->priv;
> +if(!dumpwave->s) {

The filter should just fail if it's not configured correctly. You'll get
this behavior for free with better default values.


> +static int config_output(AVFilterLink *outlink)
> +{
> +AVFilterContext *ctx = outlink->src;
> +DumpWaveContext *dumpwave = ctx->priv;
> +const int width = dumpwave->w;
> +dumpwave->values = av_realloc(NULL, width * sizeof(double));
> +dumpwave->str = av_realloc(NULL, width * sizeof(int));

You don't need a giant buffer to hold the entire string. Just keep a file
open a write to it every frame. Maybe we could just write if to stdout
instead?


> +
> +/**
> + * Converts sample to dB and calculates root mean squared value
> + */
> +static inline void dbRms(DumpWaveContext *dumpwave, double smpl)
>
Just call this RMS and spit something out between 0.0 and 1.0.  Avoid
camelcase for function names.


>
> +switch (inlink->format) {
> +case AV_SAMPLE_FMT_DBLP:
> +for (c = 0; c < channels; c++) {
> +const double *src = (const double *)buf->extended_data[c];
> +
> +for (i = 0; i < buf->nb_samples; i++, src++)
> +dbRms(dumpwave, *src);
> +}
> +break;
> +case AV_SAMPLE_FMT_DBL: {
> +const double *src = (const double *)buf->extended_data[0];
> +
> +for (i = 0; i < buf->nb_samples; i++) {
> +for (c = 0; c < channels; c++, src++)
> +dbRms(dumpwave, *src);
> +}}
> +break;
> +case AV_SAMPLE_FMT_FLTP:
> +for (c = 0; c < channels; c++) {
> +const float *src = (const float *)buf->extended_data[c];
> +
> +for (i = 0; i < buf->nb_samples; i++, src++)
> +dbRms(dumpwave, *src);
> +}
> +break;
> +case AV_SAMPLE_FMT_FLT: {
> +const float *src = (const float *)buf->extended_data[0];
> +
> +for (i = 0; i < buf->nb_samples; i++) {
> +for (c = 0; c < channels; c++, src++)
> +dbRms(dumpwave, *src);
> +}}
> +break;
> +case AV_SAMPLE_FMT_S64P:
> +for (c = 0; c < channels; c++) {
> +const int64_t *src = (const int64_t
> *)buf->extended_data[c];
> +
> +for (i = 0; i < buf->nb_samples; i++, src++)
> +dbRms(dumpwave, *src / (double)INT64_MAX);
> +}
> +break;
> +case AV_SAMPLE_FMT_S64: {
> +const int64_t *src = (const int64_t *)buf->extended_data[0];
> +
> +for (i = 0; i < buf->nb_samples; i++) {
> +for (c = 0; c < channels; c++, src++)
> +dbRms(dumpwave, *src / (double)INT64_MAX);
> +}}
> +break;
> +case AV_SAMPLE_FMT_S32P:
> +for (c = 0; c < channels; c++) {
> +const int32_t *src = (const 

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

Same JSON schema used by SoundCloud

> On 10 Jan 2018, at 10:16, Дмитрий Гуменюк  wrote:
> 
>> wavesurer.js  - Web Audio API
> I mean its would be hard to do the same for large files
> https://developer.mozilla.org/en-US/docs/Web/API/BaseAudioContext/decodeAudioData
> 
>> On 10 Jan 2018, at 09:04, Дмитрий Гуменюк  wrote:
>> 
>> Hi, 
>> While Waveform.js converts old SoundCloud PNGs, wavesurer.js is using Web 
>> Audio API which is limited/not supported by all browsers
>> 
>>> On 10 Jan 2018, at 08:51, Дмитрий Гуменюк  wrote:
>>> 
>>> There is no rush on this. Could you please do a code review so I can see 
>>> how to do things properly?
 On 10 Jan 2018, at 08:43, Kyle Swanson  wrote:
 
 Hi,
 
 On Tue, Jan 9, 2018 at 3:49 PM,   wrote:
> From: Dmytro Humeniuk 
> 
> Signed-off-by: Dmytro Humeniuk 
> ---
> Changelog  |   1 +
> doc/filters.texi   |  23 
> libavfilter/Makefile   |   1 +
> libavfilter/af_dumpwave.c  | 285
 +
> libavfilter/allfilters.c   |   1 +
> libavfilter/version.h  |   4 +-
> tests/fate/filter-audio.mak|   5 +
> tests/ref/fate/filter-dumpwave |   1 +
> 8 files changed, 319 insertions(+), 2 deletions(-)
> create mode 100644 libavfilter/af_dumpwave.c
> create mode 100644 tests/ref/fate/filter-dumpwave
 
 I could see this possibly being a useful filter, but I'm confused about
 where the JSON schema came from. The two JS libraries that do this type of
 thing (waveform.js, and wavesurer.js) both just load waveform data as an
 array of floats. If we're going to add something like this to libavfilter
 it should be as generic and extensible as possible. I'm not wild about the
 string stuff, and the big sample format switch isn't necessary. I could do
 a code review, but it might just be faster if I rewrite it and send another
 patch. Is that OK with you?
 
 Thanks,
 Kyle
 ___
 ffmpeg-devel mailing list
 ffmpeg-devel@ffmpeg.org
 http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>> 
> 



smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

> wavesurer.js  - Web Audio API
I mean its would be hard to do the same for large files
https://developer.mozilla.org/en-US/docs/Web/API/BaseAudioContext/decodeAudioData

> On 10 Jan 2018, at 09:04, Дмитрий Гуменюк  wrote:
> 
> Hi, 
> While Waveform.js converts old SoundCloud PNGs, wavesurer.js is using Web 
> Audio API which is limited/not supported by all browsers
> 
>> On 10 Jan 2018, at 08:51, Дмитрий Гуменюк  wrote:
>> 
>> There is no rush on this. Could you please do a code review so I can see how 
>> to do things properly?
>>> On 10 Jan 2018, at 08:43, Kyle Swanson  wrote:
>>> 
>>> Hi,
>>> 
>>> On Tue, Jan 9, 2018 at 3:49 PM,   wrote:
 From: Dmytro Humeniuk 
 
 Signed-off-by: Dmytro Humeniuk 
 ---
 Changelog  |   1 +
 doc/filters.texi   |  23 
 libavfilter/Makefile   |   1 +
 libavfilter/af_dumpwave.c  | 285
>>> +
 libavfilter/allfilters.c   |   1 +
 libavfilter/version.h  |   4 +-
 tests/fate/filter-audio.mak|   5 +
 tests/ref/fate/filter-dumpwave |   1 +
 8 files changed, 319 insertions(+), 2 deletions(-)
 create mode 100644 libavfilter/af_dumpwave.c
 create mode 100644 tests/ref/fate/filter-dumpwave
>>> 
>>> I could see this possibly being a useful filter, but I'm confused about
>>> where the JSON schema came from. The two JS libraries that do this type of
>>> thing (waveform.js, and wavesurer.js) both just load waveform data as an
>>> array of floats. If we're going to add something like this to libavfilter
>>> it should be as generic and extensible as possible. I'm not wild about the
>>> string stuff, and the big sample format switch isn't necessary. I could do
>>> a code review, but it might just be faster if I rewrite it and send another
>>> patch. Is that OK with you?
>>> 
>>> Thanks,
>>> Kyle
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
> 



smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

Hi, 
While Waveform.js converts old SoundCloud PNGs, wavesurer.js is using Web Audio 
API which is limited/not supported by all browsers

> On 10 Jan 2018, at 08:51, Дмитрий Гуменюк  wrote:
> 
> There is no rush on this. Could you please do a code review so I can see how 
> to do things properly?
>> On 10 Jan 2018, at 08:43, Kyle Swanson  wrote:
>> 
>> Hi,
>> 
>> On Tue, Jan 9, 2018 at 3:49 PM,   wrote:
>>> From: Dmytro Humeniuk 
>>> 
>>> Signed-off-by: Dmytro Humeniuk 
>>> ---
>>> Changelog  |   1 +
>>> doc/filters.texi   |  23 
>>> libavfilter/Makefile   |   1 +
>>> libavfilter/af_dumpwave.c  | 285
>> +
>>> libavfilter/allfilters.c   |   1 +
>>> libavfilter/version.h  |   4 +-
>>> tests/fate/filter-audio.mak|   5 +
>>> tests/ref/fate/filter-dumpwave |   1 +
>>> 8 files changed, 319 insertions(+), 2 deletions(-)
>>> create mode 100644 libavfilter/af_dumpwave.c
>>> create mode 100644 tests/ref/fate/filter-dumpwave
>> 
>> I could see this possibly being a useful filter, but I'm confused about
>> where the JSON schema came from. The two JS libraries that do this type of
>> thing (waveform.js, and wavesurer.js) both just load waveform data as an
>> array of floats. If we're going to add something like this to libavfilter
>> it should be as generic and extensible as possible. I'm not wild about the
>> string stuff, and the big sample format switch isn't necessary. I could do
>> a code review, but it might just be faster if I rewrite it and send another
>> patch. Is that OK with you?
>> 
>> Thanks,
>> Kyle
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 



smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Дмитрий Гуменюк]

There is no rush on this. Could you please do a code review so I can see how to 
do things properly?
> On 10 Jan 2018, at 08:43, Kyle Swanson  wrote:
> 
> Hi,
> 
> On Tue, Jan 9, 2018 at 3:49 PM,   wrote:
>> From: Dmytro Humeniuk 
>> 
>> Signed-off-by: Dmytro Humeniuk 
>> ---
>> Changelog  |   1 +
>> doc/filters.texi   |  23 
>> libavfilter/Makefile   |   1 +
>> libavfilter/af_dumpwave.c  | 285
> +
>> libavfilter/allfilters.c   |   1 +
>> libavfilter/version.h  |   4 +-
>> tests/fate/filter-audio.mak|   5 +
>> tests/ref/fate/filter-dumpwave |   1 +
>> 8 files changed, 319 insertions(+), 2 deletions(-)
>> create mode 100644 libavfilter/af_dumpwave.c
>> create mode 100644 tests/ref/fate/filter-dumpwave
> 
> I could see this possibly being a useful filter, but I'm confused about
> where the JSON schema came from. The two JS libraries that do this type of
> thing (waveform.js, and wavesurer.js) both just load waveform data as an
> array of floats. If we're going to add something like this to libavfilter
> it should be as generic and extensible as possible. I'm not wild about the
> string stuff, and the big sample format switch isn't necessary. I could do
> a code review, but it might just be faster if I rewrite it and send another
> patch. Is that OK with you?
> 
> Thanks,
> Kyle
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



smime.p7s
Description: S/MIME cryptographic signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add dumpwave filter

[Kyle Swanson]

Hi,

On Tue, Jan 9, 2018 at 3:49 PM,   wrote:
> From: Dmytro Humeniuk 
>
> Signed-off-by: Dmytro Humeniuk 
> ---
>  Changelog  |   1 +
>  doc/filters.texi   |  23 
>  libavfilter/Makefile   |   1 +
>  libavfilter/af_dumpwave.c  | 285
+
>  libavfilter/allfilters.c   |   1 +
>  libavfilter/version.h  |   4 +-
>  tests/fate/filter-audio.mak|   5 +
>  tests/ref/fate/filter-dumpwave |   1 +
>  8 files changed, 319 insertions(+), 2 deletions(-)
>  create mode 100644 libavfilter/af_dumpwave.c
>  create mode 100644 tests/ref/fate/filter-dumpwave

I could see this possibly being a useful filter, but I'm confused about
where the JSON schema came from. The two JS libraries that do this type of
thing (waveform.js, and wavesurer.js) both just load waveform data as an
array of floats. If we're going to add something like this to libavfilter
it should be as generic and extensible as possible. I'm not wild about the
string stuff, and the big sample format switch isn't necessary. I could do
a code review, but it might just be faster if I rewrite it and send another
patch. Is that OK with you?

Thanks,
Kyle
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel