[FFmpeg-devel] [GSOC] [PATCH] avfilter: add panorama filter

[Moritz Barsnick]

On Sat, Apr 13, 2019 at 15:50:14 +0300, Eugene Lyapustin wrote:
>  libavfilter/Makefile      |   1 +
>  libavfilter/allfilters.c  |   1 +
>  libavfilter/vf_panorama.c | 637 ++++++++++++++++++++++++++++++++++++++

The documentation in doc/filters.texi also needs to be amended.

> +static const AVOption panorama_options[] = {
> +    {    "input", "set input projection",         OFFSET(in), AV_OPT_TYPE_INT,   {.i64=EQUIRECTANGULAR}, 0,    NB_PROJECTIONS-1, FLAGS, "in" },
> +    {        "e", "equirectangular",                       0, AV_OPT_TYPE_CONST, {.i64=EQUIRECTANGULAR}, 0,                   0, FLAGS, "in" },
> +    {     "c6x1", "cubemap",                               0, AV_OPT_TYPE_CONST, {.i64=CUBEMAP_6_1},     0,                   0, FLAGS, "in" },
> +    {     "c3x2", "cubemap",                               0, AV_OPT_TYPE_CONST, {.i64=CUBEMAP_3_2},     0,                   0, FLAGS, "in" },

These two options should get differing descriptions.
(BTW, though others may hate it, I find this formatting very well
readable!)

> +static inline int equal(double a, double b, double epsilon)
> +{
> +    return fabs(a - b) < epsilon;
> +}
> +
> +static inline int smaller(double a, double b, double epsilon)
> +{
> +    return ((a - b) < 0.0) && (!equal(a, b, epsilon));
> +}

If something comparable doesn't already exist, these could become
macros similar to FF_MAX/MIX et.al.

> +    if (face == BACK) {
[...]
> +    } else if (face == LEFT) {
[...]
> +    } else if (face == FRONT) {
[...]

What about switch/case? Just wondering.

> +    } else {
> +        ;
> +    }

I'm not sure an empty else should be expressed. ;-)

(Both previous comments are valid for various sections in the code.)

I don't understand most of the code, so no further comments from me.

Cheers,
Moritz