[FFmpeg-devel] [PATCH 1/2] avfilter: add thistogram video filter

[Paul B Mahol]

On 12/27/19, Nicolas George <george at nsup.org> wrote:
> Paul B Mahol (12019-12-26):
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>  doc/filters.texi           |  46 ++++++++++++++
>>  libavfilter/Makefile       |   1 +
>>  libavfilter/allfilters.c   |   1 +
>>  libavfilter/vf_histogram.c | 127 ++++++++++++++++++++++++++++++-------
>>  4 files changed, 151 insertions(+), 24 deletions(-)
>
> Thanks for merging them. I have no objection to this version, just a
> small remark below.
>
> I did not look at the arithmetic of the code closely, but I can do it if
> you want me to.
>
>> +    if (!strcmp(ctx->filter->name, "thistogram"))
>> +        s->thistogram = 1;
>
> Could you not test if s->width == 1 and have just one filter:
>
> 	histogram=... -> equivalent to current histogram
>
> 	histogram=width=1080:... -> equivalent to new thistogram
>
> ? It would save you the task of using a macro for common options.

Nope, as it would not be logical. It would become big mess.

> Regards,
>
> --
>   Nicolas George
>