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

Kostya kostya.shishkov
Thu May 21 19:51:44 CEST 2009


On Thu, May 21, 2009 at 04:37:09PM +0200, Michael Niedermayer wrote:
> On Wed, May 20, 2009 at 06:05:28PM +0300, Kostya wrote:
> > On Wed, May 20, 2009 at 01:07:36AM +0200, Michael Niedermayer wrote:
> > > 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:
> > [...]
> > > >  
> > > > 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
> >  
> > Okay,  here it is - tested, seems to work fine.
> 
> have you also tried swscale_example with SWS_ACCURATE_RND / SWS_BITEXACT?
 
I've tried it with all the flags it uses by default, error seems to be
low (i.e. one-digit number).

[...]
> 
> > @@ -2176,6 +2202,8 @@ static int planarCopy(SwsContext *c, uint8_t* src[
> >              fillPlane(dst[plane], dstStride[plane], length, height, y, (plane==3) ? 255 : 128);
> >          }else
> >          {
> > +            if (isRGB(c->srcFormat))
> > +                length *= isALPHA(c->srcFormat) ? 4 : 3;
> >              if(is16BPS(c->srcFormat) && !is16BPS(c->dstFormat)){
> >                  if (!isBE(c->srcFormat)) srcPtr++;
> >                  for (i=0; i<height; i++){
> 
> iam sorry but that goes too far
> some people say sws is a mess or at least undocumented and thus hard to
> understand but what you do here is beyond the worst code in sws.
> 
> The function is called planarCopy() that implicates planar data, and you
> simply use it to copy packed pixel formats and randomly hack it so it
> halfway works
> 
> I think it should go without saying that the name of a function has to match
> what it does and that all non obvious tricks must be documented.
> Also if for whatever reason you wish to merge packed and planarCopy that
> would be a seperate patch with a few words of why the merge is a good idea/
> needed
> 
> similarly yuv2rgb48 should be a seperate patch and the basic rgb48 support
> should be seperate from the unscaled special converteres for rgb48

What about this patch for RGB48 -> YUV conversion?
Also YUV -> RGB48 is easy too but rgb2rgb stuff is PITA to integrate
in clean and logical way and will require a great deal more time to do
it properly.
 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 48bpp-in.patch
Type: text/x-diff
Size: 4899 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090521/1e81d803/attachment.patch>



More information about the ffmpeg-devel mailing list