Hi,
On Sun, Dec 25, 2011 at 7:09 AM, Ronald S. Bultje <[email protected]>wrote:
> Until line 3060.
>
> @@ -3071,245 +3139,250 @@ static int
dct_quantize_trellis_c(MpegEncContext *s,
[..]
> - if(((unsigned)(level+threshold1))>threshold2){
> + if (((unsigned)(level+threshold1)) > threshold2) {
Spaces aorund +.
> +// if ( bias+level >= (1<<(QMAT_SHIFT - 3))
> +// || bias-level >= (1<<(QMAT_SHIFT - 3))) {
Even commented code should be indented correctly, and also || on previous
line, spaces around <<, and // itself should be correctly indented.
> // coeff[2][k]= level-2;
Same.
> // coeff[2][k]= -level+2;
Same.
> + *overflow = s->max_qcoeff < max; //overflow might have happened
Space between // and comment text.
> + int dct_coeff = FFABS(block[ scantable[i] ]);
No space after [ and before ].
> + dct_coeff = (dct_coeff * ff_inv_aanscales[ scantable[i] ])
>> 12;
Same.
> + zero_distortion = dct_coeff*dct_coeff;
Spaces around *.
> + if (s->out_format == FMT_H263) {
> + unquant_coeff = alevel * qmul + qadd;
> + } else { //MPEG1
Space between // and comment text.
> + j = s->dsp.idct_permutation[scantable[i]]; //FIXME
optimize
Same.
> + if (s->mb_intra) {
> + unquant_coeff = (int)(alevel * qscale *
s->intra_matrix[j]) >> 3;
>80 characters.
> unquant_coeff = ((( alevel << 1) + 1) * qscale
* ((int) s->inter_matrix[j])) >> 4;
Same, plus why all the spaces around alevel? Spaces around << is enough in
that part of the line.
> unquant_coeff = (unquant_coeff - 1) | 1;
Or here, why all the spaces after the =? One is enough.
> distortion= (unquant_coeff - dct_coeff) * (unquant_coeff -
dct_coeff) - zero_distortion;
Space before =, and >80 characters.
> + int score = distortion +
length[UNI_AC_ENC_INDEX(run, level)] * lambda;
>80 characters.
> + for (j = survivor_count - 1; j >= 0; j--) {
> + int run = i - survivor[j];
> + int score = distortion +
last_length[UNI_AC_ENC_INDEX(run, level)] * lambda;
Same.
> + if (s->out_format == FMT_H263) {
> + for (j = survivor_count - 1; j >= 0; j--) {
This line is incorrectly indented, needs 2 more spaces.
> //Note: there is a vlc code in mpeg4 which is 1 bit shorter then
another one with a shorter run and the same level
>80 characters, and space between // and comment text.
> + if (score_tab[ survivor[survivor_count - 1]] <=
best_score + lambda)
>80 characters.
> + if (i)
> + score += lambda * 2; //FIXME exacter?
Space between // and comment text.
> + if (s->out_format == FMT_H263) {
> + unquant_coeff= (alevel * qmul + qadd) >> 3;
> + } else { //MPEG1
Same.
> unquant_coeff = ((( alevel << 1) + 1) * qscale *
((int) s->inter_matrix[0])) >> 4;
> unquant_coeff = (unquant_coeff - 1) | 1;
See what I said before about too many spaces.
> + distortion = (unquant_coeff - dc) * (unquant_coeff - dc);
> + level += 64;
> + if ((level&(~127)) == 0)
Spaces around &.
> @@ -3318,19 +3391,22 @@ static int dct_quantize_trellis_c(MpegEncContext
*s,
[..]
> + basis[perm_index][8 * x + y]= lrintf(s * cos((M_PI /
8.0) * i * (x + 0.5)) * cos((M_PI / 8.0) * j * (y + 0.5)));
>80 characters.
> static int dct_quantize_refine(MpegEncContext *s, //FIXME breaks denoise?
Space between // and comment text.
> DCTELEM *block, int16_t *weight, DCTELEM *orig,
> + int n, int qscale)
These aren't vertically aligned with the first character after the ( in the
first line.
> // unsigned int threshold1, threshold2;
> // int bias=0;
Commented code indenting.
> int rle_index, run, q = 1, sum; //q is only used when s->mb_intra is
true
Space between // and comment text.
> // block[0] = (block[0] + (q >> 1)) / q;
Commented code indenting.
> +// if (s->mpeg_quant || s->out_format == FMT_MPEG1)
> // bias= 1<<(QMAT_SHIFT-1);
Same.
> #ifdef REFINE_STATS
> {START_TIMER
> #endif
[..]
> #ifdef REFINE_STATS
> STOP_TIMER("memset rem[]")}
> #endif
So I'll apply an exception here, you don't need to indent this code.
Pretend it isn't there. Same for other occurrences of REFINE_STATS (or
START/STOP_TIMER in general).
> + dc += (1 << (RECON_SHIFT - 1));
> + for (i = 0; i < 64; i++) {
> + rem[i] = dc - (orig[i] << RECON_SHIFT); //FIXME use orig
dirrectly instead of copying to rem[]
> }
>80 characters, space between // and comment text. You can fix this by
doing:
for (i = 0; ...) {
// comment
rem[i] = ..;
}
> weight[i] = w;
> // w=weight[i] = (63*qns + (w/2)) / w;
Commented code indenting and spaces around *.
> @@ -3482,160 +3561,164 @@ STOP_TIMER("dct")}
[..]
> score= s->dsp.try_8x8basis(rem, weight, basis[0],
new_coeff - old_coeff);
>80 characters.
> + run2 = run_tab[rle_index++]; //FIXME ! maybe after last
Space between // and comment text.
> + assert(run2 >= 0 || i >= last_non_zero );
Remove the space between "zero" and ")".
> + if (s->avctx->quantizer_noise_shaping < 2 &&
FFABS(new_level) > FFABS(level))
> continue;
>80 characters.
> //FIXME check for overflow
Space between // and comment text.
> + if (i < last_non_zero)
> + score += length[UNI_AC_ENC_INDEX(run,
new_level + 64)]
> + - length[UNI_AC_ENC_INDEX(run,
level + 64)];
> else
> + score +=
last_length[UNI_AC_ENC_INDEX(run, new_level + 64)]
> + -
last_length[UNI_AC_ENC_INDEX(run, level + 64)];
>80 characters, and - gors on previous line. One way to fix that is to save
the index into a variable:
int idx1, idx2;
idx1 = UNI_AC_ENC_INDEX(..);
idx2 = UNI...;
score += length[idx1] - length[idx2];
> + if (analyze_gradient) {
> + int g = d1[ scantable[i] ];
No space after [ or before ].
> + if (i < last_non_zero) {
> + int next_i = i + run2 + 1;
> + int next_level = block[
perm_scantable[next_i] ] + 64;
Same.
> + if (next_i < last_non_zero)
> + score += length[UNI_AC_ENC_INDEX(run,
65)] +
> + length[UNI_AC_ENC_INDEX(run2,
next_level)] -
> + length[UNI_AC_ENC_INDEX(run +
run2 + 1, next_level)];
> else
> + score += length[UNI_AC_ENC_INDEX(run,
65)] +
> +
last_length[UNI_AC_ENC_INDEX(run2, next_level)] -
> +
last_length[UNI_AC_ENC_INDEX(run + run2 + 1, next_level)];
> + } else {
> score += last_length[UNI_AC_ENC_INDEX(run,
65)];
> + if (prev_level) {
> + score +=
length[UNI_AC_ENC_INDEX(prev_run, prev_level)] -
> +
last_length[UNI_AC_ENC_INDEX(prev_run, prev_level)];
> }
>80 characters for many of the above, see comment earlier about possible
way to fix that. Also too many spaces after +=.
> + if (i < last_non_zero) {
> + int next_i = i + run2 + 1;
> + int next_level = block[ perm_scantable[next_i] ]
+ 64;
No space after [ or before ].
> + if (next_level&(~127))
> + next_level = 0;
Spaces around &.
> + if (next_i < last_non_zero)
> + score += length[UNI_AC_ENC_INDEX(run +
run2 + 1, next_level)] -
> + length[UNI_AC_ENC_INDEX(run2,
next_level)] -
> + length[UNI_AC_ENC_INDEX(run, 65)];
> else
> + score += last_length[UNI_AC_ENC_INDEX(run
+ run2 + 1, next_level)] -
> +
last_length[UNI_AC_ENC_INDEX(run2, next_level)] -
> + length[UNI_AC_ENC_INDEX(run, 65)];
> + } else {
> score += -last_length[UNI_AC_ENC_INDEX(run, 65)];
> + if (prev_level) {
> + score +=
last_length[UNI_AC_ENC_INDEX(prev_run, prev_level)] -
> + length[UNI_AC_ENC_INDEX(prev_run,
prev_level)];
> }
> }
>80 characters, too many spaces after +=, see comments earlier.
> + assert((score < 100 * lambda && score > -100 * lambda)
|| lambda == 0);
>80 characters.
> + score += s->dsp.try_8x8basis(rem, weight, basis[j],
unquant_change);
>80 characters.
> @@ -3643,65 +3726,66 @@ STOP_TIMER("dct")}
[..]
> #ifdef REFINE_STATS
> after_last++;
> #endif
[..]
> #ifdef REFINE_STATS
> +if (block[j]) {
> + if (block[j] - best_change) {
> + if (FFABS(block[j]) > FFABS(block[j] - best_change)) {
> raise++;
> + } else {
> lower++;
> }
> + } else {
> from_zero++;
> }
> +} else {
> to_zero++;
> }
> #endif
This is not indented at all?
> #ifdef REFINE_STATS
> count++;
> +if (256 * 256 * 256 * 64 % count == 0) {
> + printf("after_last:%d to_zero:%d from_zero:%d raise:%d lower:%d
sign:%d xyp:%d/%d/%d\n", after_last,
> + to_zero, from_zero, raise, lower, messed_sign, s->mb_x,
s->mb_y, s->picture_number);
> }
> #endif
Same, plus >80 characters.
> #ifdef REFINE_STATS
> -if(last_non_zero>0){
> +if (last_non_zero > 0) {
> STOP_TIMER("iterative search")
> }
> }
See comments earlier. You can ignore the indenting of the STOP_TIMER and
the terminating } (which is part of STOP_TIMER), but the rest needs proper
indenting.
> @@ -3742,46 +3826,46 @@ int dct_quantize_c(MpegEncContext *s,
[..]
> + bias = s->intra_quant_bias<<(QMAT_SHIFT - QUANT_BIAS_SHIFT);
Spaces around <<.
> + bias = s->inter_quant_bias<<(QMAT_SHIFT - QUANT_BIAS_SHIFT);
Same.
> +// if ( bias+level >= (1<<QMAT_SHIFT)
> +// || bias-level >= (1<<QMAT_SHIFT)) {
Commented code indenting.
> + *overflow = s->max_qcoeff < max; //overflow might have happened
Space between // and comment text.
> /* we need this permutation so that we correct the IDCT, we only
permute the !=0 elements */
>80 characters.
> @@ -3813,7 +3897,7 @@ AVCodec ff_h263_encoder = {
[..]
> + .pix_fmts= (const enum PixelFormat[]) {PIX_FMT_YUV420P,
PIX_FMT_NONE},
[..]
> + .pix_fmts= (const enum PixelFormat[]) {PIX_FMT_YUV420P,
PIX_FMT_NONE},
[..]
> + .pix_fmts= (const enum PixelFormat[]) {PIX_FMT_YUV420P,
PIX_FMT_NONE},
[..]
> + .pix_fmts= (const enum PixelFormat[]) {PIX_FMT_YUV420P,
PIX_FMT_NONE},
[..]
> + .pix_fmts= (const enum PixelFormat[]) {PIX_FMT_YUV420P,
PIX_FMT_NONE},
Space after { and before }:
{ a, b }
Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
