[Ffmpeg-devel] [PATCH] some lzo optimizations and test code

[Reimar Döffinger]

Hello,
On Sat, Jan 27, 2007 at 01:57:57PM +0100, Michael Niedermayer wrote:
> On Sat, Jan 27, 2007 at 12:56:38PM +0100, Reimar D?ffinger wrote:
> > attached code avoids checks for input buffer end where not needed due to
> > padding (up to 5% speedup when decompressing e.g. my mplayer binary, 85
> > bytes smaller code size) and adds test code.
> > It might (did not think that properly about it) require the input buffer
> > to be padded a bit more to avoid crashes on invalid data, but it is not
> > above FF_INPUT_BUFFER_PADDING_SIZE.
> > I would apply the two parts separately of course.
> > Does it look fine to you?
> 
> does this not cause a truncated compressed stream to end up with random
> trash at the end instead of just a truncated decompressed stream?

Not more than before if I hadn't forgotten the all-important check.
The get_byte function does not break decoding immediately but only at
the end of the while loop.
Attached is a fixed version that I'll apply somewhen this evening if
there are no objections.

[...]
> changing that to (dst + cnt > c->out_end || dst + cnt < dst) should
> prevent this and a optimizung compiler should be able to make
> quite efficient code out of it

Did not make a speed difference in my (limited) tests, so I changed it
like that. The backptr check is probably unneeded, since back is <
(1 << 15) and on most systems there will be no program data in the first
32k of virtual memory, but since it made no real speed difference...

Greetings,
Reimar D?ffinger
-------------- next part --------------
Index: libavcodec/lzo.c
===================================================================
--- libavcodec/lzo.c	(revision 7731)
+++ libavcodec/lzo.c	(working copy)
@@ -26,7 +26,7 @@
 
 //! define if we may write up to 12 bytes beyond the output buffer
 #define OUTBUF_PADDED 1
-//! define if we may read up to 4 bytes beyond the input buffer
+//! define if we may read up to 8 bytes beyond the input buffer
 #define INBUF_PADDED 1
 typedef struct LZOContext {
     uint8_t *in, *in_end;
@@ -45,6 +45,12 @@
     return 1;
 }
 
+#ifdef INBUF_PADDED
+#define GETB(c) (*(c).in++)
+#else
+#define GETB(c) get_byte(&(c))
+#endif
+
 /**
  * \brief decode a length value in the coding used by lzo
  * \param x previous byte value
@@ -170,10 +176,10 @@
     c.out = c.out_start = out;
     c.out_end = (uint8_t *)out + * outlen;
     c.error = 0;
-    x = get_byte(&c);
+    x = GETB(c);
     if (x > 17) {
         copy(&c, x - 17);
-        x = get_byte(&c);
+        x = GETB(c);
         if (x < 16) c.error |= LZO_ERROR;
     }
     while (!c.error) {
@@ -181,16 +187,16 @@
         if (x >> 4) {
             if (x >> 6) {
                 cnt = (x >> 5) - 1;
-                back = (get_byte(&c) << 3) + ((x >> 2) & 7) + 1;
+                back = (GETB(c) << 3) + ((x >> 2) & 7) + 1;
             } else if (x >> 5) {
                 cnt = get_len(&c, x, 31);
-                x = get_byte(&c);
-                back = (get_byte(&c) << 6) + (x >> 2) + 1;
+                x = GETB(c);
+                back = (GETB(c) << 6) + (x >> 2) + 1;
             } else {
                 cnt = get_len(&c, x, 7);
                 back = (1 << 14) + ((x & 8) << 11);
-                x = get_byte(&c);
-                back += (get_byte(&c) << 6) + (x >> 2);
+                x = GETB(c);
+                back += (GETB(c) << 6) + (x >> 2);
                 if (back == (1 << 14)) {
                     if (cnt != 1)
                         c.error |= LZO_ERROR;
@@ -202,15 +208,15 @@
             case COPY:
                 cnt = get_len(&c, x, 15);
                 copy(&c, cnt + 3);
-                x = get_byte(&c);
+                x = GETB(c);
                 if (x >> 4)
                     continue;
                 cnt = 1;
-                back = (1 << 11) + (get_byte(&c) << 2) + (x >> 2) + 1;
+                back = (1 << 11) + (GETB(c) << 2) + (x >> 2) + 1;
                 break;
             case BACKPTR:
                 cnt = 0;
-                back = (get_byte(&c) << 2) + (x >> 2) + 1;
+                back = (GETB(c) << 2) + (x >> 2) + 1;
                 break;
         }
         copy_backptr(&c, back, cnt + 2);
@@ -218,9 +224,13 @@
         state = cnt ? BACKPTR : COPY;
         if (cnt)
             copy(&c, cnt);
-        x = get_byte(&c);
+        x = GETB(c);
+        if (c.in > c.in_end)
+            c.error |= LZO_INPUT_DEPLETED;
     }
     *inlen = c.in_end - c.in;
+    if (c.in > c.in_end)
+        *inlen = 0;
     *outlen = c.out_end - c.out;
     return c.error;
 }
Index: libavcodec/lzo.h
===================================================================
--- libavcodec/lzo.h	(revision 7731)
+++ libavcodec/lzo.h	(working copy)
@@ -27,7 +27,7 @@
 #define LZO_INVALID_BACKPTR 4
 #define LZO_ERROR 8
 
-#define LZO_INPUT_PADDING 4
+#define LZO_INPUT_PADDING 8
 #define LZO_OUTPUT_PADDING 12
 
 int lzo1x_decode(void *out, int *outlen, void *in, int *inlen);