[FFmpeg-devel] Extend/optimize RGB to RGB conversions funcsintorgb2rgb.c

[yann.lepetitcorps at free.fr]

Thanks for your contribution

I have tested it and found that the conversion isn't valid :(

void rgb24to32_uint32(const uint8_t *src, uint8_t *dst, int src_size )
{
   int nPixels = src_size / 3;
   int pixels4 = nPixels >> 2;
   int extra = nPixels % 4;
   uint32_t * pDst =  (uint32_t*)dst;
   uint32_t * pSrc = (uint32_t*)src;
   uint8_t* pBytes;
   int i;

   for ( i = 0; i < pixels4; ++i)    {
#if HAVE_BIGENDIAN
       pDst[0] = 0xFF000000 | (pSrc[0] >> 8);
       pDst[1] = 0xFF000000 | (pSrc[0] << 16) | (pSrc[1] >> 16);
       pDst[2] = 0xFF000000 | (pSrc[1] << 8) | (pSrc[2] >> 24);
       pDst[3] = 0xFF000000 | pSrc[2];
#else
       pDst[0] = 0xFF000000 | pSrc[0];
       pDst[1] = 0xFF000000 | (pSrc[1] << 8) | (pSrc[0] >> 24);
       pDst[2] = 0xFF000000 | (pSrc[2] << 16) | (pSrc[1] >> 16);
       pDst[3] = 0xFF000000 | (pSrc[2] >> 8);
#endif
       pDst +=4;
       pSrc +=3;
    }

    pBytes = (uint8_t*)pSrc;

  for ( i = 0; i < extra; i++)    {
#if HAVE_BIGENDIAN
       *pDst++ = 0xFF000000 | (pBytes[0] << 16) | (pBytes[1] << 8) |
(pBytes[2]);
#else
       *pDst++ = 0xFF000000 | (pBytes[2] << 16) | (pBytes[1] << 8) |
(pBytes[0]);
#endif
       pBytes += 3;
   }
}


Because it give this in my procedure test :

Test original rgb24to32() func : 477 ms
Test new rgb24to32() func : 474 ms
R components of entry 0 aren't the sames (51 vs 223) :(
B components of entry 0 aren't the sames (223 vs 51) :(
R components of entry 1 aren't the sames (46 vs 50) :(
B components of entry 1 aren't the sames (50 vs 46) :(
R components of entry 2 aren't the sames (205 vs 188) :(
B components of entry 2 aren't the sames (188 vs 205) :(
R components of entry 3 aren't the sames (146 vs 87) :(
B components of entry 3 aren't the sames (87 vs 146) :(
R components of entry 4 aren't the sames (109 vs 35) :(
B components of entry 4 aren't the sames (35 vs 109) :(
R components of entry 5 aren't the sames (229 vs 92) :(
B components of entry 5 aren't the sames (92 vs 229) :(

=> we have a very little gain but the conversion is false :(
    (my procedure test automatically exit when it found more than 10 errors)
    [but each loop work with 3x components tests, so this make 3x4 = 12 errors
before to automaticaly  exit]

My procedure test is outside my FFMPEG git repertory, so I put the source code
of this test procedure as an attachment


@+
Yannoo


Selon Don Moir <donmoir at comcast.net>:

>
> ----- Original Message -----
> From: "Don Moir" <donmoir at comcast.net>
> To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
> Sent: Monday, September 10, 2012 3:48 PM
> Subject: Re: [FFmpeg-devel] Extend/optimize RGB to RGB conversions
> funcsintorgb2rgb.c
>
>
> >> void rgb24to32(const uint8_t *src, uint8_t *dst, int src_size )
> >> {
> >>    int i;
> >>    uint8_t *psrc = src;
> >>
> >>    for ( i = 0 ; i < src_size ; i += 3, psrc +=3, dst +=4 )
> >>    {
> >> #if HAVE_BIGENDIAN
> >>        /* RGB24 (= R,G,B) -> BGR32 (= 255,R,G,B) */
> >>        dst[0] = 255;
> >>        dst[1] = psrc[0];
> >>        dst[2] = psrc[1];
> >>        dst[3] = psrc[2];
> >> #else
> >>        dst[0] = psrc[2];
> >>        dst[1] = psrc[1];
> >>        dst[2] = psrc[0];
> >>        dst[3] = 255;
> >> #endif
> >>    }
> >> }
> >
> > You might try something like this that does 4 pixels within the loop. It
> > might be interesting to see if performance is better for this. I do it asm
> > and don't do it line by line for my own purposes.
> >
> > Note: somewhat pseudo code. I do it differently so modified here.
> >
> > void rgb24to32(const uint8_t *src, uint8_t *dst, int src_size )
> > {
> >    int nPixels = src_size / 3;
> >    int pixels4 = nPixels >> 2;
> >    int extra = nPixels % 4;
> >    uint32_t* pDst (uint32_t*)dst;
> >    uint32_t* pSrc (uint32_t*)src;
> >
> >    for (int i = 0; i < pixels4; ++i)
> >    {
> > #if HAVE_BIGENDIAN
> >        pDst[0] = 0xFF000000 | (pSrc[0] >> 8);
> >        pDst[1] = 0xFF000000 | (pSrc[0] << 16) | (pSrc[1] >> 16);
> >        pDst[2] = 0xFF000000 | (pSrc[1] << 8) | (pSrc[2] >> 24);
> >        pDst[3] = 0xFF000000 | pSrc[2];
> > #else
> >        pDst[0] = 0xFF000000 | pSrc[0];
> >        pDst[1] = 0xFF000000 | (pSrc[1] << 8) | (pSrc[0] >> 24);
> >        pDst[2] = 0xFF000000 | ((pSrc[2] << 16) | (pSrc[1] >> 16);
> >        pDst[3] = 0xFF000000 | (pSrc[2] >> 8);
> > #endif
> >        pDst +=4;
> >        pSrc +=3;
> >    }
> >
>
> Sorry mistake:
>
> -    uint8_t* pBytes = (uint8_t*)pDst;
> +   uint8_t* pBytes = (uint8_t*)pSrc;
>
> >    for (int i = 0; i < extra; i++)
> >    {
> > #if HAVE_BIGENDIAN
> >        *pDst++ = 0xFF000000 | (pBytes[0] << 16) | (pBytes[1] << 8) |
> > (pBytes[2]);
> > #else
> >        *pDst++ = 0xFF000000 | (pBytes[2] << 16) | (pBytes[1] << 8) |
> > (pBytes[0]);
> > #endif
> >        pBytes += 3;
> >    }
> > }
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: test_rgb2rgba.c
Type: text/x-csrc
Size: 7074 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120910/02cd91a3/attachment.bin>