[FFmpeg-devel] [PATCH] Yet another stab at RGB48 support

[Michael Niedermayer]

On Tue, May 19, 2009 at 08:32:53PM +0300, Kostya wrote:
> On Tue, May 19, 2009 at 04:02:44AM +0200, Michael Niedermayer wrote:
> > On Sat, May 16, 2009 at 09:24:13AM +0300, Kostya wrote:
[...]
> 
> > [...]
> > 
> > /------------------------------------------------\
> > 
> > > @@ -299,6 +301,8 @@
> > >  #define is16BPS(x)      (           \
> > >             (x)==PIX_FMT_GRAY16BE    \
> > >          || (x)==PIX_FMT_GRAY16LE    \
> > > +        || (x)==PIX_FMT_RGB48BE     \
> > > +        || (x)==PIX_FMT_RGB48LE     \
> > >          || (x)==PIX_FMT_YUV420PLE   \
> > >          || (x)==PIX_FMT_YUV422PLE   \
> > >          || (x)==PIX_FMT_YUV444PLE   \
> > 
> > \------------hunk ok-----------------------------/
> > 
> > 
> > [...]
> > 
> > /------------------------------------------------\
> > 
> > >  #define isBGR(x)        (           \
> > >             (x)==PIX_FMT_BGR32       \
> > > @@ -376,6 +382,9 @@
> > >  static inline int fmt_depth(int fmt)
> > >  {
> > >      switch(fmt) {
> > > +        case PIX_FMT_RGB48BE:
> > > +        case PIX_FMT_RGB48LE:
> > > +            return 48;
> > >          case PIX_FMT_BGRA:
> > >          case PIX_FMT_ABGR:
> > >          case PIX_FMT_RGBA:
> > 
> > \------------hunk ok-----------------------------/
>  
> I suppose adding it to sws_format_name() is also ok.

yes


>  
> > [...]
> > > +SwsFunc sws_hires_gray16rgbConv(SwsContext *c)
> > > +{
> > > +    const enum PixelFormat srcFormat = c->srcFormat;
> > > +    const enum PixelFormat dstFormat = c->dstFormat;
> > > +
> > > +    if (isGray16(srcFormat) && (dstFormat == PIX_FMT_RGB48BE ||
> > > +                                dstFormat == PIX_FMT_RGB48LE)) {
> > > +        return gray16torgb48;
> > > +    }
> > > +    if (isGray16(dstFormat) && (srcFormat == PIX_FMT_RGB48BE ||
> > > +                                srcFormat == PIX_FMT_RGB48LE)) {
> > > +        if (dstFormat == PIX_FMT_GRAY16BE) {
> > > +            if (srcFormat == PIX_FMT_RGB48BE)
> > > +                return rgb48betogray16be;
> > > +            else
> > > +                return rgb48letogray16be;
> > > +        } else {
> > > +            if (srcFormat == PIX_FMT_RGB48BE)
> > > +                return rgb48betogray16le;
> > > +            else
> > > +                return rgb48letogray16le;
> > > +        }
> > > +    }
> > > +
> > > +    return NULL;
> > > +}
> > 
> > the if / else looks messy why are this not 2 functions?
> > 
> > also dont forget to test the code with swscale-example, that is all new
> > convertions should work and have small differences to the correct image
>  
> I've tested it on 48-bpp TIFF image, conversions to PNG and JPEG went
> fine, will test a bit more.

you can test all ~1000 cases by hand too if you dont want to use
swscale_example, but testing just 2 cases is not enough

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090520/d9e88581/attachment.pgp>