[FFmpeg-devel] [PATCH] vble optimizations and simplifications

[Derek Buitenhuis]

On 11/11/2011 8:18 PM, Michael Niedermayer wrote:
> Hi
>
> please review:)

KK.

>  From aa1e4ac04581a4db0db2b81d08241664fcf1bcdb Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer<michaelni at gmx.at>
> Date: Sat, 12 Nov 2011 01:39:05 +0100
> Subject: [PATCH 1/7] vble: change variable to int, its slightly faster and int is preferable if no specific size is needed.

I don't see any harm in this. I use uint8_t because nothing >8 bits
would ever be in it. Interesting that int is slightly faster.

>  From cf055da8ab63227fe9c352e9c1bff695c169431f Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer<michaelni at gmx.at>
> Date: Sat, 12 Nov 2011 01:40:37 +0100
> Subject: [PATCH 2/7] vble: use LUT for vble_read_reverse_unary()
>   slightly faster

My only worry is that it may make it less clear what's going on,
though the function name should describe it fairly well.

> -        val = 7 - av_log2_16bit(av_reverse[val]);
> +        val= LUT[val];

Please keep the coding style the same and put a space between
val and =.

It's fine besides that.

>  From fdc03e5439911845352f8a01d21f6825202ad123 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer<michaelni at gmx.at>
> Date: Sat, 12 Nov 2011 01:47:34 +0100
> Subject: [PATCH 3/7] vble: remove vble_read_reverse_unary(), the code is a bit simpler this way

Looks good, but could you add a comment in the code about what it's doing?
Something about unary coding, obviously. :)

>  From 323be15417618d4e3f799245983bb24c085b5039 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer<michaelni at gmx.at>
> Date: Sat, 12 Nov 2011 02:02:22 +0100
> Subject: [PATCH 4/7] vble: move get_bits_left() check out of inner loop, we can perform the check completely before the loop.

[...]

>   +    int allbits=0;

Please space this properly: int allbits = 0;

>       memset(ctx->val, 0, ctx->size);
> -
>       for (i = 0; i<  ctx->size; i++) {

Why was this space removed?

Rest looks OK.

>  From 5cee143d93936195cf8acf2c4bc04cf6ba575cd4 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer<michaelni at gmx.at>
> Date: Sat, 12 Nov 2011 02:06:56 +0100
> Subject: [PATCH 5/7] vble: remove unused variable len.

Looks good.

>  From 4fd706310e6873379af8365740208ae6b5fef70a Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer<michaelni at gmx.at>
> Date: Sat, 12 Nov 2011 02:07:37 +0100
> Subject: [PATCH 6/7] vble: remove len array, its unneeded
>   also remove unneeded memset()

I worry that it will make it less clear what is going on in the code...
Perhaps a comment?

>  From aeb455ea370d45cf22dbd3ce55f1dc2af414ada8 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer<michaelni at gmx.at>
> Date: Sat, 12 Nov 2011 02:10:06 +0100
> Subject: [PATCH 7/7] vble: remove flags copy, its not used in any speed relevant code.

Looks good.

- Derek