[FFmpeg-devel] [RFC] simplify interplayvideo.c

Reimar Döffinger Reimar.Doeffinger
Wed Mar 4 11:25:21 CET 2009


Hello,
reportedly this decoder has some issue on big-endian systems.
Unfortunately I consider that decoder to complex for debugging right
now, so I propose to simplify it first, as attached patch does.
I would apply it by "subject", e.g. "use memcpy and memset instead of
loops" or "use ?: instead of if/else".
There are also a lot of pointless () and stuff like "(y == 0) || (y ==
4)" that could be simplified to "!(y & 3)" or "(y >= 4) ? 4 : 0" to "y & 4"
(well, I consider that a simplification).
Also stuff like
> (flags >> shifter) & 0x03
> shifter += 2
that can be replaced with
> flags & 3
> flags >>= 2
So basically I am asking if I am free to play around with it as it
pleases me ;-)

Greetings,
Reimar D?ffinger
-------------- next part --------------
diff --git a/libavcodec/interplayvideo.c b/libavcodec/interplayvideo.c
index 83a9a47..068a8d3 100644
--- a/libavcodec/interplayvideo.c
+++ b/libavcodec/interplayvideo.c
@@ -265,7 +265,6 @@ static int ipvideo_decode_block_opcode_0x7(IpvideoContext *s)
 {
     int x, y;
     unsigned char P0, P1;
-    unsigned char B[8];
     unsigned int flags;
     int bitmask;
 
@@ -279,16 +278,11 @@ static int ipvideo_decode_block_opcode_0x7(IpvideoContext *s)
 
         /* need 8 more bytes from the stream */
         CHECK_STREAM_PTR(8);
-        for (y = 0; y < 8; y++)
-            B[y] = *s->stream_ptr++;
 
         for (y = 0; y < 8; y++) {
-            flags = B[y];
+            flags = *s->stream_ptr++;
             for (x = 0x01; x <= 0x80; x <<= 1) {
-                if (flags & x)
-                    *s->pixel_ptr++ = P1;
-                else
-                    *s->pixel_ptr++ = P0;
+                *s->pixel_ptr++ = flags & x ? P1 : P0;
             }
             s->pixel_ptr += s->line_inc;
         }
@@ -302,17 +296,10 @@ static int ipvideo_decode_block_opcode_0x7(IpvideoContext *s)
         bitmask = 0x0001;
         for (y = 0; y < 8; y += 2) {
             for (x = 0; x < 8; x += 2, bitmask <<= 1) {
-                if (flags & bitmask) {
-                    *(s->pixel_ptr + x) = P1;
-                    *(s->pixel_ptr + x + 1) = P1;
-                    *(s->pixel_ptr + s->stride + x) = P1;
-                    *(s->pixel_ptr + s->stride + x + 1) = P1;
-                } else {
-                    *(s->pixel_ptr + x) = P0;
-                    *(s->pixel_ptr + x + 1) = P0;
-                    *(s->pixel_ptr + s->stride + x) = P0;
-                    *(s->pixel_ptr + s->stride + x + 1) = P0;
-                }
+                s->pixel_ptr[            x    ] =
+                s->pixel_ptr[            x + 1] =
+                s->pixel_ptr[s->stride + x    ] =
+                s->pixel_ptr[s->stride + x + 1] = flags & bitmask ? P1 : P0;
             }
             s->pixel_ptr += s->stride * 2;
         }
@@ -382,10 +369,7 @@ static int ipvideo_decode_block_opcode_0x8(IpvideoContext *s)
                     P1 = P[lower_half + 5];
                 }
 
-                if (flags & bitmask)
-                    *s->pixel_ptr++ = P1;
-                else
-                    *s->pixel_ptr++ = P0;
+                *s->pixel_ptr++ = flags & bitmask ? P1 : P0;
             }
             s->pixel_ptr += s->line_inc;
         }
@@ -433,10 +417,7 @@ static int ipvideo_decode_block_opcode_0x8(IpvideoContext *s)
                         P1 = P[3];
                     }
 
-                    if (flags & bitmask)
-                        *s->pixel_ptr++ = P1;
-                    else
-                        *s->pixel_ptr++ = P0;
+                    *s->pixel_ptr++ = flags & bitmask ? P1 : P0;
                 }
                 s->pixel_ptr += s->line_inc;
             }
@@ -458,10 +439,7 @@ static int ipvideo_decode_block_opcode_0x8(IpvideoContext *s)
 
                 for (bitmask = 0x01; bitmask <= 0x80; bitmask <<= 1) {
 
-                    if (flags & bitmask)
-                        *s->pixel_ptr++ = P1;
-                    else
-                        *s->pixel_ptr++ = P0;
+                    *s->pixel_ptr++ = flags & bitmask ? P1 : P0;
                 }
                 s->pixel_ptr += s->line_inc;
             }
@@ -478,13 +456,12 @@ static int ipvideo_decode_block_opcode_0x9(IpvideoContext *s)
     unsigned char P[4];
     unsigned int flags = 0;
     int shifter = 0;
-    unsigned char pix;
 
     /* 4-color encoding */
     CHECK_STREAM_PTR(4);
 
-    for (y = 0; y < 4; y++)
-        P[y] = *s->stream_ptr++;
+    memcpy(P, s->stream_ptr, 4);
+    s->stream_ptr += 4;
 
     if ((P[0] <= P[1]) && (P[2] <= P[3])) {
 
@@ -510,11 +487,10 @@ static int ipvideo_decode_block_opcode_0x9(IpvideoContext *s)
 
         for (y = 0; y < 8; y += 2) {
             for (x = 0; x < 8; x += 2, shifter += 2) {
-                pix = P[(flags >> shifter) & 0x03];
-                *(s->pixel_ptr + x) = pix;
-                *(s->pixel_ptr + x + 1) = pix;
-                *(s->pixel_ptr + s->stride + x) = pix;
-                *(s->pixel_ptr + s->stride + x + 1) = pix;
+                s->pixel_ptr[            x    ] =
+                s->pixel_ptr[            x + 1] =
+                s->pixel_ptr[s->stride + x    ] =
+                s->pixel_ptr[s->stride + x + 1] = P[(flags >> shifter) & 0x03];
             }
             s->pixel_ptr += s->stride * 2;
         }
@@ -531,9 +507,8 @@ static int ipvideo_decode_block_opcode_0x9(IpvideoContext *s)
                 shifter = 0;
             }
             for (x = 0; x < 8; x += 2, shifter += 2) {
-                pix = P[(flags >> shifter) & 0x03];
-                *(s->pixel_ptr + x) = pix;
-                *(s->pixel_ptr + x + 1) = pix;
+                s->pixel_ptr[x    ] =
+                s->pixel_ptr[x + 1] = P[(flags >> shifter) & 0x03];
             }
             s->pixel_ptr += s->stride;
         }
@@ -550,9 +525,8 @@ static int ipvideo_decode_block_opcode_0x9(IpvideoContext *s)
                 shifter = 0;
             }
             for (x = 0; x < 8; x++, shifter += 2) {
-                pix = P[(flags >> shifter) & 0x03];
-                *(s->pixel_ptr + x) = pix;
-                *(s->pixel_ptr + s->stride + x) = pix;
+                s->pixel_ptr[            x] =
+                s->pixel_ptr[s->stride + x] = P[(flags >> shifter) & 0x03];
             }
             s->pixel_ptr += s->stride * 2;
         }
@@ -577,21 +551,21 @@ static int ipvideo_decode_block_opcode_0xA(IpvideoContext *s)
      * either top and bottom or left and right halves */
     CHECK_STREAM_PTR(4);
 
-    for (y = 0; y < 4; y++)
-        P[y] = *s->stream_ptr++;
+    memcpy(P, s->stream_ptr, 4);
+    s->stream_ptr += 4;
 
     if (P[0] <= P[1]) {
 
         /* 4-color encoding for each quadrant; need 28 more bytes */
         CHECK_STREAM_PTR(28);
 
-        for (y = 0; y < 4; y++)
-            B[y] = *s->stream_ptr++;
+        memcpy(B, s->stream_ptr, 4);
+        s->stream_ptr += 4;
         for (y = 4; y < 16; y += 4) {
-            for (x = y; x < y + 4; x++)
-                P[x] = *s->stream_ptr++;
-            for (x = y; x < y + 4; x++)
-                B[x] = *s->stream_ptr++;
+            memcpy(P + y, s->stream_ptr, 4);
+            s->stream_ptr += 4;
+            memcpy(B + y, s->stream_ptr, 4);
+            s->stream_ptr += 4;
         }
 
         for (y = 0; y < 8; y++) {
@@ -614,12 +588,12 @@ static int ipvideo_decode_block_opcode_0xA(IpvideoContext *s)
          * halves; need 20 more bytes */
         CHECK_STREAM_PTR(20);
 
-        for (y = 0; y < 8; y++)
-            B[y] = *s->stream_ptr++;
-        for (y = 4; y < 8; y++)
-            P[y] = *s->stream_ptr++;
-        for (y = 8; y < 16; y++)
-            B[y] = *s->stream_ptr++;
+        memcpy(B, s->stream_ptr, 8);
+        s->stream_ptr += 8;
+        memcpy(P + 4, s->stream_ptr, 4);
+        s->stream_ptr += 4;
+        memcpy(B + 8, s->stream_ptr, 8);
+        s->stream_ptr += 8;
 
         if (P[4] <= P[5]) {
 
@@ -662,16 +636,15 @@ static int ipvideo_decode_block_opcode_0xA(IpvideoContext *s)
 
 static int ipvideo_decode_block_opcode_0xB(IpvideoContext *s)
 {
-    int x, y;
+    int y;
 
     /* 64-color encoding (each pixel in block is a different color) */
     CHECK_STREAM_PTR(64);
 
     for (y = 0; y < 8; y++) {
-        for (x = 0; x < 8; x++) {
-            *s->pixel_ptr++ = *s->stream_ptr++;
-        }
-        s->pixel_ptr += s->line_inc;
+        memcpy(s->pixel_ptr, s->stream_ptr, 8);
+        s->stream_ptr += 8;
+        s->pixel_ptr  += s->stride;
     }
 
     /* report success */
@@ -681,18 +654,16 @@ static int ipvideo_decode_block_opcode_0xB(IpvideoContext *s)
 static int ipvideo_decode_block_opcode_0xC(IpvideoContext *s)
 {
     int x, y;
-    unsigned char pix;
 
     /* 16-color block encoding: each 2x2 block is a different color */
     CHECK_STREAM_PTR(16);
 
     for (y = 0; y < 8; y += 2) {
         for (x = 0; x < 8; x += 2) {
-            pix = *s->stream_ptr++;
-            *(s->pixel_ptr + x) = pix;
-            *(s->pixel_ptr + x + 1) = pix;
-            *(s->pixel_ptr + s->stride + x) = pix;
-            *(s->pixel_ptr + s->stride + x + 1) = pix;
+            s->pixel_ptr[            x    ] =
+            s->pixel_ptr[            x + 1] =
+            s->pixel_ptr[s->stride + x    ] =
+            s->pixel_ptr[s->stride + x + 1] = *s->stream_ptr++;
         }
         s->pixel_ptr += s->stride * 2;
     }
@@ -703,15 +674,15 @@ static int ipvideo_decode_block_opcode_0xC(IpvideoContext *s)
 
 static int ipvideo_decode_block_opcode_0xD(IpvideoContext *s)
 {
-    int x, y;
+    int y;
     unsigned char P[4];
     unsigned char index = 0;
 
     /* 4-color block encoding: each 4x4 block is a different color */
     CHECK_STREAM_PTR(4);
 
-    for (y = 0; y < 4; y++)
-        P[y] = *s->stream_ptr++;
+    memcpy(P, s->stream_ptr, 4);
+    s->stream_ptr += 4;
 
     for (y = 0; y < 8; y++) {
         if (y < 4)
@@ -719,12 +690,9 @@ static int ipvideo_decode_block_opcode_0xD(IpvideoContext *s)
         else
             index = 2;
 
-        for (x = 0; x < 8; x++) {
-            if (x == 4)
-                index++;
-            *s->pixel_ptr++ = P[index];
-        }
-        s->pixel_ptr += s->line_inc;
+        memset(s->pixel_ptr    , P[index    ], 4);
+        memset(s->pixel_ptr + 4, P[index + 1], 4);
+        s->pixel_ptr += s->stride;
     }
 
     /* report success */
@@ -733,7 +701,7 @@ static int ipvideo_decode_block_opcode_0xD(IpvideoContext *s)
 
 static int ipvideo_decode_block_opcode_0xE(IpvideoContext *s)
 {
-    int x, y;
+    int y;
     unsigned char pix;
 
     /* 1-color encoding: the whole block is 1 solid color */
@@ -741,10 +709,8 @@ static int ipvideo_decode_block_opcode_0xE(IpvideoContext *s)
     pix = *s->stream_ptr++;
 
     for (y = 0; y < 8; y++) {
-        for (x = 0; x < 8; x++) {
-            *s->pixel_ptr++ = pix;
-        }
-        s->pixel_ptr += s->line_inc;
+        memset(s->pixel_ptr, pix, 8);
+        s->pixel_ptr += s->stride;
     }
 
     /* report success */
@@ -778,7 +744,16 @@ static int ipvideo_decode_block_opcode_0xF(IpvideoContext *s)
     return 0;
 }
 
-static int (*ipvideo_decode_block[16])(IpvideoContext *s);
+static int (* const ipvideo_decode_block[])(IpvideoContext *s) = {
+    ipvideo_decode_block_opcode_0x0, ipvideo_decode_block_opcode_0x1,
+    ipvideo_decode_block_opcode_0x2, ipvideo_decode_block_opcode_0x3,
+    ipvideo_decode_block_opcode_0x4, ipvideo_decode_block_opcode_0x5,
+    ipvideo_decode_block_opcode_0x6, ipvideo_decode_block_opcode_0x7,
+    ipvideo_decode_block_opcode_0x8, ipvideo_decode_block_opcode_0x9,
+    ipvideo_decode_block_opcode_0xA, ipvideo_decode_block_opcode_0xB,
+    ipvideo_decode_block_opcode_0xC, ipvideo_decode_block_opcode_0xD,
+    ipvideo_decode_block_opcode_0xE, ipvideo_decode_block_opcode_0xF,
+};
 
 static void ipvideo_decode_opcodes(IpvideoContext *s)
 {
@@ -786,15 +761,12 @@ static void ipvideo_decode_opcodes(IpvideoContext *s)
     int index = 0;
     unsigned char opcode;
     int ret;
-    int code_counts[16];
+    int code_counts[16] = {0};
     static int frame = 0;
 
     debug_interplay("------------------ frame %d\n", frame);
     frame++;
 
-    for (x = 0; x < 16; x++)
-        code_counts[x] = 0;
-
     /* this is PAL8, so make the palette available */
     memcpy(s->current_frame.data[1], s->avctx->palctrl->palette, PALETTE_COUNT * 4);
 
@@ -828,8 +800,7 @@ static void ipvideo_decode_opcodes(IpvideoContext *s)
             }
         }
     }
-    if ((s->stream_ptr != s->stream_end) &&
-        (s->stream_ptr + 1 != s->stream_end)) {
+    if (s->stream_end - s->stream_ptr > 1) {
         av_log(s->avctx, AV_LOG_ERROR, " Interplay video: decode finished with %td bytes left over\n",
             s->stream_end - s->stream_ptr);
     }
@@ -852,24 +823,6 @@ static av_cold int ipvideo_decode_init(AVCodecContext *avctx)
     /* decoding map contains 4 bits of information per 8x8 block */
     s->decoding_map_size = avctx->width * avctx->height / (8 * 8 * 2);
 
-    /* assign block decode functions */
-    ipvideo_decode_block[0x0] = ipvideo_decode_block_opcode_0x0;
-    ipvideo_decode_block[0x1] = ipvideo_decode_block_opcode_0x1;
-    ipvideo_decode_block[0x2] = ipvideo_decode_block_opcode_0x2;
-    ipvideo_decode_block[0x3] = ipvideo_decode_block_opcode_0x3;
-    ipvideo_decode_block[0x4] = ipvideo_decode_block_opcode_0x4;
-    ipvideo_decode_block[0x5] = ipvideo_decode_block_opcode_0x5;
-    ipvideo_decode_block[0x6] = ipvideo_decode_block_opcode_0x6;
-    ipvideo_decode_block[0x7] = ipvideo_decode_block_opcode_0x7;
-    ipvideo_decode_block[0x8] = ipvideo_decode_block_opcode_0x8;
-    ipvideo_decode_block[0x9] = ipvideo_decode_block_opcode_0x9;
-    ipvideo_decode_block[0xA] = ipvideo_decode_block_opcode_0xA;
-    ipvideo_decode_block[0xB] = ipvideo_decode_block_opcode_0xB;
-    ipvideo_decode_block[0xC] = ipvideo_decode_block_opcode_0xC;
-    ipvideo_decode_block[0xD] = ipvideo_decode_block_opcode_0xD;
-    ipvideo_decode_block[0xE] = ipvideo_decode_block_opcode_0xE;
-    ipvideo_decode_block[0xF] = ipvideo_decode_block_opcode_0xF;
-
     s->current_frame.data[0] = s->last_frame.data[0] =
     s->second_last_frame.data[0] = NULL;
 



More information about the ffmpeg-devel mailing list