Re: [FFmpeg-devel] [PATCH] avfilter: add ahistogram multimedia filter

2016-01-06 Thread Moritz Barsnick 
On Tue, Jan 05, 2016 at 21:36:20 +0100, Moritz Barsnick wrote:
> This is not the first of your filters where you describe a parameter as
> a "percentage", but expect a float value [0, 1]. That is extremely
> confusing, if not even wrong. To me, a percentage covers [0, 100]. You
> need to find a different term for the "factor" ranging from zero to
> one.

Sorry, that was an unfair remark, as my memory failed me. The commit I
was primarily referring to was by a different author.

I see this use in the tremolo, vibrato, chromakey, colorkey filters,
while several others use the term percentage for something with a [0,
100] full range.

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

Re: [FFmpeg-devel] [PATCH] avfilter: add ahistogram multimedia filter

2016-01-06 Thread Ganesh Ajjanagadde 
On Wed, Jan 6, 2016 at 5:36 AM, Moritz Barsnick  wrote:
> On Tue, Jan 05, 2016 at 21:36:20 +0100, Moritz Barsnick wrote:
>> This is not the first of your filters where you describe a parameter as
>> a "percentage", but expect a float value [0, 1]. That is extremely
>> confusing, if not even wrong. To me, a percentage covers [0, 100]. You
>> need to find a different term for the "factor" ranging from zero to
>> one.
>
> Sorry, that was an unfair remark, as my memory failed me. The commit I
> was primarily referring to was by a different author.
>
> I see this use in the tremolo, vibrato, chromakey, colorkey filters,
> while several others use the term percentage for something with a [0,
> 100] full range.

Patch welcome ;).

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

Re: [FFmpeg-devel] [PATCH] avfilter: add ahistogram multimedia filter

2016-01-05 Thread Moritz Barsnick 
On Tue, Jan 05, 2016 at 21:19:12 +0100, Paul B Mahol wrote:
> +It accepts the following values:
> +@table @samp
> +@item replace
> +logarithmic

   ^^^ This description is a copy/paste error.

> +@item pheight
> +Set histogram percentage of window height.
[...]
> +{ "pheight", "set histogram percentage of window height", 
> OFFSET(phisto), AV_OPT_TYPE_FLOAT, {.dbl=0.10}, 0, 1, FLAGS },

This is not the first of your filters where you describe a parameter as
a "percentage", but expect a float value [0, 1]. That is extremely
confusing, if not even wrong. To me, a percentage covers [0, 100]. You
need to find a different term for the "factor" ranging from zero to
one.

I left the rest of the code functionally unreviewed so far. I will test
later.

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

Re: [FFmpeg-devel] [PATCH] avfilter: add ahistogram multimedia filter

2015-12-30 Thread Lou Logan 
On Wed, 30 Dec 2015 13:49:07 +0100, Paul B Mahol wrote:

> diff --git a/doc/filters.texi b/doc/filters.texi

[...]

> +@item ascale
> +Set amplitude scale.
> +
> +It accepts the following values:
> +@table @samp
> +@item log
> +logarithmic
> +@item lin
> +linear
> +@end table

Default is @code{lin}.

> +@item pheight
> +Set histogram percentage of window height.

Default is 0.25.

> +@item slide
> +Set sonogram sliding.
> +
> +It accepts the following values:
> +@table @samp
> +@item replace
> +logarithmic
> +@item scroll
> +scroll from to bottom.

scroll from top to bottom

> +@end table

Default is @code{replace}.

[...]

> +static const AVOption ahistogram_options[] = {
> +{ "mode", "set how is histogram calculated", OFFSET(mode), 
> AV_OPT_TYPE_INT, {.i64=SINGLE}, 0, NB_MODES-1, FLAGS, "mode" },

"set method to display channels" makes more sense to me.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel