On Mon, Jan 16, 2012 at 11:11:42PM +0200, Donald wrote: > Patch attached here.
> >From 0eea4ea1cd482941b4d8581e2b94a5729b928379 Mon Sep 17 00:00:00 2001 > From: Donald Ovcharov <[email protected]> > Date: Sat, 17 Dec 2011 21:30:01 +0200 > Subject: [PATCH] error_resilience:cosmetics So which is your official email address now? > --- a/libavcodec/error_resilience.c > +++ b/libavcodec/error_resilience.c > @@ -40,26 +40,42 @@ > > -static void decode_mb(MpegEncContext *s, int ref){ > - s->dest[0] = s->current_picture.f.data[0] + (s->mb_y * 16 * > s->linesize) + s->mb_x * 16; > - s->dest[1] = s->current_picture.f.data[1] + (s->mb_y * (16 >> > s->chroma_y_shift) * s->uvlinesize) + s->mb_x * (16 >> s->chroma_x_shift); > - s->dest[2] = s->current_picture.f.data[2] + (s->mb_y * (16 >> > s->chroma_y_shift) * s->uvlinesize) + s->mb_x * (16 >> s->chroma_x_shift); > - > - if(CONFIG_H264_DECODER && s->codec_id == CODEC_ID_H264){ > - H264Context *h= (void*)s; > - h->mb_xy= s->mb_x + s->mb_y*s->mb_stride; > +static void decode_mb(MpegEncContext *s, int ref) > +{ > + s->dest[0] = s->current_picture.f.data[0] + > + (s->mb_y * 16 * s->linesize) + > + s->mb_x * 16; Why two spaces after *? > + s->dest[1] = s->current_picture.f.data[1] + > + (s->mb_y * (16 >> s->chroma_y_shift) * > + s->uvlinesize) + s->mb_x * > + (16 >> s->chroma_x_shift); > + s->dest[2] = s->current_picture.f.data[2] + > + (s->mb_y * (16 >> s->chroma_y_shift) * > + s->uvlinesize) + s->mb_x * > + (16 >> s->chroma_x_shift); The linebreaks look pretty arbitrary to me. You could keep the expressions in parentheses on the same line for example. > fill_rectangle(&h->ref_cache[0][scan8[0]], 4, 4, 8, ref, 1); > - fill_rectangle(h->mv_cache[0][ scan8[0] ], 4, 4, 8, > pack16to32(s->mv[0][0][0],s->mv[0][0][1]), 4); > + fill_rectangle(h->mv_cache[0][scan8[0]], 4, 4, 8, > + pack16to32(s->mv[0][0][0],s->mv[0][0][1]), 4); space after comma > @@ -67,82 +83,92 @@ static void decode_mb(MpegEncContext *s, int ref){ > + for (i = 0; i < 4; i++) { > + dc = s->dc_val[0][ mb_x * 2 + (i & 1) + > + (mb_y * 2 + (i >> 1)) * s->b8_stride]; If you prettyprint this so much, then align the '1' as well. > - for(x=0; x<8; x++){ > - dest_y[x + (i&1)*8 + (y + (i>>1)*8)*s->linesize]= dc/8; > + for (x = 0; x < 8; x++) { > + dest_y[ x + (i & 1) * 8 + > + (y + (i >> 1) * 8) * s->linesize] = dc / 8; same > - for(x=0; x<8; x++){ > - dest_cb[x + y*(s->uvlinesize)]= dcu/8; > - dest_cr[x + y*(s->uvlinesize)]= dcv/8; > + for (x = 0; x < 8; x++) { > + dest_cb[x + y * (s->uvlinesize)] = dcu / 8; > + dest_cr[x + y * (s->uvlinesize)] = dcv / 8; This is not against HEAD. You apparently never updated your tree, but you must; only patches against the current HEAD version of Libav are valid. I would also like to know if you at least test-compiled this. I'm stopping the review here. Before you rebase your tree against HEAD it is pointless as your patch cannot be applied anyway. Happy rebasing! Diego _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
