[FFmpeg-devel] need help fixing sws on PPC

[Michael Niedermayer]

On Wed, Oct 08, 2008 at 09:42:39PM +0200, Vitor Sessak wrote:
> Michael Niedermayer escreveu:
> > On Wed, Oct 08, 2008 at 08:16:46PM +0200, Vitor Sessak wrote:
> >> Michael Niedermayer escreveu:
> >>> On Tue, Oct 07, 2008 at 11:26:40PM +0200, Vitor Sessak wrote:
> >>>   
> >>>> Michael Niedermayer escreveu:
> >>>>     
> >>>>> On Tue, Oct 07, 2008 at 10:36:51PM +0200, Vitor Sessak wrote:
> >>>>>         
> >>>>>> Michael Niedermayer escreveu:
> >>>>>>             
> >>>>>>> On Tue, Oct 07, 2008 at 09:24:26PM +0200, Vitor Sessak wrote:
> >>>>>>>                   
> >>>>>>>> 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:
> >>>>>>>>>>>>                                                 
> >>>>> [...]
> >>>>>         
> >>>>>>> [...]
> >>>>>>>                   
> >>>>>>>> @@ -2658,12 +2705,7 @@
> >>>>>>>>                int srcSliceH, uint8_t* dst[], int dstStride[]){
> >>>>>>>>      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
> >>>>>>>> -                || c->srcFormat == PIX_FMT_RGB4_BYTE
> >>>>>>>> -                || c->srcFormat == PIX_FMT_BGR8
> >>>>>>>> -                || c->srcFormat == PIX_FMT_RGB8;
> >>>>>>>> +    uint32_t pal[512];
> >>>>>>>>                         
> >>>>>>> what about putting a rgbpal and yuvpal in the context?
> >>>>>>> i think this would be cleaner than the "didnt you know the rgb pal was
> >>>>>>> at pal+256?!"
> >>>>>>>                   
> >>>>>> I agree. New patch attached.
> >>>>>>
> >>>>>> I've also attached a second patch to use pal_yuv instead of src[1] that 
> >>>>>> is completely untested for the moment (I will obviously not apply 
> >>>>>> anything before doing a full test)...
> >>>>>>             
> >>>>> [...]
> >>>>>
> >>>>>         
> >>>>>> @@ -1711,6 +1718,39 @@
> >>>>>>      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;
> >>>>>>             
> >>>>> tabs ...
> >>>>>         
> >>>> Ok, tab-less version attached.
> >>>>
> >>>>     
> >>>>> [...]
> >>>>>         
> >>>>>> @@ -2701,9 +2742,10 @@
> >>>>>>              y= av_clip_uint8((RY*r + GY*g + BY*b + ( 
> >>>>>> 33<<(RGB2YUV_SHIFT-1)))>>RGB2YUV_SHIFT);
> >>>>>>              u= av_clip_uint8((RU*r + GU*g + BU*b + 
> >>>>>> (257<<(RGB2YUV_SHIFT-1)))>>RGB2YUV_SHIFT);
> >>>>>>              v= av_clip_uint8((RV*r + GV*g + BV*b + 
> >>>>>> (257<<(RGB2YUV_SHIFT-1)))>>RGB2YUV_SHIFT);
> >>>>>> -            pal[i]= y + (u<<8) + (v<<16);
> >>>>>>             
> >>>>>         
> >>>>>> +            c->pal_yuv[i]= y + (u<<8) + (v<<16);
> >>>>>> +            c->pal_rgb[i]= b | (g<<8) | (r<<16);
> >>>>>>             
> >>>>> is there any reason why one uses + and one | ?
> >>>>>         
> >>>> none. Changed to both "+"
> >>>>
> >>>>     
> >>>>> [...]
> >>>>>         
> >>>>>> Index: libswscale/swscale_template.c
> >>>>>> ===================================================================
> >>>>>> --- libswscale/swscale_template.c	(revis?o 27716)
> >>>>>> +++ libswscale/swscale_template.c	(c?pia de trabalho)
> >>>>>> @@ -2926,7 +2926,7 @@
> >>>>>>      const int chrSrcSliceY= srcSliceY >> c->chrSrcVSubSample;
> >>>>>>      const int chrSrcSliceH= -((-srcSliceH) >> c->chrSrcVSubSample);
> >>>>>>      int lastDstY;
> >>>>>> -    uint32_t *pal=NULL;
> >>>>>> +    uint32_t *pal=c->pal_yuv;
> >>>>>>             
> >>>>> if the cde is changed to use pal_yuv then the src2[1] setting stuff 
> >>>>> becomes
> >>>>> unneded
> >>>>>         
> >>>> Done too.
> >>>>     
> >>> looks ok if it passes the tests ...
> >>>   
> >> Applied. One more small tested simplification attached.
> > 
> > iam mildly against this one, the code is wrong after the patch it just
> > works as the changed variables is not used.
> 
> The point was also to prevent new code to read the palette from src[1], 
> since now it is much cleaner to read it from c->pal_yuv.

you arent preventing it though, src[1] is just randomized in a limited
set of cases. Its more likely this would lead to hard to debug bugs
than preventing its unintentional use.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct awnser.
-------------- 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/20081008/cf586328/attachment.pgp>