[FFmpeg-devel] [PATCH] swscale: Support setting filters through AVOptions

[Stefano Sabatini]

On date Saturday 2013-10-05 22:49:55 +0200, Michael Niedermayer encoded:
> anything else could be used or the routine even be made to accept
> any character as separator.
> 
> Docs are omitted as i expect long bikesheds on the syntax and which
> separator char to use
> 
> Examples:

> -vf scale=640:480:src_filter=1#h-0.5#0#0#0#1
> -vf scale=640:480:src_filter=1#c1#1#1#1#1#1#1#1#1#1#1#1#1#1#1#1#1#1
> -vf scale=640:480:src_filter=1#lh-0.2#1#-0.2#lv1#0#0#0#0#0#0#0#0#0#0#0#0#1

1. this needs sh escaping, since "#" introduces a comment
2. in case we extend lavfi graph syntac, this may need escaping inner
   escaping since are likely going to use "#" for comments (indeed I
   was surprised that we already don't)

Thus I suggest another separator character. We use "|" in such
cases. Ideally we should have a syntax to represent matrixes / vectors
(e.g. employing Matlab notation), but this would cause escaping hell
since we already use both ";" and ":" in the filtergraph.

In conclusion "|" seems at the moment preferable.

> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libswscale/options.c          |    3 ++
>  libswscale/swscale_internal.h |    4 +++
>  libswscale/utils.c            |   79 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+)
> 
> diff --git a/libswscale/options.c b/libswscale/options.c
> index 2b3147b..0ebb2b6 100644
> --- a/libswscale/options.c
> +++ b/libswscale/options.c
> @@ -74,6 +74,9 @@ static const AVOption swscale_options[] = {
>      { "bayer",           "bayer dither",                  0,                 AV_OPT_TYPE_CONST,  { .i64  = SWS_DITHER_BAYER  }, INT_MIN, INT_MAX,        VE, "sws_dither" },
>      { "ed",              "error diffusion",               0,                 AV_OPT_TYPE_CONST,  { .i64  = SWS_DITHER_ED     }, INT_MIN, INT_MAX,        VE, "sws_dither" },
>  
> +    { "src_filter",      "source filter",                 OFFSET(src_filter_string), AV_OPT_TYPE_STRING,  { .str = NULL      }, INT_MIN, INT_MAX,        VE },
> +    { "dst_filter",      "destination filter",            OFFSET(dst_filter_string), AV_OPT_TYPE_STRING,  { .str = NULL      }, INT_MIN, INT_MAX,        VE },

set source filter
set destination filter

> +
>      { NULL }
>  };
>  
> diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
> index 33fdfc2..4b6fff6 100644
> --- a/libswscale/swscale_internal.h
> +++ b/libswscale/swscale_internal.h
> @@ -296,6 +296,10 @@ typedef struct SwsContext {
>      int vChrDrop;                 ///< Binary logarithm of extra vertical subsampling factor in source image chroma planes specified by user.
>      int sliceDir;                 ///< Direction that slices are fed to the scaler (1 = top-to-bottom, -1 = bottom-to-top).
>      double param[2];              ///< Input parameters for scaling algorithms that need them.
> +    char *src_filter_string;
> +    char *dst_filter_string;
> +    SwsFilter *src_filter;
> +    SwsFilter *dst_filter;
>  
>      uint32_t pal_yuv[256];
>      uint32_t pal_rgb[256];
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index a2e3ce1..29faabb 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -42,6 +42,7 @@
>  #include "libavutil/avutil.h"
>  #include "libavutil/bswap.h"
>  #include "libavutil/cpu.h"
> +#include "libavutil/eval.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/mathematics.h"
>  #include "libavutil/opt.h"
> @@ -238,6 +239,68 @@ const char *sws_format_name(enum AVPixelFormat format)
>  }
>  #endif
>  
> +static SwsVector *parse_subfilter(const SwsContext *c, const char *str, const char *id)
> +{
> +    const char *p;
> +    int i;
> +    SwsVector *v = av_mallocz(sizeof(SwsVector));
> +
> +    p = strstr(str, id);

not very robust (this will ignore leading characters, possibly
introduced because of typos)

> +    if (p) {
> +        p++;
> +    } else
> +        p = strchr(str, id[0]);

> +    if (!p)
> +        p = strchr(str, id[1]);

this is weird

> +    if (p)
> +        p++;
> +    else
> +        p = str;
> +    for (i=0; ; i++) {
> +        char *end;
> +        if (av_reallocp(&v->coeff, (i+1) * sizeof(*v->coeff)) < 0)
> +            goto fail;
> +        v->coeff[i] = av_strtod(p, &end);
> +        if (end == p)
> +            goto fail;
> +        p = (const char *)end;
> +        if(*p == '#' && p[1]) {
> +            p++;
> +            if (*p == 'l' || *p == 'c' || *p == 'h' || *p == 'v')
> +                break;
> +        } else if(!*p)
> +            break;
> +        else
> +            goto fail;
> +    }
> +    v->length = i + 1;
> +    return v;
> +fail:
> +    av_log(c, AV_LOG_ERROR, "parse_subfilter failed at %s\n", p);
> +    sws_freeVec(v);
> +    return NULL;
> +}
> +
> +static SwsFilter *parse_filter(const SwsContext *c, const char *str)
> +{
> +    SwsFilter *f;
> +    if (!str)
> +        return NULL;
> +    f = av_mallocz(sizeof(SwsFilter));
> +    if (!f)
> +        return NULL;
> +    f->lumH = parse_subfilter(c, str, "lh");
> +    f->lumV = parse_subfilter(c, str, "lv");
> +    f->chrH = parse_subfilter(c, str, "ch");
> +    f->chrV = parse_subfilter(c, str, "cv");
> +    if (!f->lumH || !f->lumV || !f->chrH || !f->chrV) {
> +        sws_freeFilter(f);
> +        return NULL;
> +    }

I'd prefer to have something like:

int parse_vector(SwsVector **vec, enun VectorType *vec_type, char **buf, void *log_ctx) {...}

int parse_filter(SwsFilter **filt, const char *str, void *log_ctx)
{
   SwsFilter *f = alloc_filter();
   char *p = str;
   while (p && *p) {
       SwsVector *v;
       ret = parse_vector(&v, ...);
       if (ret < 0) ERROR;
       switch (vec_type) {
           LH: filt->lumH = v; break;
           LV: filt->lumV = v; break;
           ...
       }
   }
   ...
   *filt = f;
   return 0;
   ...
}

[...]

I volunteer to write the parsing code during the weekend if you don't
want to do it yourself.
-- 
FFmpeg = Fostering and Fabulous Mere Porno Evanescent Glue