[FFmpeg-devel] [PATCH] swscale: Make sws_alloc_set_opts() public

[Michael Niedermayer]

On Sat, Aug 08, 2015 at 06:49:10PM +0200, wm4 wrote:
> On Sat, 8 Aug 2015 18:17:37 +0200
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Sat, Aug 08, 2015 at 05:58:29PM +0200, wm4 wrote:
> > > On Sat,  8 Aug 2015 17:14:58 +0200
> > > Michael Niedermayer <michaelni at gmx.at> wrote:
> > > 
> > > > From: Michael Niedermayer <michael at niedermayer.cc>
> > > > 
> > > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > > ---
> > > >  libswscale/swscale.h          |   11 +++++++++++
> > > >  libswscale/swscale_internal.h |   11 -----------
> > > >  libswscale/version.h          |    4 ++--
> > > >  3 files changed, 13 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> > > > index 903e120..055c9fc 100644
> > > > --- a/libswscale/swscale.h
> > > > +++ b/libswscale/swscale.h
> > > > @@ -161,6 +161,17 @@ int sws_isSupportedEndiannessConversion(enum AVPixelFormat pix_fmt);
> > > >  struct SwsContext *sws_alloc_context(void);
> > > >  
> > > >  /**
> > > > + * Allocate and return an SwsContext.
> > > > + * This is like sws_getContext() but does not perform the init step, allowing
> > > > + * the user to set additional AVOptions.
> > > > + *
> > > > + * @see sws_getContext()
> > > > + */
> > > > +struct SwsContext *sws_alloc_set_opts(int srcW, int srcH, enum AVPixelFormat srcFormat,
> > > > +                                      int dstW, int dstH, enum AVPixelFormat dstFormat,
> > > > +                                      int flags, const double *param);
> > > > +
> > > 
> > > This looks excessively useless and un-nice to use.
> > 
> > what do you suggest ?
> > 
> > The function is intended to replace:
> > 
> > context = sws_alloc_context()
> > if (!context)
> >     handle error
> > 
> > ret0 = av_opt_set_int(context, "sws_flags",  flags, 0);
> > ret1 = av_opt_set_int(context, "srcw",       srcW, 0);
> > ret2 = av_opt_set_int(context, "srch",       srcH, 0);
> > ret3 = av_opt_set_int(context, "dstw",       dstW, 0);
> > ret4 = av_opt_set_int(context, "dsth",       dstH, 0);
> > ret5 = av_opt_set_int(context, "src_format", srcFormat, 0);
> > ret6 = av_opt_set_int(context, "dst_format", dstFormat, 0);
> > ret7 = av_opt_set(context, "alphablend", "none", 0);
> 
> This does look better to me than a function which has some required
> parameters (and at least 1 optional one!), but not all parameters which
> are reasonably needed for general pixfmt conversion.

> Where are the
> input and output colorspaces and ranges set?

they would be set with sws_setColorspaceDetails()


> 
> The "param" parameter is the worst part if this proposed function. What
> does it even mean? I don't know because I didn't read the
> implementation.

I was not aware of it being undocumented, ill fix that


[...]
> 
> There's my very old patch, which made libswscale transparently set the
> parameters of an input AVFrame. This would actually be the simplest API.

can you update the patch so the comments raised by stefano are taken
care of ?


[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150808/dbaf32c7/attachment.sig>