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

[Michael Niedermayer]

On Thu, Sep 30, 2010 at 12:56:20AM +0200, Stefano Sabatini wrote:
> 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.

YUVJ* is deprecated
so designing new code toward it doesnt feel very good at all

also i dont see a problem
if YUVJ set Range appropriately
if not dont touch Range and leave it at the default / what the usert did set

[...]
> > > +
> > > +    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()...

i think we can remove this emms



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

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100930/0bfa2f31/attachment.pgp>