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

[Nicolas George]

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.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20191227/45fc7f4f/attachment.sig>