[FFmpeg-devel] [PATCH] Yet another stab at RGB48 support
Michael Niedermayer
michaelni
Sat May 23 03:47:46 CEST 2009
On Fri, May 22, 2009 at 08:31:41AM +0300, Kostya wrote:
> On Fri, May 22, 2009 at 03:08:56AM +0200, Michael Niedermayer wrote:
> > On Thu, May 21, 2009 at 08:51:44PM +0300, Kostya wrote:
> > > 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).
> >
> > all cases must work not only the default flags
>
> it tests conversions for flags 1, 2, 4, 8, 16 and 32
so |SWS_ACCURATE_RND to that
then | SWS_BITEXACT to it
>
> > >
> > > [...]
> > > >
> > > > > @@ -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
> >
> > > swscale.c | 2 +
> > > swscale_internal.h | 4 ++-
> > > swscale_template.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 60 insertions(+), 1 deletion(-)
> > > 8b0dea176685da1b215ef85dafde3594459d0c5c 48bpp-in.patch
> > > Index: swscale.c
> > > ===================================================================
> > > --- swscale.c (revision 29318)
> > > +++ swscale.c (working copy)
> > > @@ -108,6 +108,8 @@ unsigned swscale_version(void)
> > > || (x)==PIX_FMT_YUVA420P \
> > > || (x)==PIX_FMT_YUYV422 \
> > > || (x)==PIX_FMT_UYVY422 \
> > > + || (x)==PIX_FMT_RGB48BE \
> > > + || (x)==PIX_FMT_RGB48LE \
> > > || (x)==PIX_FMT_RGB32 \
> > > || (x)==PIX_FMT_RGB32_1 \
> > > || (x)==PIX_FMT_BGR24 \
> > > Index: swscale_internal.h
> > > ===================================================================
> > > --- swscale_internal.h (revision 29318)
> > > +++ swscale_internal.h (working copy)
> > > @@ -344,7 +344,9 @@ const char *sws_format_name(int format);
> > > || (x)==PIX_FMT_GRAY16LE \
> > > )
> > > #define isRGB(x) ( \
> > > - (x)==PIX_FMT_RGB32 \
> > > + (x)==PIX_FMT_RGB48BE \
> > > + || (x)==PIX_FMT_RGB48LE \
> > > + || (x)==PIX_FMT_RGB32 \
> >
> > you can add these with fewer changed lines
>
> Indeed, but I have the impression those are ordered by decreasing depth.
hmmm, well, ok
>
> > > || (x)==PIX_FMT_RGB32_1 \
> > > || (x)==PIX_FMT_RGB24 \
> > > || (x)==PIX_FMT_RGB565 \
> >
> > > Index: swscale_template.c
> > > ===================================================================
> > > --- swscale_template.c (revision 29318)
> > > +++ swscale_template.c (working copy)
> > > @@ -2130,6 +2130,48 @@ static inline void RENAME(monoblack2Y)(uint8_t *ds
> > > }
> > > }
> > >
> > > +static inline void RENAME(rgb48ToY)(uint8_t *dst, const uint8_t *src, int width)
> > > +{
> > > + int i;
> > > + for (i = 0; i < width; i++) {
> > > + int r = src[i*6+0];
> > > + int g = src[i*6+2];
> > > + int b = src[i*6+4];
> > > +
> > > + dst[i] = ((RY*r + GY*g + BY*b + (33<<(RGB2YUV_SHIFT-1))) >> RGB2YUV_SHIFT);
> > > + }
> > > +}
> >
> > these functions will be duplicated as swscale_template is included multiple
> > times
>
> Theoretically someone can adapt MMX counterpart from rgb24to{Y,UV}
> Anyway, what do you suggest to avoid that?
if its not building different variants then t doesnt belong in the template
> That's an issue with most of
> non-YUV functions there.
sws needs some cleanup ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- 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/20090523/00351f6b/attachment.pgp>
More information about the ffmpeg-devel
mailing list