[FFmpeg-devel] [PATCH] Move param initialization and colorspace details from sws_getContext() to sws_init_context().

[Stefano Sabatini]

On date Wednesday 2010-09-29 19:25:28 +0200, Michael Niedermayer encoded:
> On Wed, Sep 29, 2010 at 06:00:27PM +0200, Stefano Sabatini wrote:
> > Allow to automatically set the default parameters without explicitely
> > set them, also simplify conversion to the new sws_init_context() API.
> > ---
> >  utils.c |   18 +++++++-----------
> >  1 files changed, 7 insertions(+), 11 deletions(-)
> > 
> > diff --git a/utils.c b/utils.c
> > index 938af9e..bee5a3a 100644
> > --- a/utils.c
> > +++ b/utils.c
> > @@ -761,10 +761,17 @@ int sws_init_context(SwsContext *c, SwsFilter *srcFilter, SwsFilter *dstFilter)
> >      int dstW= c->dstW;
> >      int dstH= c->dstH;
> >      int flags;
> > +    c->srcRange = handle_jpeg(&c->srcFormat);
> > +    c->dstRange = handle_jpeg(&c->dstFormat);
> 
> that breaks user setted *Range

I'm aware but I cannot see a simpler way. A possibility would be to
introduce an hack external to this function, but then doesn't look
that good as well.

The problem is: how should the new API deal with JPEG colorspaces?
Currently when we want to use the JPEG colorspace we pass one of the
YUVJ* pixel formats. Avoiding this hack would mean to make the various
FFmpeg components (well, mainly codecs) explicitely define which
colorspace/range to use.

My suggestion is to simply discard the problem for the moment, when
we'll have the ideas more clear about how this should be implemented
we can change it to a sane behavior, I don't want to fall again in the
usual ad-infinitum API revisiting process.

> >      enum PixelFormat srcFormat= c->srcFormat;
> >      enum PixelFormat dstFormat= c->dstFormat;
> >  
> >      flags= c->flags = update_flags_cpu(c->flags);
> > +    if (!c->param[0]) c->param[0] = SWS_PARAM_DEFAULT;
> > +    if (!c->param[1]) c->param[1] = SWS_PARAM_DEFAULT;
> 
> that breaks setting the params to 0

That should be already addressed.
 
> > +
> > +    sws_setColorspaceDetails(c, ff_yuv2rgb_coeffs[SWS_CS_DEFAULT], c->srcRange, ff_yuv2rgb_coeffs[SWS_CS_DEFAULT] /* FIXME*/, c->dstRange, 0, 1<<16, 1<<16);
> > +
> >  #if ARCH_X86
> >      if (flags & SWS_CPU_CAPS_MMX)
> >          __asm__ volatile("emms\n\t"::: "memory");
> 
> doing that before emms sounds like a really stupid idea, are you sure theres
> no float code in there and never will be ...

Uh? For the very little that I know emms re-allows FPU instruction use
after some MMX instructions were used, in this case I don't see why
this would be a proble, since sws_setColorspaceDetails() is called
*before* this. Also I'm simply keeping the same sequential order as in
the original sws_getContext()...

Regards.
-- 
FFmpeg = Fiendish and Formidable Mythic Portentous Ecumenical Geek