[FFmpeg-devel] [PATCH] swscale: Support setting filters through AVOptions
Michael Niedermayer
michaelni at gmx.at
Fri Oct 11 16:45:43 CEST 2013
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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Avoid a single point of failure, be that a person or equipment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131011/7de3317d/attachment.asc>
More information about the ffmpeg-devel
mailing list