[FFmpeg-devel] [PATCH]Simplify targa decoding

[Michael Niedermayer]

On Fri, Jan 20, 2012 at 04:05:01AM +0100, yann.lepetitcorps at free.fr wrote:
> > >          for(i = 0; i < count; i++){
> > > -            switch(depth){
> > > -            case 1:
> > > -                *dst = *src;
> > > -                break;
> > > -            case 2:
> > > -                *((uint16_t*)dst) = AV_RL16(src);
> > > -                break;
> > > -            case 3:
> > > -                dst[0] = src[0];
> > > -                dst[1] = src[1];
> > > -                dst[2] = src[2];
> > > -                break;
> > > -            case 4:
> > > -                *((uint32_t*)dst) = AV_RL32(src);
> > > -                break;
> > > -            }
> > > +            memcpy(dst, src, count * depth);
> > >              dst += depth;
> > >              if(!type)
> > >                  src += depth;
> >
> > Without having looked properly, I suspect something isn't right here,
> > you memcpy count*depth but increment only by depth and loop
> > around all this count times, or am I missing something?
> 
> I think that the idea is to make only one memcpy for each chunk instead of a lot
> of switchs (one for each pixel) that use always the same path.
> 
> One problem seem to be the case where x+count > width
> => we have to handle multiples lines **and** the necessary padding at each line
> 
> Another problem is src pointer "incrementation" logic (per pixel or per chunk)
> that is handled into the while(y < h) and for( i=0; i <count ; i++) loops
> 
> => I see if all this can to be reorganised for to make only one test of the type
> value at the beginning (insteead at each pixel) and for to  copy pixels per
> chunk (instead per pixel) with the necessary padding (if needed) at each line
> 
> 
> I have too see this test :
> 
>       if((x + count > w) && (x + count + 1 > (h - y) * w)){
> 
> Is really the "x + count > w" necessary and why not to  use ">=" instead "+ 1 >"
>  for to test if the chunk can effectivly to be contained into remaining pixels
> until the end of the image ?
> (because since h is always > y, (h - y) * w is always >= w)
> 

> => isn't the same test that   if( x + count >= (h-y) * w ) ???

no, this would break decoding, but ill send a patch in a moment that
will simplify this check

thanks


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

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120121/eaf29d68/attachment.asc>