[FFmpeg-devel] [PATCH] Optimization of original IFF codec

[Ronald S. Bultje]

Hi,

On Sun, Apr 25, 2010 at 12:47 PM, Sebastian Vater
<cdgs.basty at googlemail.com> wrote:
> Sebastian Vater a ?crit :
>> So, here are the two patches seperated.
>>
>> Now comes the micro-op patch as requested by Mans...
>>
> And now the restructuring optimization patch...

[..]
> -    for(y = 0; y < avctx->height; y++ ) {
> -        uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
> -        memset(row, 0, avctx->pix_fmt == PIX_FMT_PAL8 ? avctx->width : (avctx->width * 4));
> -        for (plane = 0; plane < avctx->bits_per_coded_sample && buf < buf_end; plane++) {
> -            if (avctx->pix_fmt == PIX_FMT_PAL8) {
> +    if (avctx->pix_fmt == PIX_FMT_PAL8) {
> +        for(y = 0; y < avctx->height; y++ ) {
> +            uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
> +            memset(row, 0, avctx->width);
> +            for (plane = 0; plane < avctx->bits_per_coded_sample && buf < buf_end; plane++) {
>                  decodeplane8(row, buf, FFMIN(s->planesize, buf_end - buf), avctx->bits_per_coded_sample, plane);
> -            } else { // PIX_FMT_BGR32
> -                decodeplane32(row, buf, FFMIN(s->planesize, buf_end - buf), avctx->bits_per_coded_sample, plane);
> +                buf += s->planesize;
> +            }
> +        }

Notice how many indentation changes are present here. It's better to
not change such lines, so that the patch looks like this:

+    if (avctx->pix_fmt == PIX_FMT_PAL8) {
     for(y = 0; y < avctx->height; y++ ) {
         uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
         memset(row, 0, avctx->pix_fmt == PIX_FMT_PAL8 ? avctx->width
: (avctx->width * 4));
         for (plane = 0; plane < avctx->bits_per_coded_sample && buf <
buf_end; plane++) {
-            if (avctx->pix_fmt == PIX_FMT_PAL8) {
                 decodeplane8(row, buf, FFMIN(s->planesize, buf_end -
buf), avctx->bits_per_coded_sample, plane);
+                buf += s->planesize;
+            }
+        }
+    } else { // PIX_FMT_BGR32
+        for(y = 0; y < avctx->height; y++ ) {
+            uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
+            memset(row, 0, (avctx->width << 2));
+            for (plane = 0; plane < avctx->bits_per_coded_sample &&
buf < buf_end; plane++) {
-            } else { // PIX_FMT_BGR32
                 decodeplane32(row, buf, FFMIN(s->planesize, buf_end -
buf), avctx->bits_per_coded_sample, plane);
+                buf += s->planesize;
             }
-            buf += s->planesize;
         }
     }

Then in a second patch, you would fix the indenting of the above code.
We call this "minimal functional patches". Same for the chunk below.

> -                } else { //noop
> +                } else { // noop

Typos go in separate patches.

Ronald