[Ffmpeg-devel] [PATCH] Targa encoder

[Bobby Bingham]

Michael Niedermayer wrote:
> Hi
> 
> On Fri, Mar 16, 2007 at 02:50:32PM +0900, Bobby Bingham wrote:
>> Michael Niedermayer wrote:
>>> this loop is not optimal
>>> for example
>>> 0 1 1 0
>>> would be encoded as 0 0 81 1 0 0 but should be encoded as 3 0 1 1 0
>>>
>> updated patch attached.  gives a small size improvement for my test file.
>>
>>> [...]
>> -- 
>> Bobby Bingham
>> ??????????????????????????????????????????????????????????????????
> 
>> --- libavcodec/targaenc.c	2007-03-16 09:21:32.000000000 +0900
>> +++ libavcodec/targaenc.c	2007-03-16 14:41:21.000000000 +0900
>> @@ -21,6 +21,88 @@
>>   */
>>  #include "avcodec.h"
>>  
>> +/**
>> + * Count up to 127 consecutive pixels which are either all the same or
>> + * all differ from the previous and next pixels.
>> + * @param start Pointer to the first pixel
>> + * @param len Maximum number of pixels
>> + * @param bpp Bytes per pixel
>> + * @param same 1 if searching for identical pixel values.  0 for differing
>> + * @return Number of matching consecutive pixels found
>> + */
>> +static int count_pixels(uint8_t *start, int len, int bpp, int same)
>> +{
>> +    uint8_t *pos;
>> +    int count = 1;
>> +
>> +    if(len < 2) return len;
>> +
>> +    for(pos = start + bpp; count < FFMIN(128, len); pos += bpp, count ++) {
> 
> if len= 1 the function will return 1 without the extra len<2 check
> and len<1 should be impossible
> 

right you are. fixed.

> 
>> +        if(same != !memcmp(pos-bpp, pos, bpp)) {
>> +            if(!same) {
>> +                /* if bpp == 1, then 0 1 1 0 is more efficiently encoded as a single
>> +                 * raw block of pixels.  for larger bpp, RLE is as good or better */
>> +                if(bpp == 1 && count > 1 && count + 1 < FFMIN(128, len) && *pos != *(pos+1))
>> +                    continue;
> 
> i think count > 1 is always true

if this is the comparison of the first two pixels, then count will be 1. 
  when I wrote the code, I was thinking that if the first two pixels are 
the same it always makes sense to RLE encode them, but thinking about it 
again, that will produce larger output for sequences like 0 0 1, so this 
check should be removed.

> and the count + 1 < FFMIN(128, len) check also doesnt seem to change anything

this is to prevent the *pos != *(pos+1) check from reading past the last 
pixel.

> 
> 
>> +
>> +                /* if RLE can encode the next block better than as a raw block,
>> +                 * back up and leave _all_ the identical pixels for RLE */
>> +                count --;
>> +            }
>> +            break;
>> +        }
>> +    }
>> +
>> +    return count;
>> +}
> 
> wouldnt that encode 0 1 1 1 into 1 0 1 81 1?
> instead of 0 0 82 1

nope.  when it compares the 2nd and 3rd pixels, and sees that they're 
both ones, count is 2.  that *pos != *(pos+1) comparison will see that 
the 2nd and third pixels are the same, and rather than continuing the 
loop, it will decrease count to 1 and return that.  the end result being 
that the first 0 is encoded by itself, and the ones are all RLE'd 
together.  I just tested it too to double check.

update attached.

> 
> 
> [...]
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel


-- 
Bobby Bingham
??????????????????????
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3_targa_rle.patch
Type: text/x-patch
Size: 4653 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070316/ac158de8/attachment.bin>