Hi,
On Sat, Dec 24, 2011 at 9:05 PM, Ronald S. Bultje <[email protected]>wrote:
> Until line 2489.
>
> @@ -2484,69 +2539,69 @@ static int encode_thread(AVCodecContext *c, void
*arg){
[..]
> + avpriv_copy_bits(&backup_s.pb, bit_buf[next_block ^ 1],
pb_bits_count);
>80 characters.
> + avpriv_copy_bits(&backup_s.pb2, bit_buf2[next_block
^ 1], pb2_bits_count);
Same.
> + avpriv_copy_bits(&backup_s.tex_pb,
bit_buf_tex[next_block ^ 1], tex_pb_bits_count);
Same.
> + if (next_block == 0) { //FIXME 16 vs linesize16
> + s->dsp.put_pixels_tab[0][0](s->dest[0],
s->rd_scratchpad , s->linesize , 16);
> + s->dsp.put_pixels_tab[1][0](s->dest[1],
s->rd_scratchpad + 16 * s->linesize , s->uvlinesize, 8);
> + s->dsp.put_pixels_tab[1][0](s->dest[2],
s->rd_scratchpad + 16 * s->linesize + 8, s->uvlinesize, 8);
> }
Same.
> + for (i = 0; i < 2; i++) {
> + j = s->field_select[0][i] =
s->p_field_select_table[i][xy];
Same.
> + for (i = 0; i < 4; i++) {
> s->mv[0][i][0] =
s->current_picture.f.motion_val[0][s->block_index[i]][0];
> s->mv[0][i][1] =
s->current_picture.f.motion_val[0][s->block_index[i]][1];
> }
Same.
> @@ -2554,22 +2609,22 @@ static int encode_thread(AVCodecContext *c, void
*arg){
[..]
> case CANDIDATE_MB_TYPE_DIRECT0:
> if (CONFIG_MPEG4_ENCODER) {
> s->mv_dir =
MV_DIR_FORWARD|MV_DIR_BACKWARD|MV_DIRECT;
Spaces around |.
> @@ -2577,44 +2632,44 @@ static int encode_thread(AVCodecContext *c, void
*arg){
[..]
> + s->mb_intra = 0;
> + motion_x = s->mv[0][0][0] =
s->b_forw_mv_table[xy][0];
> + motion_y = s->mv[0][0][1] =
s->b_forw_mv_table[xy][1];
> // printf(" %d %d ", motion_x, motion_y);
Mis-indented commented code.
> + for (i = 0; i < 2; i++) {
> + j = s->field_select[0][i] =
s->b_field_select_table[0][i][xy];
>80 characters.
> + for (i = 0; i < 2; i++) {
> + j = s->field_select[1][i] =
s->b_field_select_table[1][i][xy];
Same.
> + for (i = 0; i < 2; i++) {
> + j = s->field_select[dir][i] =
s->b_field_select_table[dir][i][xy];
> s->mv[dir][i][0] =
s->b_field_mv_table[dir][i][j][xy][0];
> s->mv[dir][i][1] =
s->b_field_mv_table[dir][i][j][xy][1];
Same x3.
> @@ -2637,30 +2692,32 @@ static int encode_thread(AVCodecContext *c, void
*arg){
[..]
> + if (s->flags&CODEC_FLAG_PSNR) {
Spaces around &.
> s->current_picture.f.error[0] += sse(
> - s, s->new_picture.f.data[0] + s->mb_x*16 +
s->mb_y*s->linesize*16,
> + s, s->new_picture.f.data[0] + s->mb_x * 16 + s->mb_y
* s->linesize * 16,
> s->dest[0], w, h, s->linesize);
> s->current_picture.f.error[1] += sse(
> - s, s->new_picture.f.data[1] + s->mb_x*8 +
s->mb_y*s->uvlinesize*chr_h,
> - s->dest[1], w>>1, h>>s->chroma_y_shift,
s->uvlinesize);
> + s, s->new_picture.f.data[1] + s->mb_x * 8 + s->mb_y
* s->uvlinesize * chr_h,
> + s->dest[1], w >> 1, h >> s->chroma_y_shift,
s->uvlinesize);
> s->current_picture.f.error[2] += sse(
> - s, s->new_picture.f.data[2] + s->mb_x*8 +
s->mb_y*s->uvlinesize*chr_h,
> - s->dest[2], w>>1, h>>s->chroma_y_shift,
s->uvlinesize);
> + s, s->new_picture.f.data[2] + s->mb_x * 8 + s->mb_y
* s->uvlinesize * chr_h,
> + s->dest[2], w >> 1, h >> s->chroma_y_shift,
s->uvlinesize);
Yeah, no. >80 characters, but this is just hideous. How about introducing
some temp variable called "int err;" and doing:
err = sse(bla, bla,
bla, bla
bla, bla);
s->current_picture.f.error[x] += err;
?
> //printf("MB %d %d bits\n", s->mb_x+s->mb_y*s->mb_stride,
put_bits_count(&s->pb));
Mis-indented commented code.
> @@ -2668,14 +2725,14 @@ static int encode_thread(AVCodecContext *c, void
*arg){
[..]
> //not beautiful here but we must write it before flushing so it has
to be here
Space between // and comment.
> + if (CONFIG_MSMPEG4_ENCODER && s->msmpeg4_version &&
s->msmpeg4_version < 4 && s->pict_type == AV_PICTURE_TYPE_I)
>80 characters.
> +static void merge_context_after_encode(MpegEncContext *dst,
MpegEncContext *src)
[..]
MERGE(dct_count[0]); //note, the other dct vars are not part of the
context
Space between // and commented code. Better yet, put comment on line before.
> @@ -2752,9 +2813,9 @@ static int estimate_qp(MpegEncContext *s, int
dry_run){
[..]
> + s->lambda = s->lambda_table[0];
> //FIXME broken
Same.
> s->lambda = s->current_picture.f.quality;
> //printf("%d %d\n", s->avctx->global_quality,
s->current_picture.quality);
Mis-indented commented code.
> @@ -2792,100 +2854,102 @@ static int encode_picture(MpegEncContext *s,
int picture_number)
[..]
> // RAL: Condition added for MPEG1VIDEO
> if (s->codec_id == CODEC_ID_MPEG1VIDEO || s->codec_id ==
CODEC_ID_MPEG2VIDEO || (s->h263_pred && !s->msmpeg4_version))
> set_frame_distances(s);
>80 characters.
> + s->me.scene_change_score = 0;
>
> // s->lambda= s->current_picture_ptr->quality; //FIXME qscale / ...
stuff for ME rate distortion
Mis-indented commented code.
> + if (s->flipflop_rounding || s->codec_id == CODEC_ID_H263P ||
s->codec_id == CODEC_ID_MPEG4)
> s->no_rounding ^= 1;
>80 character.s
> + s->mb_intra = 0; //for the rate distortion & bit compare functions
Space between // and comment.
> + if (s->pict_type != AV_PICTURE_TYPE_I) {
> + s->lambda = (s->lambda * s->avctx->me_penalty_compensation +
128) >> 8;
> + s->lambda2= (s->lambda2 *
(int64_t)s->avctx->me_penalty_compensation + 128) >> 8;
> + if (s->pict_type != AV_PICTURE_TYPE_B && s->avctx->me_threshold
== 0) {
> + if ((s->avctx->pre_me && s->last_non_b_pict_type ==
AV_PICTURE_TYPE_I) || s->avctx->pre_me == 2) {
> s->avctx->execute(s->avctx, pre_estimate_motion_thread,
&s->thread_context[0], NULL, context_count, sizeof(void*));
> }
> }
> s->avctx->execute(s->avctx, estimate_motion_thread,
&s->thread_context[0], NULL, context_count, sizeof(void*));
>80 characters several times.
> + if (!s->fixed_qscale) {
> /* finding spatial complexity for I-frame rate control */
> s->avctx->execute(s->avctx, mb_var_thread,
&s->thread_context[0], NULL, context_count, sizeof(void*));
> }
Same.
> + s->current_picture.mc_mb_var_sum =
s->current_picture_ptr->mc_mb_var_sum = s->me.mc_mb_var_sum_temp;
> + s->current_picture. mb_var_sum = s->current_picture_ptr->
mb_var_sum = s->me. mb_var_sum_temp;
Same.
> + if (s->me.scene_change_score > s->avctx->scenechange_threshold &&
s->pict_type == AV_PICTURE_TYPE_P) {
Same.
> + for (i = 0; i < s->mb_stride * s->mb_height; i++)
> + s->mb_type[i] = CANDIDATE_MB_TYPE_INTRA;
> //printf("Scene change detected, encoding as I Frame %d %d\n",
s->current_picture.mb_var_sum, s->current_picture.mc_mb_var_sum);
> }
Mis-indented commented code, and >80 characters.
> + if (!s->umvplus) {
> + if (s->pict_type == AV_PICTURE_TYPE_P || s->pict_type ==
AV_PICTURE_TYPE_S) {
> s->f_code= ff_get_best_fcode(s, s->p_mv_table,
CANDIDATE_MB_TYPE_INTER);
>80 characters x2.
> + a = ff_get_best_fcode(s, s->p_field_mv_table[0][0],
CANDIDATE_MB_TYPE_INTER_I); //FIXME field_select
> + b = ff_get_best_fcode(s, s->p_field_mv_table[1][1],
CANDIDATE_MB_TYPE_INTER_I);
Same.
> ff_fix_long_mvs(s, NULL, 0, s->p_mv_table, s->f_code,
CANDIDATE_MB_TYPE_INTER, 0);
Same.
> ff_fix_long_mvs(s, s->p_field_select_table[i], j,
> s->p_field_mv_table[i][j],
s->f_code, CANDIDATE_MB_TYPE_INTER_I, 0);
Same.
> a = ff_get_best_fcode(s, s->b_forw_mv_table,
CANDIDATE_MB_TYPE_FORWARD);
Same.
>@@ -2900,13 +2964,13 @@ static int encode_picture(MpegEncContext *s, int
picture_number)
[..]
> ff_fix_long_mvs(s, NULL, 0, s->b_back_mv_table, s->b_code,
CANDIDATE_MB_TYPE_BACKWARD, 1);
> ff_fix_long_mvs(s, NULL, 0, s->b_bidir_forw_mv_table,
s->f_code, CANDIDATE_MB_TYPE_BIDIR, 1);
> ff_fix_long_mvs(s, NULL, 0, s->b_bidir_back_mv_table,
s->b_code, CANDIDATE_MB_TYPE_BIDIR, 1);
>80 characters.
> + for (dir = 0; dir < 2; dir++) {
> + for (i = 0; i < 2; i++) {
> + for (j = 0; j < 2; j++) {
> + int type = dir ?
(CANDIDATE_MB_TYPE_BACKWARD_I | CANDIDATE_MB_TYPE_BIDIR_I)
> + :
(CANDIDATE_MB_TYPE_FORWARD_I | CANDIDATE_MB_TYPE_BIDIR_I);
> ff_fix_long_mvs(s,
s->b_field_select_table[dir][i], j,
>
s->b_field_mv_table[dir][i][j], dir ? s->b_code : s->f_code, type, 1);
> }
>80 characters.
> @@ -2919,22 +2983,22 @@ static int encode_picture(MpegEncContext *s, int
picture_number)
[..]
> + if (s->qscale < 3 && s->max_qcoeff <= 128 && s->pict_type ==
AV_PICTURE_TYPE_I && !(s->flags & CODEC_FLAG_QSCALE))
> + s->qscale = 3; //reduce clipping problems
>80 characters.
> if (s->out_format == FMT_MJPEG) {
> /* for mjpeg, we do include qscale in the matrix */
> + for (i = 1; i < 64; i++) {
> + int j = s->dsp.idct_permutation[i];
>
> s->intra_matrix[j] =
av_clip_uint8((ff_mpeg1_default_intra_matrix[i] * s->qscale) >> 3);
> }
>80 characters.
> + s->y_dc_scale_table =
> + s->c_dc_scale_table =
ff_mpeg2_dc_scale_table[s->intra_dc_precision];
> + s->intra_matrix[0] =
ff_mpeg2_dc_scale_table[s->intra_dc_precision][8];
> ff_convert_matrix(&s->dsp, s->q_intra_matrix,
s->q_intra_matrix16,
> s->intra_matrix, s->intra_quant_bias, 8, 8, 1);
Reindent so the s->... of the last line vertically aligns with the
character after the opening bracket in the second-to-last line.
> //FIXME var duplication
Space between // and comment text.
> @@ -2981,76 +3045,80 @@ static int encode_picture(MpegEncContext *s, int
picture_number)
[..]
> + bits = put_bits_count(&s->pb);
> s->header_bits= bits - s->last_bits;
Spaces around '='.
> s->avctx->execute(s->avctx, encode_thread, &s->thread_context[0],
NULL, context_count, sizeof(void*));
>80 characters.
> static int dct_quantize_trellis_c(MpegEncContext *s,
[..]
> const uint8_t *scantable= s->intra_scantable.scantable;
> const uint8_t *perm_scantable= s->intra_scantable.permutated;
Spaces around '='.
> uint8_t * length;
> uint8_t * last_length;
No space between * and variable name, i.e.:
uint8_t *length;
> s->dsp.fdct (block);
No space between function name and argumentlist opening bracket, i.e.:
s->dsp.fdct(block);
Until line 3060.
Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
