On Mon, Mar 31, 2014 at 06:52:03PM +0200, Vittorio Giovara wrote:
> --- a/libavcodec/vp3.c
> +++ b/libavcodec/vp3.c
> @@ -77,51 +78,50 @@ typedef struct Vp3Fragment {
> { MODE_INTER_LAST_MV, MODE_INTER_PRIOR_LAST,
> MODE_INTER_PLUS_MV, MODE_INTER_NO_MV,
> MODE_INTRA, MODE_USING_GOLDEN,
> - MODE_GOLDEN_MV, MODE_INTER_FOURMV },
> -
> + MODE_GOLDEN_MV, MODE_INTER_FOURMV
> + },
Why?
> @@ -173,14 +173,14 @@ typedef struct Vp3DecodeContext {
>
> - int8_t (*motion_val[2])[2];
> + int8_t(*motion_val[2])[2];
Please don't blindly trust your uncrustify config, sometimes it screws up.
> @@ -355,33 +358,33 @@ static void init_dequantizer(Vp3DecodeContext *s, int
> qpi)
> - int coeff= ( 2*(sum -s->qps[qpi])*s->base_matrix[bmi][i]
> - - 2*(qistart-s->qps[qpi])*s->base_matrix[bmj][i]
> - + s->qr_size[inter][plane][qri])
> - / (2*s->qr_size[inter][plane][qri]);
> -
> + int coeff = (2 * (sum - s->qps[qpi]) *
> s->base_matrix[bmi][i] -
> + 2 * (qistart - s->qps[qpi]) *
> s->base_matrix[bmj][i] +
> + s->qr_size[inter][plane][qri]) /
> + (2 * s->qr_size[inter][plane][qri]);
Here you could align some more.
> - // all DC coefficients use the same quant so as not to interfere
> with DC prediction
> + /* all DC coefficients use the same quant so as not to interfere
> + * with DC prediction */
> s->qmat[qpi][inter][plane][0] = s->qmat[0][inter][plane][0];
> }
> - }
> }
This is uncrustify doing overzealous {} removal, keep it.
> @@ -557,14 +558,14 @@ static int unpack_superblocks(Vp3DecodeContext *s,
> GetBitContext *gb)
> s->all_fragments[current_fragment].coding_method =
> MODE_COPY;
> }
> + }
> }
> - }
> - }
same
> @@ -610,65 +609,70 @@ static int unpack_modes(Vp3DecodeContext *s,
> GetBitContext *gb)
> - else
> - coding_mode = alphabet
> - [get_vlc2(gb, s->mode_code_vlc.table, 3, 3)];
> + else
> + coding_mode = alphabet
> + [get_vlc2(gb, s->mode_code_vlc.table,
> 3, 3)];
Please unbreak this line.
> + } else {
> + for (k = 0; k < 4; k++) {
> + frag = s->all_fragments +
> + BLOCK_Y * s->fragment_width[1] + BLOCK_X;
> + SET_CHROMA_MODES
> + }
> }
> }
> }
> - }
> }
> }
Looks like more overzealous {} removal.
> @@ -701,157 +705,157 @@ static int unpack_vectors(Vp3DecodeContext *s,
> GetBitContext *gb)
>
> /* iterate through all of the macroblocks that contain 1 or more
> * coded fragments */
> - for (sb_y = 0; sb_y < s->y_superblock_height; sb_y++) {
> + for (sb_y = 0; sb_y < s->y_superblock_height; sb_y++)
> for (sb_x = 0; sb_x < s->y_superblock_width; sb_x++) {
> if (get_bits_left(gb) <= 0)
> return -1;
> }
> }
> }
> }
> - }
> - }
same
> @@ -911,30 +915,31 @@ static int unpack_block_qpis(Vp3DecodeContext *s,
> GetBitContext *gb)
> static int unpack_vlcs(Vp3DecodeContext *s, GetBitContext *gb,
> - VLC *table, int coeff_index,
> - int plane,
> - int eob_run)
> + VLC *table, int coeff_index,
> + int plane,
> + int eob_run)
> {
nit: You could coalesce some of these lines.
> int i, j = 0;
> int token;
> - int zero_run = 0;
> + int zero_run = 0;
> int16_t coeff = 0;
> int bits_to_get;
> int blocks_ended;
> - int coeff_i = 0;
> - int num_coeffs = s->num_coded_frags[plane][coeff_index];
> + int coeff_i = 0;
> + int num_coeffs = s->num_coded_frags[plane][coeff_index];
> int16_t *dct_tokens = s->dct_tokens[plane][coeff_index];
Only the last two assignments are related, only align those.
> Vp3Fragment *all_fragments = s->all_fragments;
> - VLC_TYPE (*vlc_table)[2] = table->table;
> + VLC_TYPE(*vlc_table)[2] = table->table;
stray change
> if (eob_run > num_coeffs) {
> - coeff_i = blocks_ended = num_coeffs;
> + coeff_i = blocks_ended = num_coeffs;
Break this multi-assignment line.
> @@ -946,66 +951,67 @@ static int unpack_vlcs(Vp3DecodeContext *s,
> GetBitContext *gb,
> } else {
> - av_log(s->avctx, AV_LOG_ERROR,
> - "Invalid token %d\n", token);
> + } else {
> + av_log(s->avctx, AV_LOG_ERROR,
> + "Invalid token %d\n", token);
You could coalesce these lines.
> @@ -1160,22 +1164,22 @@ static void reverse_dc_prediction(Vp3DecodeContext *s,
> - {-104,116, 0,116}, // PUL|PU|PL
> - { 24, 80, 24, 0}, // PUL|PU|PUR
> - {-104,116, 0,116} // PUL|PU|PUR|PL
> + { -104, 116, 0, 116 }, // PUL|PU|PL
> + { 24, 80, 24, 0 }, // PUL|PU|PUR
> + { -104, 116, 0, 116 } // PUL|PU|PUR|PL
Something went wrong with the // alignment.
> @@ -1202,54 +1206,50 @@ static void reverse_dc_prediction(Vp3DecodeContext *s,
>
> int transform = 0;
>
> - vul = vu = vur = vl = 0;
> + vul = vu = vur = vl = 0;
Leave as-is.
> last_dc[0] = last_dc[1] = last_dc[2] = 0;
Or maybe break all the lines.
> /* for each fragment in a row... */
> - for (x = 0; x < fragment_width; x++, i++) {
> + for (x = 0; x < fragment_width; x++, i++)
>
> @@ -1276,43 +1276,42 @@ static void reverse_dc_prediction(Vp3DecodeContext *s,
> /* save the DC */
> last_dc[current_frame_type] = DC_COEFF(i);
> }
> - }
> }
> }
I'd keep the {}.
> @@ -1358,7 +1357,7 @@ static inline int vp3_dequant(Vp3DecodeContext *s,
> Vp3Fragment *frag,
> switch (token & 3) {
> case 0: // EOB
> - if (--token < 4) // 0-3 are token types, so the EOB run must now
> be 0
> + if (--token < 4) // 0-3 are token types so the EOB run must now
> be 0
stray change
> @@ -1467,38 +1469,43 @@ static void render_slice(Vp3DecodeContext *s, int
> slice)
>
> for (plane = 0; plane < 3; plane++) {
> -
> - int fragment_width = s->fragment_width[!!plane];
> - int fragment_height = s->fragment_height[!!plane];
> - int fragment_start = s->fragment_start[plane];
> - int do_await = !plane && HAVE_THREADS &&
> (s->avctx->active_thread_type&FF_THREAD_FRAME);
> -
> + int fragment_width = s->fragment_width[!!plane];
> + int fragment_height = s->fragment_height[!!plane];
> + int fragment_start = s->fragment_start[plane];
> + int do_await = !plane && HAVE_THREADS &&
> + (s->avctx->active_thread_type &
> FF_THREAD_FRAME);
The alignment of the last line is IMO excessive.
> /* for each superblock in a row... */
> - for (sb_x = 0; sb_x < slice_width; sb_x++) {
> + for (sb_x = 0; sb_x < slice_width; sb_x++)
>
> @@ -1506,125 +1513,134 @@ static void render_slice(Vp3DecodeContext *s, int
> slice)
> + }
> }
> - }
overzealous {} removal
> + s->vp3dsp.idct_put(
> output_plane + first_pixel,
> - motion_source, stride, 8);
> - s->vp3dsp.idct_put(
> - output_plane + first_pixel,
> - stride,
> - block);
Don't break after (.
> + /* copy directly from the previous frame */
> + s->hdsp.put_pixels_tab[1][0](
> output_plane + first_pixel,
> - /* copy directly from the previous frame */
> - s->hdsp.put_pixels_tab[1][0](
> - output_plane + first_pixel,
same
(Yes, I know that I'm slightly misquoting.)
> -
> - }
> }
> - }
possibly more overzealous {} removal
> @@ -1719,135 +1738,131 @@ static av_cold int vp3_decode_init(AVCodecContext
> *avctx)
>
> - s->y_superblock_width = (s->width + 31) / 32;
> + s->y_superblock_width = (s->width + 31) / 32;
> s->y_superblock_height = (s->height + 31) / 32;
nit: You could align this some more.
> /* work out the dimensions for the C planes */
> - c_width = s->width >> s->chroma_x_shift;
> - c_height = s->height >> s->chroma_y_shift;
> - s->c_superblock_width = (c_width + 31) / 32;
> + c_width = s->width >> s->chroma_x_shift;
> + c_height = s->height >> s->chroma_y_shift;
> + s->c_superblock_width = (c_width + 31) / 32;
> s->c_superblock_height = (c_height + 31) / 32;
> - s->c_superblock_count = s->c_superblock_width * s->c_superblock_height;
> + s->c_superblock_count = s->c_superblock_width * s->c_superblock_height;
>
> - s->superblock_count = s->y_superblock_count + (s->c_superblock_count *
> 2);
> + s->superblock_count = s->y_superblock_count + (s->c_superblock_count *
> 2);
same
> - s->macroblock_width = (s->width + 15) / 16;
> + s->macroblock_width = (s->width + 15) / 16;
> s->macroblock_height = (s->height + 15) / 16;
> - s->macroblock_count = s->macroblock_width * s->macroblock_height;
> + s->macroblock_count = s->macroblock_width * s->macroblock_height;
>
> - s->fragment_width[0] = s->width / FRAGMENT_PIXELS;
> + s->fragment_width[0] = s->width / FRAGMENT_PIXELS;
> s->fragment_height[0] = s->height / FRAGMENT_PIXELS;
> - s->fragment_width[1] = s->fragment_width[0] >> s->chroma_x_shift;
> + s->fragment_width[1] = s->fragment_width[0] >> s->chroma_x_shift;
> s->fragment_height[1] = s->fragment_height[0] >> s->chroma_y_shift;
same
> - s->qr_base [inter][plane][0]=
> - s->qr_base [inter][plane][1]= 2*inter + (!!plane)*!inter;
> + s->qr_base[inter][plane][0] =
> + s->qr_base[inter][plane][1] = 2 * inter + (!!plane) *
> !inter;
Indentation is off on the second line.
> }
> - }
overzealous
> @@ -1900,13 +1914,14 @@ static int ref_frames(Vp3DecodeContext *dst,
> Vp3DecodeContext *src)
> static int vp3_update_thread_context(AVCodecContext *dst, const
> AVCodecContext *src)
> {
> Vp3DecodeContext *s = dst->priv_data, *s1 = src->priv_data;
> - int qps_changed = 0, i, err;
> + int qps_changed = 0, i, err;
Drop this excessive alignment.
> @@ -1917,13 +1932,15 @@ static int vp3_update_thread_context(AVCodecContext
> *dst, const AVCodecContext *
> if (!s->current_frame.f->data[0]) {
> int y_fragment_count, c_fragment_count;
> s->avctx = dst;
> - err = allocate_tables(dst);
> + err = allocate_tables(dst);
same
> @@ -1933,15 +1950,15 @@ static int vp3_update_thread_context(AVCodecContext
> *dst, const AVCodecContext *
> // copy qscale data if necessary
> - for (i = 0; i < 3; i++) {
> + for (i = 0; i < 3; i++)
> if (s->qps[i] != s1->qps[1]) {
> qps_changed = 1;
> memcpy(&s->qmat[i], &s1->qmat[i], sizeof(s->qmat[i]));
> }
> - }
overzealous {} removal
> @@ -2176,15 +2196,15 @@ static int theora_decode_header(AVCodecContext
> *avctx, GetBitContext *gb)
>
> - visible_width = s->width = get_bits(gb, 16) << 4;
> + visible_width = s->width = get_bits(gb, 16) << 4;
> visible_height = s->height = get_bits(gb, 16) << 4;
stray change
> @@ -2230,8 +2249,8 @@ static int theora_decode_header(AVCodecContext *avctx,
> GetBitContext *gb)
>
> - if ( visible_width <= s->width && visible_width > s->width-16
> - && visible_height <= s->height && visible_height > s->height-16
> + if (visible_width <= s->width && visible_width > s->width - 16
> + && visible_height <= s->height && visible_height > s->height - 16
> && !offset_x && (offset_y == s->height - visible_height))
Move the && to the end of the line.
> @@ -2286,48 +2305,50 @@ static int theora_decode_tables(AVCodecContext
> *avctx, GetBitContext *gb)
>
> - for (inter = 0; inter <= 1; inter++) {
> + for (inter = 0; inter <= 1; inter++)
> for (plane = 0; plane <= 2; plane++) {
> } else {
>
> @@ -2335,21 +2356,20 @@ static int theora_decode_tables(AVCodecContext
> *avctx, GetBitContext *gb)
> av_log(avctx, AV_LOG_ERROR, "invalid qi %d > 63\n", qi);
> return -1;
> }
> - s->qr_count[inter][plane]= qri;
> + s->qr_count[inter][plane] = qri;
> }
> }
> - }
overzealous {} removal
> @@ -2370,39 +2390,36 @@ static av_cold int theora_decode_init(AVCodecContext
> *avctx)
>
> - if (!(ptype & 0x80))
> - {
> - av_log(avctx, AV_LOG_ERROR, "Invalid extradata!\n");
> + if (!(ptype & 0x80)) {
> + av_log(avctx, AV_LOG_ERROR, "Invalid extradata!\n");
> // return -1;
The return remains wrongly indented.
> @@ -2412,47 +2429,50 @@ static av_cold int theora_decode_init(AVCodecContext
> *avctx)
>
> AVCodec ff_theora_decoder = {
> - .name = "theora",
> - .long_name = NULL_IF_CONFIG_SMALL("Theora"),
> - .type = AVMEDIA_TYPE_VIDEO,
> - .id = AV_CODEC_ID_THEORA,
> - .priv_data_size = sizeof(Vp3DecodeContext),
> - .init = theora_decode_init,
> - .close = vp3_decode_end,
> - .decode = vp3_decode_frame,
> - .capabilities = CODEC_CAP_DR1 | CODEC_CAP_DRAW_HORIZ_BAND |
> - CODEC_CAP_FRAME_THREADS,
> - .flush = vp3_decode_flush,
> - .init_thread_copy = ONLY_IF_THREADS_ENABLED(vp3_init_thread_copy),
> - .update_thread_context =
> ONLY_IF_THREADS_ENABLED(vp3_update_thread_context)
> + .name = "theora",
> + .long_name = NULL_IF_CONFIG_SMALL("Theora"),
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = AV_CODEC_ID_THEORA,
> + .priv_data_size = sizeof(Vp3DecodeContext),
> + .init = theora_decode_init,
> + .close = vp3_decode_end,
> + .decode = vp3_decode_frame,
> + .capabilities = CODEC_CAP_DR1 | CODEC_CAP_DRAW_HORIZ_BAND |
> + CODEC_CAP_FRAME_THREADS,
> + .flush = vp3_decode_flush,
> + .init_thread_copy = ONLY_IF_THREADS_ENABLED(vp3_init_thread_copy),
> + .update_thread_context =
> ONLY_IF_THREADS_ENABLED(vp3_update_thread_context)
> };
> #endif
>
> AVCodec ff_vp3_decoder = {
> - .name = "vp3",
> - .long_name = NULL_IF_CONFIG_SMALL("On2 VP3"),
> - .type = AVMEDIA_TYPE_VIDEO,
> - .id = AV_CODEC_ID_VP3,
> - .priv_data_size = sizeof(Vp3DecodeContext),
> - .init = vp3_decode_init,
> - .close = vp3_decode_end,
> - .decode = vp3_decode_frame,
> - .capabilities = CODEC_CAP_DR1 | CODEC_CAP_DRAW_HORIZ_BAND |
> - CODEC_CAP_FRAME_THREADS,
> - .flush = vp3_decode_flush,
> - .init_thread_copy = ONLY_IF_THREADS_ENABLED(vp3_init_thread_copy),
> - .update_thread_context =
> ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
> + .name = "vp3",
> + .long_name = NULL_IF_CONFIG_SMALL("On2 VP3"),
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = AV_CODEC_ID_VP3,
> + .priv_data_size = sizeof(Vp3DecodeContext),
> + .init = vp3_decode_init,
> + .close = vp3_decode_end,
> + .decode = vp3_decode_frame,
> + .capabilities = CODEC_CAP_DR1 | CODEC_CAP_DRAW_HORIZ_BAND |
> + CODEC_CAP_FRAME_THREADS,
> + .flush = vp3_decode_flush,
> + .init_thread_copy = ONLY_IF_THREADS_ENABLED(vp3_init_thread_copy),
> + .update_thread_context =
> ONLY_IF_THREADS_ENABLED(vp3_update_thread_context),
> };
seems unnecessary
> --- a/libavcodec/vp3dsp.c
> +++ b/libavcodec/vp3dsp.c
> @@ -233,15 +233,15 @@ static void vp3_v_loop_filter_c(uint8_t *first_pixel,
> int stride,
> filter_value =
> - (first_pixel[2 * nstride] - first_pixel[ stride])
> - +3*(first_pixel[0 ] - first_pixel[nstride]);
> + (first_pixel[2 * nstride] - first_pixel[stride])
> + + 3 * (first_pixel[0] - first_pixel[nstride]);
Move the operator to the end of the line.
> @@ -251,11 +251,11 @@ static void vp3_h_loop_filter_c(uint8_t *first_pixel,
> int stride,
> filter_value =
> - (first_pixel[-2] - first_pixel[ 1])
> - +3*(first_pixel[ 0] - first_pixel[-1]);
> + (first_pixel[-2] - first_pixel[1]) + 3 *
> + (first_pixel[ 0] - first_pixel[-1]);
Like you do here, but I'd format it like this:
filter_value = (first_pixel[-2] - first_pixel[1]) +
(first_pixel[0] - first_pixel[-1]) * 3;
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
