[FFmpeg-devel] need help fixing sws on PPC
Vitor Sessak
vitor1001
Tue Oct 7 21:24:26 CEST 2008
Michael Niedermayer escreveu:
> On Mon, Oct 06, 2008 at 09:42:56PM +0200, Vitor Sessak wrote:
>
>> Michael Niedermayer escreveu:
>>
>>> On Wed, Sep 10, 2008 at 01:42:44PM +0200, Vitor Sessak wrote:
>>>
>>>
>>>> Michael Niedermayer escreveu:
>>>>
>>>>
>>>>> Hi
>>>>>
>>>>> Can someone who has a PPC and a x86 play a little with swscale to find
>>>>> out which convertions do not match?
>>>>> This is rather important as our regression tests as well as fate depend
>>>>> on it.
>>>>> Note -sws_flags +bitexact should be used also -sws_flags +accurate_rnd
>>>>> may be needed.
>>>>> Also note, some scalers still use floats to find coeffs (like spline)
>>>>> iam not interrested in them, that is currently area, nearest neighbor
>>>>> bilinear and bicubic are the onlx ones iam interrested in.
>>>>>
>>>>> And last, if something does not match, pleas explain how it does differ
>>>>> +-1 rounding or red/blue exchanged or ...
>>>>>
>>>>>
>>>> Besides rounding, there is still something wierd about swscale. For
>>>> example, I was looking at he idcin test
>>>> http://fate.multimedia.cx/index.php?test_spec=107 .
>>>>
>>>>
>>> [...]
>>>
>>>
>>>> Unless I am missing something, there should be no calculation (and even
>>>> less rounding) doing PAL8 -> RGB conversion, but it still gives different
>>>> output with and without swscaler.
>>>>
>>>>
>>>
>>>
>> [...]
>>
>>
>>> if you only care about the unscaled pal->rgb24/32 case, you can add a
>>> special
>>> converter for that case, which should be trivial.
>>>
>>>
>> Here is my try at it. I really think it is important to be able to test
>> palette-based decoders without doing a full regression test of all asm
>> versions of rgb2yuv/yuv2rgb...
>>
>> -Vitor
>>
>
>
>> Index: libswscale/swscale.c
>> ===================================================================
>> --- libswscale/swscale.c (revis?o 27716)
>> +++ libswscale/swscale.c (c?pia de trabalho)
>> @@ -1711,6 +1711,40 @@
>> return srcSliceH;
>> }
>>
>> +static int pal2rgbWrapper(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
>> + int srcSliceH, uint8_t* dst[], int dstStride[]){
>> + const int srcFormat= c->srcFormat;
>> + const int dstFormat= c->dstFormat;
>> + void (*conv)(const uint8_t *src, uint8_t *dst, long num_pixels,
>> + const uint8_t *palette)=NULL;
>> +
>>
>
>
>> + if ((!(isRGB(dstFormat) || isBGR(dstFormat)) || srcFormat !=PIX_FMT_PAL8))
>> + av_log(c, AV_LOG_ERROR, "internal error %s -> %s converter\n",
>> + sws_format_name(srcFormat), sws_format_name(dstFormat));
>>
>
> this is redundant with the default below
>
ok
>> +
>> + switch(dstFormat){
>> + case PIX_FMT_RGB32: conv = palette8torgb32; break;
>> + case PIX_FMT_BGR32: conv = palette8tobgr32; break;
>> + case PIX_FMT_RGB24: conv = palette8torgb24; break;
>> + case PIX_FMT_BGR24: conv = palette8tobgr24; break;
>> + default: av_log(c, AV_LOG_ERROR, "internal error %s -> %s converter\n",
>> + sws_format_name(srcFormat), sws_format_name(dstFormat)); break;
>> + }
>> +
>>
>
>
>> + if (conv) {
>>
>
> are we writing code with the assumtion of internal errors?
>
Removed the check, it was based in rgb2rgbWrapper().
>> + int i;
>> + uint8_t *dstPtr= dst[0] + dstStride[0]*srcSliceY;
>> + uint8_t *srcPtr= src[0];
>> +
>> + for (i=0; i<srcSliceH; i++) {
>> + conv(srcPtr, dstPtr, c->srcW, src[1]);
>> + srcPtr+= srcStride[0];
>> + dstPtr+= dstStride[0];
>> + }
>> + }
>> +
>> + return srcSliceH;
>> +}
>> /* {RGB,BGR}{15,16,24,32,32_1} -> {RGB,BGR}{15,16,24,32} */
>> static int rgb2rgbWrapper(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
>> int srcSliceH, uint8_t* dst[], int dstStride[]){
>>
>
>
>> @@ -2305,6 +2339,15 @@
>> && (!needsDither || (c->flags&(SWS_FAST_BILINEAR|SWS_POINT))))
>> c->swScale= rgb2rgbWrapper;
>>
>> + if ((srcFormat == PIX_FMT_PAL8 && (
>> + dstFormat == PIX_FMT_RGB32 ||
>> + dstFormat == PIX_FMT_RGB24 ||
>> + dstFormat == PIX_FMT_BGR32 ||
>> + dstFormat == PIX_FMT_BGR24)))
>> + c->swScale= pal2rgbWrapper;
>>
>
> this could also be used for the other byte based formats
> PIX_FMT_BGR4_BYTE, PIX_FMT_BGR8, ...
>
Done (in a certain different way)
>> +
>> +
>> +
>> if (srcFormat == PIX_FMT_YUV422P)
>> {
>> if (dstFormat == PIX_FMT_YUYV422)
>>
>
>
>> @@ -2659,12 +2702,18 @@
>> int i;
>> uint8_t* src2[4]= {src[0], src[1], src[2]};
>> uint32_t pal[256];
>> - int use_pal= c->srcFormat == PIX_FMT_PAL8
>> - || c->srcFormat == PIX_FMT_BGR4_BYTE
>> + int use_pal= c->srcFormat == PIX_FMT_BGR4_BYTE
>> || c->srcFormat == PIX_FMT_RGB4_BYTE
>> || c->srcFormat == PIX_FMT_BGR8
>> || c->srcFormat == PIX_FMT_RGB8;
>>
>> + use_pal = use_pal
>> + && c->srcFormat == PIX_FMT_PAL8
>> + && c->dstFormat != PIX_FMT_RGB32
>> + && c->dstFormat != PIX_FMT_RGB24
>> + && c->dstFormat != PIX_FMT_BGR32
>> + && c->dstFormat != PIX_FMT_BGR24;
>> +
>> if (c->sliceDir == 0 && srcSliceY != 0 && srcSliceY + srcSliceH != c->srcH) {
>> av_log(c, AV_LOG_ERROR, "Slices start in the middle!\n");
>> return 0;
>>
>
> This code makes no sense at all
> it assumes that no scaling happens which surely is not correct but it
>
indeed, changed the logic in my patch and fixed it.
> likely is wrong in more ways than just that.
>
Is swscale such a big mess that is that easy to break stuff?
> also may i suggest that you use swscale-example to test that your changes
> dont break things!
>
I will as soon as I get a ssh from Hudo...
-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pal2rgb_2.diff
Type: text/x-patch
Size: 4226 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081007/f6bf7920/attachment.bin>
More information about the ffmpeg-devel
mailing list