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

[Ramiro Polla]

On Thu, Feb 18, 2010 at 9:23 PM, Stefano Sabatini
<stefano.sabatini-lala at poste.it> wrote:
> 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.

Things now crash at:
rgb565le 96x96 -> abgr   96x  96 flags= 1 SSD=   11,    3,    4,    0

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

A function pointer to SwsContext should be added. The offset should be
done similarly to lumSrcOffset (it might even be reusable).