[Ffmpeg-devel] [PATCH] ported SGI decoder to the new API

Michael Niedermayer michaelni
Wed Mar 28 04:50:48 CEST 2007


Hi

On Wed, Mar 28, 2007 at 09:52:36AM +0800, Xiaohui Sun wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Michael Niedermayer wrote:
> >> +
> >> +        count = (row - start) / stride;
> >> +        while (count > 0) {
> >> +            i = count > 126 ? 126 : count;
> >> +            count -= i;
> >> +
> >> +            bytestream_put_byte(buf, i);
> >> +            length++;
> >> +
> >> +            bytestream_put_byte(buf, repeat);
> >> +            length++;
> >> +        };
> >> +    };
> > 
> > with an input of 1 1
> > this loop would produce 0x82 0x01 0x01
> > instead of the shorter 0x02 0x01
> > 
> > also its code duplication, the rle encoding code from targaenc.c should be
> > used or if your code is faster and works correctly then you should place it
> > in rle.c/h and make targaenc.c use it
> > 
> 
> I have read the code in targaenc.c, the rle code is below
> [...]
>     for(pos = start + bpp; count < FFMIN(128, len); pos += bpp, count ++) {
>         if(same != !memcmp(pos-bpp, pos, bpp)) {
> [...]
> 
> This may not proper for sgi and here we compare each color in a pixel
> instead of the pixel,

yes, the code in targaenc.c would have to be extended to be able to compare
both colors and pixels


> so the bpp is always set to 1, which leads to a
> frequent call to memcmp.

true, and there are many ways to optimize that out, a simple one is to leave
it to the compiler by using code like

static inline count_pixels_internal(..., nt bpp){
    ...
}

count_pixels(..., int bpp){
    if     (bpp==1) return count_pixels_internal(..., 1);
    else if(bpp==3) return count_pixels_internal(..., 3);
    else            return count_pixels_internal(..., bpp);
}

that way bpp is known to the compiler at compile time in count_pixels_internal
and it can replace the memcmp() by 1 or 3 simple byte compares


> Besides, comparing with 128  each time is not efficient enough, it could
> be compared after we get the consecutive pixels number

the compiler should be able to figure out that FFMIN(128, len) doesnt change
between the iterations of the loop and keep its value somewhere, if it
fails to do so we could always write:

int len2= FFMIN(128, len);
for(pos = start + bpp; count < len2; pos += bpp, count ++) {


> and I think
> comparing with zero will be more effective.

yes maybe
this could be benchmarked with START/STOP_TIMER(), if its faster a seperate
patch which changes that would be welcome

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

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070328/431ea31f/attachment.pgp>



More information about the ffmpeg-devel mailing list