On Sat, May 19, 2012 at 01:06:49PM +0200, Jordi Ortiz wrote:
> ---
> libavcodec/dwt.c | 1137
> ++++++++++++++++++++++++++++++------------------------
> 1 file changed, 638 insertions(+), 499 deletions(-)
The subject line could be improved. Just
dwt: K&R formatting cosmetics
is enough. How you achieved that goal is immaterial.
While uncrustify is a great tool, you cannot trust it completely. This
patch still needs some extra changes, see my comments below. I have
fixed all issues I noticed, squashed it with the dwt.h patch and will
push in a few.
> --- a/libavcodec/dwt.c
> +++ b/libavcodec/dwt.c
> @@ -66,713 +67,843 @@ void ff_slice_buffer_release(slice_buffer * buf, int
> line)
>
> - for(i=0; i<w; i++){
> - dst[i*dst_step] =
> - LIFT(src[i*src_step],
> - ((mul*(ref[i*ref_step] + ref[(i+1)*ref_step])+add)>>shift),
> + for (i = 0; i < w; i++)
> + dst[i * dst_step] =
> + LIFT(src[i * src_step],
> + ((mul * (ref[i * ref_step] + ref[(i + 1) * ref_step]) +
> add) >> shift),
> inverse);
>
> - if(mirror_right){
> - dst[w*dst_step] =
> - LIFT(src[w*src_step],
> - ((mul*2*ref[w*ref_step]+add)>>shift),
> + if (mirror_right) {
> + dst[w * dst_step] =
> + LIFT(src[w * src_step],
> + ((mul * 2 * ref[w * ref_step] + add) >> shift),
> inverse);
We don't break lines like this in assignments.
> #define LIFTS(src, ref, inv) \
> + ((inv) ? \
> + (src) + (((ref) + 4 * (src)) >> shift) : \
> + -((-16 * (src) + (ref) + add / 4 + 1 + (5 << 25)) / (5 * 4) - (1 <<
> 23)))
Align the '\' when reformatting macros.
> - if(mirror_right){
> - dst[w*dst_step] =
> - LIFTS(src[w*src_step], mul*2*ref[w*ref_step]+add, inverse);
> + if (mirror_right) {
> + dst[w * dst_step] =
> + LIFTS(src[w * src_step], mul * 2 * ref[w * ref_step] + add,
> inverse);
> }
> }
> +
> #endif /* ! liftS */
The added empty line is a stray change introduced by uncrustify. It does
that, I don't know how to prevent it non-manually...
> #if 0
> {
> + for (x = 1; x + 1 < width2; x += 2) {
> + A3 = temp[x + width2];
> + A4 = temp[x + 1];
> + A3 -= (A2 + A4) >> 1;
> + A2 += (A1 + A3 + 2) >> 2;
> + b[x + width2] = A3;
> + b[x] = A2;
> +
> + A1 = temp[x + 1 + width2];
> + A2 = temp[x + 2];
> + A1 -= (A2 + A4) >> 1;
> + A4 += (A1 + A3 + 2) >> 2;
> + b[x + 1 + width2] = A1;
> + b[x + 1] = A4;
> + }
The '=' could be aligned across the empty line.
> +void ff_spatial_dwt(DWTELEM *buffer, int width, int height, int stride,
> + int type, int decomposition_count)
> +{
> int level;
>
> - for(level=0; level<decomposition_count; level++){
> - switch(type){
> - case DWT_97: spatial_decompose97i(buffer, width>>level,
> height>>level, stride<<level); break;
> - case DWT_53: spatial_decompose53i(buffer, width>>level,
> height>>level, stride<<level); break;
> + for (level = 0; level < decomposition_count; level++) {
> + switch (type) {
> + case DWT_97: spatial_decompose97i(buffer,
> + width >> level, height >> level,
> + stride << level);
> + break;
> + case DWT_53: spatial_decompose53i(buffer,
> + width >> level, height >> level,
> + stride << level);
> + break;
This is still missing another line break after the case label.
> +static void spatial_compose53i_dy_buffered(DWTCompose *cs, slice_buffer *sb,
> + int width, int height,
> + int stride_line)
> +{
> + int y = cs->y;
>
> - IDWTELEM *b0= cs->b0;
> - IDWTELEM *b1= cs->b1;
> - IDWTELEM *b2= slice_buffer_get_line(sb, mirror(y+1, height-1) *
> stride_line);
> - IDWTELEM *b3= slice_buffer_get_line(sb, mirror(y+2, height-1) *
> stride_line);
> + IDWTELEM *b0 = cs->b0;
> + IDWTELEM *b1 = cs->b1;
> + IDWTELEM *b2 = slice_buffer_get_line(sb, mirror(y + 1, height - 1)
> + * stride_line);
> + IDWTELEM *b3 = slice_buffer_get_line(sb, mirror(y + 2, height - 1)
> + * stride_line);
We usually have the operator at the end of the line.
> - case DWT_97: spatial_compose97i_buffered_init(cs+level, sb,
> height>>level, stride_line<<level); break;
> - case DWT_53: spatial_compose53i_buffered_init(cs+level, sb,
> height>>level, stride_line<<level); break;
> + case DWT_97:
> + spatial_compose97i_buffered_init(cs + level, sb, height >> level,
> + stride_line << level);
> + break;
> + case DWT_53:
> + spatial_compose53i_buffered_init(cs + level, sb, height >> level,
> + stride_line << level);
> + break;
Indentation is off in the function calls.
> + static const int scale[2][2][4][4] = {
> {
> + {
> + // 9/7 8x8 dec=3
> + { 268, 239, 239, 213 },
> + { 0, 224, 224, 152 },
> + { 0, 135, 135, 110 },
> + }, {
> + // 9/7 16x16 or 32x32 dec=4
> + { 344, 310, 310, 280 },
> + { 0, 320, 320, 228 },
> + { 0, 175, 175, 136 },
> + { 0, 129, 129, 102 },
> + }
> + }, {
> + {
> + // 5/3 8x8 dec=3
> + { 275, 245, 245, 218 },
> + { 0, 230, 230, 156 },
> + { 0, 138, 138, 113 },
> + }, {
> + // 5/3 16x16 or 32x32 dec=4
> + { 352, 317, 317, 286 },
> + { 0, 328, 328, 233 },
> + { 0, 180, 180, 140 },
> + { 0, 132, 132, 105 },
> + }
> }
The placement of the opening braces is inconsistent.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
