Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Martin Vignali]

>
> You forgot to update color in documentation.
>
> Patchset OK otherwise.
>
>
Fix doc, and pushed.
Thanks

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

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Paul B Mahol]

On 4/15/18, Martin Vignali  wrote:
>>
>> Default white color is good decision ? Better use orange?
>>
>> Put { in own line like for every function:
>>
>> void its_me_function()
>> {<- have own line
>> 
>> }
>>
>> Generally follow code style of file.
>> ___
>>
>>
> New patchs in attach
> fix bracket after func declaration
> change default persistent max line color to orange

You forgot to update color in documentation.

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

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Martin Vignali]

>
> Default white color is good decision ? Better use orange?
>
> Put { in own line like for every function:
>
> void its_me_function()
> {<- have own line
> 
> }
>
> Generally follow code style of file.
> ___
>
>
New patchs in attach
fix bracket after func declaration
change default persistent max line color to orange

also add a cosmetic,


Martin


0001-avfilter-showvolume-add-display-scale-option.patch
Description: Binary data


0002-avfilter-showvolume-add-persistent-max-display.patch
Description: Binary data


0003-avfilter-showvolume-cosmetic-move-bracket-after-func.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Paul B Mahol]

On 4/15/18, Martin Vignali  wrote:
> and when there is no silence i get completly empty display +
>
>> current volume in dB.
>>
>>
> You're right, my previous calc was wrong (doesn't display for level below
> -42 db)
>
> New patchs in attach
>
> use another calc for the log display : av_clipf(0.21 * log10(max) + 1, 0,
> 1);
>
>
> for the persistent max display, i add a new option for line color
> and rename dm_duration to dm
>
>
> Martin
>

Default white color is good decision ? Better use orange?

Put { in own line like for every function:

void its_me_function()
{<- have own line

}

Generally follow code style of file.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Paul B Mahol]

On 4/14/18, Martin Vignali  wrote:
>>
>> Please use short name for options, max 2 characters.
>>
>> The log scale shows nothing even thought there is no silence. Perhaps
>> use ceil()?
>>
> Doesn't understand this part.
> Do you mean, you think the log scale display option is not a good idea ?
> Or doesn't work in some case ?
>
> In my test, the display log scale works ok, and more common way to check
> level
> (in linear scale, the level seems to be too low, when we usually see in log
> scale)

I dunno what music you do listen, but mine have very high
dynamic range, and when there is no silence i get completly empty display +
current volume in dB.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Martin Vignali]

>
> Please use short name for options, max 2 characters.
>
> The log scale shows nothing even thought there is no silence. Perhaps
> use ceil()?
>
Doesn't understand this part.
Do you mean, you think the log scale display option is not a good idea ?
Or doesn't work in some case ?

In my test, the display log scale works ok, and more common way to check
level
(in linear scale, the level seems to be too low, when we usually see in log
scale)




> The persistent max display color should be configurable.
>
> Ok will send a new patch for that.

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

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Paul B Mahol]

On 4/11/18, Martin Vignali  wrote:
> Hello,
>
> Thanks for the comments.
>
> New patchs in attach :
>
> 001 : Add display_scale volume
> Change since prev patch :
> - use enum for the value
> - move the max_draw calc part to an inline func (avoid code duplication).
>
> 002 : Add Persistent max display
> Change since prev patch
> - Use only one param (dm_duration), if set to 0. (the default), disabled
> display and calc
> - move some part in inline func, to reduce code duplication
> - offset line draw by 1 pixel (and use FFMAX to clip to 0). Fix display max
> line, when max is 0db in horizontal mode
> - add doc
>
> Martin
>

Please use short name for options, max 2 characters.

The log scale shows nothing even thought there is no silence. Perhaps
use ceil()?

The persistent max display color should be configurable.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Martin Vignali]

Hello,

Thanks for the comments.

New patchs in attach :

001 : Add display_scale volume
Change since prev patch :
- use enum for the value
- move the max_draw calc part to an inline func (avoid code duplication).

002 : Add Persistent max display
Change since prev patch
- Use only one param (dm_duration), if set to 0. (the default), disabled
display and calc
- move some part in inline func, to reduce code duplication
- offset line draw by 1 pixel (and use FFMAX to clip to 0). Fix display max
line, when max is 0db in horizontal mode
- add doc

Martin


0001-avfilter-showvolume-add-display-scale-option.patch
Description: Binary data


0002-avfilter-showvolume-add-persistent-max-display.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Michael Niedermayer]

On Sat, Mar 31, 2018 at 04:21:36PM +0200, Martin Vignali wrote:
> Hello,
> 
> In attach new patchs for showvolume filter
> 
> 001 : Move the clear picture part to a new func, and use it if fade option
> == 0.
> (no need to calculate it in float)
> 
> 002/003 : Move "height" condition for draw volume at the start of the loop
> and indent
> 
> 004 : add a new option for choosing the display scale
> currently, the filter use linear display,
> most audio meter use some kind of log display
> 
> 
> 005 : WIP :
> The idea is to add an option to draw a line for the max level for a given
> duration.
> Several audio meter have this kind of display, and its help user, to check
> peak level (even
> if the "peak" happen during a very short time.
> 
> I'm not entirely sure, this is the best way to add this.
> 
> For now, i store the max value for each channel, update it, if a new peak
> is bigger than the store value
> And count the number of frame for display. If a given display duration is
> exceed, update the max value to the current max value.
> 
> Missing docs and probably code reorganization to reduce code duplication.
> 
> I also have had very few inspiration for option naming and variable naming
> (and i don't know the "official name" of this little line in audio meter).
> So i'm open to better naming suggestion.
> 
> This display need to be enable using dm=1
> and the duration during the max value is still display can be set with
> dm_duration (in seconds)
[...]

>  doc/filters.texi |4 
>  libavfilter/avf_showvolume.c |   22 +++---
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 727228fe3128595cad5a00f9370b63c1c3c06273  
> 0004-avfilter-showvolume-add-display-scale-option.patch
> From 06b19cff28558885b8c4a987d27f56a3a90528f7 Mon Sep 17 00:00:00 2001
> From: Martin Vignali 
> Date: Sat, 31 Mar 2018 15:52:20 +0200
> Subject: [PATCH 4/5] avfilter/showvolume : add display scale option
> 
> linear (current behaviour) or log display (more close to classic audio meter)
> ---
>  doc/filters.texi |  4 
>  libavfilter/avf_showvolume.c | 22 +++---
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index bf2b94e240..764abd4be7 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -19974,6 +19974,10 @@ Set background opacity, allowed range is [0, 1]. 
> Default is 0.
>  @item m
>  Set metering mode, can be peak: @code{p} or rms: @code{r},
>  default is @code{p}.
> +
> +@item ds
> +Set display scale, can be linear: @code{lin} or log: @code{log},
> +default is @code{lin}.
>  @end table
>  
>  @section showwaves
> diff --git a/libavfilter/avf_showvolume.c b/libavfilter/avf_showvolume.c
> index 7fe3916855..50ff80e022 100644
> --- a/libavfilter/avf_showvolume.c
> +++ b/libavfilter/avf_showvolume.c
> @@ -54,6 +54,7 @@ typedef struct ShowVolumeContext {
>  uint32_t *color_lut;
>  float *max;
>  float rms_factor;

> +int display_scale; /* 0 for linear, 1 for log */

please use #define or values from a enum.
self explanatory code is better than comments.

also leave the type of display_scale int dont change it to enum if you
use a enum, (generic code like AVOptions cannot support arbitrary enum types)

i like the idea of this patch though


[...]
>  avf_showvolume.c |   63 
> +++
>  1 file changed, 63 insertions(+)
> e256cdd9a6d44f721f19aab6eedba48be089ad03  
> 0005-avfilter-showvolume-add-persistent-max-display.patch
> From 1cac1a13e5142b0eff4acd17aecaac596ae359d4 Mon Sep 17 00:00:00 2001
> From: Martin Vignali 
> Date: Sat, 31 Mar 2018 15:53:04 +0200
> Subject: [PATCH 5/5] avfilter/showvolume : add persistent max display
> 
> WIP
> ---
>  libavfilter/avf_showvolume.c | 63 
> 
>  1 file changed, 63 insertions(+)

didnt review as its still WIP but i think the idea is good as well

you could combine the dm and dm_duration options though making the interface
simpler

thx
[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Martin Vignali]

2018-03-31 16:21 GMT+02:00 Martin Vignali :

> Hello,
>
> In attach new patchs for showvolume filter
>
> 001 : Move the clear picture part to a new func, and use it if fade option
> == 0.
> (no need to calculate it in float)
>
> 002/003 : Move "height" condition for draw volume at the start of the loop
> and indent
>
> 004 : add a new option for choosing the display scale
> currently, the filter use linear display,
> most audio meter use some kind of log display
>
>
> 005 : WIP :
> The idea is to add an option to draw a line for the max level for a given
> duration.
> Several audio meter have this kind of display, and its help user, to check
> peak level (even
> if the "peak" happen during a very short time.
>
> I'm not entirely sure, this is the best way to add this.
>
> For now, i store the max value for each channel, update it, if a new peak
> is bigger than the store value
> And count the number of frame for display. If a given display duration is
> exceed, update the max value to the current max value.
>
> Missing docs and probably code reorganization to reduce code duplication.
>
> I also have had very few inspiration for option naming and variable naming
> (and i don't know the "official name" of this little line in audio meter).
> So i'm open to better naming suggestion.
>
> This display need to be enable using dm=1
> and the duration during the max value is still display can be set with
> dm_duration (in seconds)
>
>
Ping for patch 004 and 005

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

[FFmpeg-devel] avfilter/showvolume : add new options and minor clean

[Martin Vignali]

Hello,

In attach new patchs for showvolume filter

001 : Move the clear picture part to a new func, and use it if fade option
== 0.
(no need to calculate it in float)

002/003 : Move "height" condition for draw volume at the start of the loop
and indent

004 : add a new option for choosing the display scale
currently, the filter use linear display,
most audio meter use some kind of log display


005 : WIP :
The idea is to add an option to draw a line for the max level for a given
duration.
Several audio meter have this kind of display, and its help user, to check
peak level (even
if the "peak" happen during a very short time.

I'm not entirely sure, this is the best way to add this.

For now, i store the max value for each channel, update it, if a new peak
is bigger than the store value
And count the number of frame for display. If a given display duration is
exceed, update the max value to the current max value.

Missing docs and probably code reorganization to reduce code duplication.

I also have had very few inspiration for option naming and variable naming
(and i don't know the "official name" of this little line in audio meter).
So i'm open to better naming suggestion.

This display need to be enable using dm=1
and the duration during the max value is still display can be set with
dm_duration (in seconds)


Comments welcome


Martin


0001-avfilter-showvolume-move-clear-picture-part-to-a-fun.patch
Description: Binary data


0002-avfilter-showvolume-move-width-test-for-draw-volume-.patch
Description: Binary data


0003-avfilter-showvolume-indent-after-prev-commit-and-add.patch
Description: Binary data


0004-avfilter-showvolume-add-display-scale-option.patch
Description: Binary data


0005-avfilter-showvolume-add-persistent-max-display.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel