[Ffmpeg-devel] [PATCH] Targa encoder

[Michael Niedermayer]

Hi

On Fri, Mar 16, 2007 at 11:23:13PM +0900, Bobby Bingham wrote:
[...]
> >
> >>+        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.

i know, i was thinking that that isnt really a problem but iam fine with
leaving the check in there, its cleaner that way ...


> 
> >
> >
> >>+
> >>+                /* 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.

indeed

patch looks ok

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- 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/20070316/a3161672/attachment.pgp>