Hi,
On Mon, Dec 5, 2011 at 2:21 PM, KJT TOD <[email protected]> wrote:
[..]
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> @@ -589,159 +677,205 @@ av_cold int MPV_common_init(MpegEncContext *s)
> threads = (s->encoding ||
> (HAVE_THREADS &&
> s->avctx->active_thread_type & FF_THREAD_SLICE)) ?
> - s->avctx->thread_count : 1;
> + s->avctx->thread_count : 1;
This is wrong, original indentation was correct.
> - /* set default edge pos, will be overriden in decode_header if
> needed */
> - s->h_edge_pos= s->mb_width*16;
> - s->v_edge_pos= s->mb_height*16;
> + /* set default edge pos, will be overriden
> + * in decode_header if needed */
> + s->h_edge_pos = s->mb_width * 16;
> + s->v_edge_pos = s->mb_height * 16;
Why the second space after the '='? 1 is enough.
> - FF_ALLOCZ_OR_GOTO(s->avctx, s->mb_index2xy,
> (s->mb_num+1)*sizeof(int), fail) //error ressilience code looks cleaner with
> this
> - for(y=0; y<s->mb_height; y++){
> - for(x=0; x<s->mb_width; x++){
> - s->mb_index2xy[ x + y*s->mb_width ] = x + y*s->mb_stride;
> - }
> - }
> - s->mb_index2xy[ s->mb_height*s->mb_width ] =
> (s->mb_height-1)*s->mb_stride + s->mb_width; //FIXME really needed?
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) *
> sizeof(int),
> + fail); // error ressilience code looks cleaner
> with this
One space between ';' and '//' is enough.
> + for (y = 0; y < s->mb_height; y++)
> + for (x = 0; x < s->mb_width; x++)
> + s->mb_index2xy[x + y * s->mb_width] = x + y * s->mb_stride;
> +
> + s->mb_index2xy[s->mb_height * s->mb_width] =
> + (s->mb_height - 1) * s->mb_stride + s->mb_width;
> // FIXME really needed?
Same here.
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->p_mv_table_base ,
> + mv_table_size * 2 * sizeof(int16_t), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base ,
> + mv_table_size * 2 * sizeof(int16_t), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base ,
> + mv_table_size * 2 * sizeof(int16_t), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base ,
> + mv_table_size * 2 * sizeof(int16_t), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base ,
> + mv_table_size * 2 * sizeof(int16_t), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base ,
> + mv_table_size * 2 * sizeof(int16_t), fail);
All spaces before the last ',' on the first line can be removed. Same
for all occurrences below:
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->mb_type, mb_array_size *
> + sizeof(uint16_t), fail) // needed for
> encoding
> +
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size *
> + sizeof(int), fail);
> +
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->q_intra_matrix ,
> + 64 * 32 * sizeof(int), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->q_inter_matrix ,
> + 64 * 32 * sizeof(int), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->q_intra_matrix16,
> + 64 * 32 * 2 * sizeof(uint16_t), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->q_inter_matrix16,
> + 64 * 32 * 2 * sizeof(uint16_t), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->input_picture, MAX_PICTURE_COUNT *
> + sizeof(Picture *), fail);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->reordered_input_picture,
> + MAX_PICTURE_COUNT * sizeof(Picture *), fail);
Please add a ';' at the end of each macro. Then remove all but one of
the spaces between ';' and '//', and lastly remove spaces before the
last ',' of each line.
> - FF_ALLOCZ_OR_GOTO(s->avctx, s->picture, s->picture_count *
> sizeof(Picture), fail)
> - for(i = 0; i < s->picture_count; i++) {
> - avcodec_get_frame_defaults((AVFrame *)&s->picture[i]);
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->picture, s->picture_count *
> + sizeof(Picture), fail);
Please split this line such that individual arguments are on the same
line, i.e. "s->picture_count * sizeof(Picture)" can fit on a single
line.
> /* cbp, ac_pred, pred_dir */
> - FF_ALLOCZ_OR_GOTO(s->avctx, s->cbp_table , mb_array_size *
> sizeof(uint8_t), fail)
> - FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size *
> sizeof(uint8_t), fail)
> + FF_ALLOCZ_OR_GOTO(s->avctx, s->cbp_table ,
> + mb_array_size * sizeof(uint8_t), fail);
Remove spaces before last ',' on first line.
> - for(i=0; i<threads; i++){
> - if(init_duplicate_context(s->thread_context[i], s) < 0)
> + for (i = 0; i < threads; i++) {
> + if (init_duplicate_context(s->thread_context[i], s) < 0)
> + goto fail;
> + s->thread_context[i]->start_mb_y =
> + (s->mb_height * (i) + s->avctx->thread_count / 2) /
> + s->avctx->thread_count;
> + s->thread_context[i]->end_mb_y =
> + (s->mb_height * (i + 1) + s->avctx->thread_count / 2) /
> + s->avctx->thread_count;
This is still not correct. The "(s->mb_height * (i .." lines should
get 4 more spaces indentation.
> - if (s->encoding || (HAVE_THREADS &&
> s->avctx->active_thread_type&FF_THREAD_SLICE)) {
> - for(i=0; i<s->avctx->thread_count; i++){
> + if (s->encoding || (HAVE_THREADS &&
> + s->avctx->active_thread_type&FF_THREAD_SLICE)) {
Spaces around '&'.
> - if(!(s->avctx->active_thread_type&FF_THREAD_FRAME))
> + if (!(s->avctx->active_thread_type&FF_THREAD_FRAME))
Same.
> -void init_rl(RLTable *rl, uint8_t static_store[2][2*MAX_RUN + MAX_LEVEL + 3])
> +void init_rl(RLTable *rl, uint8_t static_store[2][2 *MAX_RUN
> + + MAX_LEVEL + 3])
Start a new line (vertically aligned with the 'R' in "RLTable") at
"uint8_t static...", so that that function argument itself fits on one
line. Also "2 * MAX_RUN", i.e. spaces around the '*'.
> - if(static_store)
> + if (static_store)
> rl->max_level[last] = static_store[last];
> else
> - rl->max_level[last] = av_malloc(MAX_RUN + 1);
> + rl->max_level[last] = av_malloc( MAX_RUN + 1);
> memcpy(rl->max_level[last], max_level, MAX_RUN + 1);
Heh, nice. :-). Looks a little funny. Maybe just keep this the way it
was (i.e. remove the spaces before "MAX_RUN" in the av_malloc() line).
> - rl->max_run[last] = av_malloc(MAX_LEVEL + 1);
> - memcpy(rl->max_run[last], max_run, MAX_LEVEL + 1);
> - if(static_store)
> + rl->max_run[last] = av_malloc(MAX_LEVEL + 1);
> + memcpy(rl->max_run[last], max_run, MAX_LEVEL + 1);
> + if (static_store)
> rl->index_run[last] = static_store[last] + MAX_RUN + MAX_LEVEL +
> 2;
> else
> - rl->index_run[last] = av_malloc(MAX_RUN + 1);
> + rl->index_run[last] = av_malloc( MAX_RUN + 1);
> memcpy(rl->index_run[last], index_run, MAX_RUN + 1);
Same here for the memcpy() line and the av_malloc() line that you changed.
> + } else {
> + run = rl->table_run[code] + 1;
> + level = rl->table_level[code] * qmul + qadd;
> + if (code >= rl->last) run += 192;
Same here - very inventive, but '>=' and '=' are very different, so no
need to vertically align these. The '=' for run/level should of course
be aligned, nice work there.
> /* release non reference frames */
> - for(i=0; i<s->picture_count; i++){
> + for (i = 0; i < s->picture_count; i++) {
> if (s->picture[i].f.data[0] && !s->picture[i].f.reference
> - && (!s->picture[i].owner2 || s->picture[i].owner2 == s)
> - && (remove_current || &s->picture[i] != s->current_picture_ptr)
> - /*&& s->picture[i].type!=FF_BUFFER_TYPE_SHARED*/){
> + && (!s->picture[i].owner2 || s->picture[i].owner2 == s)
> + && (remove_current || &s->picture[i] != s->current_picture_ptr)
> + /*&& s->picture[i].type!= FF_BUFFER_TYPE_SHARED*/) {
I would vote to put the '&&' on the preceeding lines.
Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
