[FFmpeg-devel] [RFC] Make swscale-test perform only one convertion

[Stefano Sabatini]

On date Thursday 2010-02-18 09:29:19 +0100, Stefano Sabatini encoded:
> On date Thursday 2010-02-18 04:10:44 -0200, Ramiro Polla encoded:
> > On Thu, Feb 18, 2010 at 2:25 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
> > > swscale-test is currently crashing in Windows in ./swscale-test.exe
> > > rgb24 argb. Apparently due to heap corruption (that's what windows
> > > says), but I couldn't find out the exact cause yet.
> > 
> > It crashes on:
> >  rgb24 96x96 -> argb   96x  96 flags=16
> > 
> > While trying to free dst in swscale-test.c:doTest()
> >     for (i=0; i<4; i++) {
> >         av_free(src[i]);
> >         av_free(dst[i]);
> >         av_free(out[i]);
> >     }
> > 
> > The function rgb24tobgr32 has overwritten data past the end of its
> > destination pointer because swscale.c:rgbToRgbWrapper() checks:
> >         if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat ==
> > PIX_FMT_BGR32_1) && !isRGBA32(srcFormat))
> >             dstPtr += ALT32_CORR;
> > and increments dstPtr.
> > 
> > Stefano, I see that you have added that !isRGBA32() there. Could you
> > double check if this is correct for all cases? Shouldn't it actually
> > be:
> > 
> > Index: swscale.c
> > ===================================================================
> > --- swscale.c	(revision 30631)
> > +++ swscale.c	(working copy)
> > @@ -1502,10 +1502,10 @@
> >      } else {
> >          const uint8_t *srcPtr= src[0];
> >                uint8_t *dstPtr= dst[0];
> > -        if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat ==
> > PIX_FMT_BGR32_1) && !isRGBA32(dstFormat))
> > +        if ((srcFormat == PIX_FMT_RGB32_1 || srcFormat ==
> > PIX_FMT_BGR32_1) && !isRGBA32(srcFormat))
> >              srcPtr += ALT32_CORR;
> > 
> > -        if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat ==
> > PIX_FMT_BGR32_1) && !isRGBA32(srcFormat))
> > +        if ((dstFormat == PIX_FMT_RGB32_1 || dstFormat ==
> > PIX_FMT_BGR32_1) && !isRGBA32(dstFormat))
> >              dstPtr += ALT32_CORR;
> > 
> >          if (dstStride[0]*srcBpp == srcStride[0]*dstBpp && srcStride[0] > 0)
> 
> No this is wrong.
> 
> But I see there is a problem here, we have rgb24to32 writing over the
> end of the slice. Maybe we need to implement some specific function
> for dealing with that case (BTW we are also skipping to write the
> first byte of the ARGB array).

Please try the attached patch.

BTW we should find some way to bypass the rgbToRgbWrapper and cache
conv function once and for all.

Regards.
-- 
FFmpeg = Fostering Fascinating Muttering Peaceful Everlasting Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-rgb24-to32-conv.patch
Type: text/x-diff
Size: 5949 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100219/0647edad/attachment.patch>