Hi, On Sun, Dec 25, 2011 at 2:09 PM, Ronald S. Bultje <rsbul...@gmail.com>wrote:
> Until line 537. Rest will come in a separate review. > > @@ -537,150 +614,167 @@ skip_mean_and_median: [..] > + prev_x = s->last_picture.f.motion_val[0][mot_index][0]; > + prev_y = s->last_picture.f.motion_val[0][mot_index][1]; > + prev_ref = s->last_picture.f.ref_index[0][4 * mb_xy]; [..] > + prev_x = s->current_picture.f.motion_val[0][mot_index][0]; > + prev_y = s->current_picture.f.motion_val[0][mot_index][1]; > + prev_ref = s->current_picture.f.ref_index[0][4 * mb_xy]; 80 characters. Same as what I said before, you can fix this by making the motion_val[0] and ref_index[0] local variables so the lines become shorter. > + uint8_t *src = s->current_picture.f.data[0] + mb_x * 16 + > + mb_y * 16 * s->linesize; Break at the first + (instead of the second) so it fits on 80 characters. > - if(ref[j]<0) //predictor intra or otherwise not available > + if (ref[j] < 0) // predictor intra or otherwise not available > continue; 80 characters. This you can fix by putting the comment on its own line: // comment if (..) continue; > + for (k = 0; k < 16; k++) > + score += FFABS(src[k * s->linesize - 1] - src[k * s->linesize]); 80 characters. fix by e.g. breaking at the -: score += FFABS(a - b); > + for (j = 0; j < mot_step; j++) { > + s->current_picture.f.motion_val[0][mot_index + i + j * > + mot_stride][0] = s->mv[0][0][0]; > + s->current_picture.f.motion_val[0][mot_index + i + j * > + mot_stride][1] = s->mv[0][0][1]; > } 80 characters, probably best solved by making s->current_picture.f.motion_val[0] a local variable. > +static int is_intra_more_likely(MpegEncContext *s) [..] > if (h->list_count <= 0 || h->ref_count[0] <= 0 || !h->ref_list[0][0].f.data[0]) 80 characters. > @@ -688,9 +782,11 @@ static int is_intra_more_likely(MpegEncContext *s){ [..] > + is_intra_likely -= s->dsp.sad[0](NULL, last_mb_ptr, last_mb_ptr + > + s->linesize * 16,s->linesize, 16); 80 characters, plus space after each , (see the missing one on the last line). > @@ -698,15 +794,17 @@ static int is_intra_more_likely(MpegEncContext *s){ [..] > -//printf("is_intra_likely: %d type:%d\n", is_intra_likely, s->pict_type); > +// printf("is_intra_likely: %d type:%d\n", is_intra_likely, s->pict_type); > return is_intra_likely > 0; > } Please indent the commented line correctly, i.e.: // printf(..); return ..; } > +void ff_er_frame_start(MpegEncContext *s) { { goes on its own line. > @@ -716,99 +814,111 @@ void ff_er_frame_start(MpegEncContext *s){ [..] > * @param status the status at the end (ER_MV_END, ER_AC_ERROR, ...), it is assumed that no earlier end or 80 characters. > +void ff_er_add_slice(MpegEncContext *s, int startx, int starty, > + int endx, int endy, int status) [..] > + const int start_i = av_clip(startx + starty * s->mb_width, 0, s->mb_num - 1); > + const int end_i = av_clip(endx + endy * s->mb_width, 0, s->mb_num); 80 characters. > + if (start_i > end_i || start_xy > end_xy) { > av_log(s->avctx, AV_LOG_ERROR, "internal error, slice end before start\n"); > return; > } Same. > + if (status & (ER_AC_ERROR | ER_AC_END)) { > mask &= ~(ER_AC_ERROR|ER_AC_END); Spaces around |. > + if (status & (ER_DC_ERROR | ER_DC_END)) { > mask &= ~(ER_DC_ERROR|ER_DC_END); Same. > + if (status & (ER_MV_ERROR | ER_MV_END)) { > mask &= ~(ER_MV_ERROR|ER_MV_END); Same. > + for (i = start_xy; i < end_xy; i++) { > s->error_status_table[ i ] &= mask; > } No spaces after [ or before ]. > +void ff_er_frame_end(MpegEncContext *s) [..] > + if (!s->err_recognition || s->error_count == 0 || s->avctx->lowres || > s->avctx->hwaccel || > s->avctx->codec->capabilities&CODEC_CAP_HWACCEL_VDPAU || > + s->picture_structure != PICT_FRAME || > + // we do not support ER of field pictures yet, > + // though it should not crash if enabled > + s->error_count == 3 * s->mb_width * (s->avctx->skip_top + s->avctx->skip_bottom)) { > + return; > + }; Wrong indenting. Everything in the if() should vertically align with the character after the opening bracket, not the opening bracket itself: if (a || b || ..) { return; } Also the last line is >80 characters, and the field-comment should go on the line above. > + for (i = 0; i < 2; i++) { > + pic->f.ref_index[i] = av_mallocz(s->mb_stride * > + s->mb_height * 4 * sizeof(uint8_t)); > + pic->motion_val_base[i] = av_mallocz((size + 4) * 2 * sizeof(uint16_t)); > + pic->f.motion_val[i] = pic->motion_val_base[i] + 4; > } 2 lines are >80 characters. > + if (s->avctx->debug&FF_DEBUG_ER) { Spaces around &. > @@ -817,140 +927,142 @@ void ff_er_frame_end(MpegEncContext *s){ [..] > + for (i = s->mb_num - 1; i >= 0; i--) { > + const int mb_xy = s->mb_index2xy[i]; > + int error = s->error_status_table[mb_xy]; "int error =", not "int error =". > + if ((error & ER_MV_END) || (error & ER_DC_END) || (error & ER_AC_ERROR)) > + end_ok = 1; >80 characters. > + if (error&VP_START) > + end_ok = 0; Spaces around &. > + for (i = s->mb_num - 2; i >= s->mb_width + 100; i--) { // FIXME + 100 hack >80 characters; fix it by putting the comment on the line before. > + if (error2 == (VP_START | ER_MB_ERROR | ER_MB_END) && > + error1 != (VP_START | ER_MB_ERROR | ER_MB_END) && > + ((error1 & ER_AC_END) || (error1 & ER_DC_END) || (error1 & ER_MV_END))) { > + // end & uninit > + end_ok = 0; > } Last line in the if() is >80 characters. > + for (i = 0; i < s->mb_num; i++) { > + const int mb_xy = s->mb_index2xy[i]; > + int old_error = s->error_status_table[mb_xy]; "int old_error =", not "int old_error =". > + if (old_error&VP_START) > + error = old_error & ER_MB_ERROR; Spaces around &. > @@ -958,164 +1070,186 @@ void ff_er_frame_end(MpegEncContext *s){ [..] > + for (mb_y = 0; mb_y < s->mb_height; mb_y++) { > + for (mb_x = 0; mb_x < s->mb_width; mb_x++) { > + const int mb_xy = mb_x + mb_y * s->mb_stride; > + const int mb_type = s->current_picture.f.mb_type[mb_xy]; > + int dir = !s->last_picture.f.data[0]; "int dir =", not "int dir =". > + if (IS_INTRA(mb_type)) > + continue; // intra > + if (error & ER_MV_ERROR) > + continue; // inter with damaged MV > + if (!(error & ER_AC_ERROR)) > + continue; // undamaged inter One space between ; and // is enough. > + for (j = 0; j < 4; j++) { > + s->mv[0][j][0] = > + s->current_picture.f.motion_val[dir][mb_index + (j & 1) + > + (j >> 1) * s->b8_stride][0]; > + s->mv[0][j][1] = > + s->current_picture.f.motion_val[dir][mb_index + (j & 1) + > + (j >> 1) * s->b8_stride][1]; > } >80 characters for all of them; solve by making s->current_picture.f.motion_val[dir] a local variable. > + } else { > + s->mv_type = MV_TYPE_16X16; > + s->mv[0][0][0] = s->current_picture.f.motion_val[dir][mb_x * 2 + mb_y * 2 * > + s->b8_stride][0]; > + s->mv[0][0][1] = s->current_picture.f.motion_val[dir][mb_x * 2 + mb_y * 2 * > + s->b8_stride][1]; > } Same. > /* guess MVs */ [..] > + if (!(error & ER_MV_ERROR)) > + continue; // inter with undamaged MV > + if (!(error & ER_AC_ERROR)) > + continue; // undamaged inter 1 space between ; and // is enough. > + if (!s->last_picture.f.data[0]) s->mv_dir &= ~MV_DIR_FORWARD; > + if (!s->next_picture.f.data[0]) s->mv_dir &= ~MV_DIR_BACKWARD; Please split if() code into: if (..) code; > /* the filters below are not XvMC compatible, skip them */ [..] > + for (mb_y = 0; mb_y < s->mb_height; mb_y++) { > + for (mb_x = 0; mb_x < s->mb_width; mb_x++) { > + int dc, dcu, dcv, y, n; > + int16_t *dc_ptr; > + uint8_t *dest_y, *dest_cb, *dest_cr; You can remove the spaces between variable type and variable name. > dest_y = s->current_picture.f.data[0] + mb_x * 16 + mb_y * 16 * s->linesize; > dest_cb = s->current_picture.f.data[1] + mb_x * 8 + mb_y * 8 * s->uvlinesize; > dest_cr = s->current_picture.f.data[2] + mb_x * 8 + mb_y * 8 * s->uvlinesize; >80 characters. > + for (x = 0; x < 8; x++) { > + dc += dest_y[x + (n & 1) * 8 + (y + (n >> 1) * 8) * s->linesize]; > } >80 characters. > /* guess DC for damaged blocks */ > + guess_dc(s, s->dc_val[0], s->mb_width * 2, s->mb_height * 2, s->b8_stride, 1); > + guess_dc(s, s->dc_val[1], s->mb_width, s->mb_height, s->mb_stride, 0); > + guess_dc(s, s->dc_val[2], s->mb_width, s->mb_height, s->mb_stride, 0); >80 characters. > /* render DC only intra */ [..] > + if (!(error & ER_AC_ERROR)) > + continue; // undamaged One space between ; and // is enough. > dest_y = s->current_picture.f.data[0] + mb_x * 16 + mb_y * 16 * s->linesize; > dest_cb = s->current_picture.f.data[1] + mb_x * 8 + mb_y * 8 * s->uvlinesize; >80 characters. > @@ -1125,27 +1259,34 @@ void ff_er_frame_end(MpegEncContext *s){ [..] > + h_block_filter(s, s->current_picture.f.data[0], s->mb_width * 2, > + s->mb_height * 2, s->linesize, 1); > + h_block_filter(s, s->current_picture.f.data[1], s->mb_width , > + s->mb_height , s->uvlinesize, 0); > + h_block_filter(s, s->current_picture.f.data[2], s->mb_width , > + s->mb_height , s->uvlinesize, 0); Remove the spaces before the trailing comma on each line. > /* filter vertical block boundaries */ [..] > + v_block_filter(s, s->current_picture.f.data[0], s->mb_width * 2, > + s->mb_height * 2, s->linesize, 1); > + v_block_filter(s, s->current_picture.f.data[1], s->mb_width , > + s->mb_height , s->uvlinesize, 0); > + v_block_filter(s, s->current_picture.f.data[2], s->mb_width , > + s->mb_height , s->uvlinesize, 0); Same. Ronald
_______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel