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

Stefano Sabatini stefasab at gmail.com
Sun Oct 13 20:57:40 CEST 2013


On date Friday 2013-10-11 16:45:43 +0200, Michael Niedermayer encoded:
> On Fri, Oct 11, 2013 at 02:41:50PM +0200, Stefano Sabatini wrote:
> > 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.
> 
> fine with me
> 
> 
> > 
> > > 
> > > 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
> 
> all that is required for the parser in its current form to work
> 
> there are 4 filters, luma horizontal, vertical and chroma horizontal
> and vertical
> 
> filter specifications can be for all 4, for 2 (like luma h+v or for
> luma+chroma horizontal for example) or for just one specific
> filter.
> If there are multiple overlapping specifications, the most specific
> is used for each of the 4 filters
> This way you can specify a default and override 1 specific case to
> get filter A on 3 and B on 1 case.
> 
> a use case would be to sharpen luma and shift chroma by 1 pixel left
> to compensate some error
> (this would need 2 filters to be written while 3 of the 4 would be
>  set)
> iam sure there exist many such use cases ...
> 
> also multiple specifications of a filter possibly could/should be
> combined by convolution to actually apply both filters
> 
> 
> > 
> > > +    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.
> 
> do whatever you prefer as long as it doesnt loose features and isnt
> more messy than mine
> 
> Also please check with wm4, iam not sure what he is working on and
> if that includes a filter string parser

Incomplete and partially untested, for your comments.

Result of the test is:

Incomplete filter specification in '', components lumH lumV chrH chrV are not specified
 -> error
Vector specification '1+2' contains trailing invalid data
1+2 -> error
1 -> lumH:1.000000 lumV:1.000000 chrH:1.000000 chrV:1.000000
h1,2,3v2,3,4 -> lumH:1.000000,2.000000,3.000000 lumV:2.000000,3.000000,4.000000 chrH:1.000000,2.000000,3.000000 chrV:2.000000,3.000000,4.000000
0.2,0.123 -> lumH:0.200000,0.123000 lumV:0.200000,0.123000 chrH:0.200000,0.123000 chrV:0.200000,0.123000
Vector specification '1,23,45,343+3' contains trailing invalid data
1,23,45,343+3 -> error
lh1,2,3c4,5.123 -> lumH:1.000000,2.000000,3.000000 lumV:1.000000,2.000000,3.000000 chrH:4.000000,5.123000 chrV:4.000000,5.123000
Incomplete filter specification in 'lh1,2,3', components chrV are not specified
lh1,2,3 -> error
Incomplete filter specification in 'hl1,2,3', components chrV are not specified
hl1,2,3 -> error
Could not parse element 'foo' in vector 'foo' as a number
foo -> error
-- 
FFmpeg = Faithful & Faithful Mind-dumbing Political Emblematic Genius
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-swscale-support-setting-filters-through-AVOptions.patch
Type: text/x-diff
Size: 11751 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131013/0a6e7c45/attachment.bin>


More information about the ffmpeg-devel mailing list