[FFmpeg-devel] [PATCH] swscale: add API to convert AVFrames directly

[Ronald S. Bultje]

Hi,

On Mon, Sep 30, 2013 at 11:39 AM, Michael Niedermayer <michaelni at gmx.at>wrote:

> On Sun, Sep 29, 2013 at 08:21:35PM +0200, wm4 wrote:
> > On Sun, 29 Sep 2013 17:50:52 +0200
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> [...]
> > >
> > >
> > > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> > > > index 42702b7..60733f0 100644
> > > > --- a/libswscale/swscale.h
> > > > +++ b/libswscale/swscale.h
> > > > @@ -39,6 +39,8 @@
> > > >  #include "libavutil/pixfmt.h"
> > > >  #include "version.h"
> > > >
> > > > +struct AVFrame;
> > > > +
> > > >  /**
> > > >   * Return the LIBSWSCALE_VERSION_INT constant.
> > > >   */
> > > > @@ -169,6 +171,81 @@ struct SwsContext *sws_alloc_context(void);
> > > >  int sws_init_context(struct SwsContext *sws_context, SwsFilter
> *srcFilter, SwsFilter *dstFilter);
> > > >
> > > >  /**
> > > > + * Set source filter. This works only with
> sws_reinit_cached_context() and
> > > > + * sws_scale_frame().
> > > > + *
> > > > + * @return zero or positive value on success, a negative value on
> > > > + * error
> > > > + */
> > > > +int sws_set_src_filter(struct SwsContext *sws_context, SwsFilter
> *srcFilter);
> > > > +
> > > > +/**
> > > > + * Set destination filter. This works only with
> sws_reinit_cached_context() and
> > > > + * sws_scale_frame().
> > > > + *
> > > > + * @return zero or positive value on success, a negative value on
> > > > + * error
> > > > + */
> > > > +int sws_set_dst_filter(struct SwsContext *sws_context, SwsFilter
> *dstFilter);
> > >
> > > would it make sense to allow seting the filter via AVOption as a
> > > char* ?
> > > iam wondering because this maybe would avoid these 2 functions
> >
> > SwsFilter is a struct with 4 SwsVector members, so I'm not sure how
> > that would work. Also, I need to know if the filter changes. These
> > functions basically set a flag that they changed, and if they use the
> > AVOption API, I'd have to compare the SwsFilters with the old state to
> > detect a change.
>
> the char * string could allow something like
>
> "1,2,1" (simple lowpass applied to luma & chroma & horizontal and vertical)
> "v:1,2,1" same lowpass applied only to vertical luma & chroma that is poor
> deinterlace
> "lh:0,0,1,ch:0,0.5,0.5" horizontal shift luma by 1 pixelm chroma by half a
> pixel
>
> thats just a quick and random idea for a syntax and probably a bit of topic
> so its more for consideration of future design direction ...
>
> one advantage of such string based system with AVOptions is that it
> shouldnt require any specific code in a user application that supports
> avoptions
>
>
> >
> > >
> > > [...]
> > > > +static void sws_set_src_frame_options(struct SwsContext *c, struct
> AVFrame *src)
> > > > +{
> > > > +    c->srcFormat = src->format;
> > > > +    c->srcW      = src->width;
> > > > +    c->srcH      = src->height;
> > > > +    c->srcRange  = src->color_range;
> > > > +    sws_set_avframe_colorspace_table(c->srcColorspaceTable,
> src->colorspace);
> > > > +}
> > > > +
> > > > +static void sws_set_dst_frame_options(struct SwsContext *c, struct
> AVFrame *dst)
> > > > +{
> > > > +    c->dstFormat = dst->format;
> > > > +    c->dstW      = dst->width;
> > > > +    c->dstH      = dst->height;
> > > > +    c->dstRange  = dst->color_range;
> > > > +    sws_set_avframe_colorspace_table(c->dstColorspaceTable,
> dst->colorspace);
> > > > +}
> > >
> > > static functions dont need sws_ prefixes, if you want to shorten them
> >
> > I removed it from some functions. I left it for others, because maybe
> > they should actually be public.
> >
> > By the way, I'm not all that sure whether my patch is correct.
> > libswscale is very scary, and there could be some things I missed. In
> > particular, there's the question whether sws_uninit_context followed
> > sws_init_context really does full reinitialization, or if there are
> > additional members that must be reset in sws_uninit_context. (Members
> > which are set only by some init code paths, but are read by everything
> > would belong into this category.)
>
> you could try to memset the context to some non 0 value on alloc
> and clear just the values from uninit and then try fate
>
>
> [...]
> >  options.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > ca2e03fcb8ed9b12e9a96e1d3e97e238dcdf9eff
>  0001-swscale-make-bilinear-scaling-the-default.patch
> > From 06ec0cacb9e59c009e61b920c7ac645f5c9fda46 Mon Sep 17 00:00:00 2001
> > From: wm4 <nfxjfg at googlemail.com>
> > Date: Sun, 29 Sep 2013 19:52:37 +0200
> > Subject: [PATCH 1/2] swscale: make bilinear scaling the default
> >
> > Before this commit, sws_init_context() failed with an error if no scaler
> > was explicitly set.
> >
> > Defaulting to something reasonable is better behavior.
> > ---
> >  libswscale/options.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libswscale/options.c b/libswscale/options.c
> > index 8985e6b..2b3147b 100644
> > --- a/libswscale/options.c
> > +++ b/libswscale/options.c
> > @@ -34,7 +34,7 @@ static const char *sws_context_to_name(void *ptr)
> >  #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> >
> >  static const AVOption swscale_options[] = {
> > -    { "sws_flags",       "scaler flags",
>  OFFSET(flags),     AV_OPT_TYPE_FLAGS,  { .i64 = DEFAULT            }, 0,
>     UINT_MAX,       VE, "sws_flags" },
> > +    { "sws_flags",       "scaler flags",
>  OFFSET(flags),     AV_OPT_TYPE_FLAGS,  { .i64  = SWS_BILINEAR       }, 0,
>       UINT_MAX,       VE, "sws_flags" },
> >      { "fast_bilinear",   "fast bilinear",                 0,
>       AV_OPT_TYPE_CONST,  { .i64  = SWS_FAST_BILINEAR  }, INT_MIN, INT_MAX,
>        VE, "sws_flags" },
> >      { "bilinear",        "bilinear",                      0,
>       AV_OPT_TYPE_CONST,  { .i64  = SWS_BILINEAR       }, INT_MIN, INT_MAX,
>        VE, "sws_flags" },
> >      { "bicubic",         "bicubic",                       0,
>       AV_OPT_TYPE_CONST,  { .i64  = SWS_BICUBIC        }, INT_MIN, INT_MAX,
>        VE, "sws_flags" },
> > --
>
> applied


Bilinear seems kind of ... poor as a default?

Ronald