On Tue, Oct 01, 2013 at 11:51:40AM +0200, Maxim Polijakowski wrote:
> [...]
> >>--- a/libavcodec/atrac3.c
> >>+++ b/libavcodec/atrac3.c
> >>@@ -417,90 +412,32 @@ static int decode_tonal_components(GetBitContext *gb,
> >> static int decode_gain_control(GetBitContext *gb, GainBlock *block,
> >> int num_bands)
> >> {
> >>- int i, cf, num_data;
> >>+ int i, b;
> >> int *level, *loc;
> >>- for (i = 0; i <= num_bands; i++) {
> >>- num_data = get_bits(gb, 3);
> >>- gain[i].num_gain_data = num_data;
> >>- level = gain[i].lev_code;
> >>- loc = gain[i].loc_code;
> >>+ for (b = 0; b <= num_bands; b++) {
> >>+ gain[b].num_points = get_bits(gb, 3);
> >>+ level = gain[b].levcode;
> >>+ loc = gain[b].loccode;
> >>- for (cf = 0; cf < gain[i].num_gain_data; cf++) {
> >>- level[cf] = get_bits(gb, 4);
> >>- loc [cf] = get_bits(gb, 5);
> >>- if (cf && loc[cf] <= loc[cf - 1])
> >>+ for (i = 0; i < gain[b].num_points; i++) {
> >>+ level[i] = get_bits(gb, 4);
> >>+ loc [i] = get_bits(gb, 5);
> >>+ if (i && loc[i] <= loc[i-1])
> >> return AVERROR_INVALIDDATA;
> >> }
> >> }
> >>- /* Clear the unused blocks. */
> >>- for (; i < 4 ; i++)
> >>- gain[i].num_gain_data = 0;
> >>+ /* Clear unused blocks. */
> >>+ for (; b < 4 ; b++)
> >>+ gain[b].num_points = 0;
> >
> >Is there a reason to rename the counter variables? It seems rather
> >arbitrary and complicates the diff.
>
> Yes, the reason for rename this variable is to make it look more
> "standard" and readable. What does that "cf" stand for? "Core
> Foundation" or "close file"?
> "i" is unambiguous and clear, isn't it?
Yes, we usually use 'i' and 'j' as counter variables.
> The above mentioned changes look rather trivial IMHO so I wonder if
> someone else could fix them all and push the patches. Taking in
> consideration the amount of work I'm currently doing in order to
> provide Libav with support for several other obscured and
> proprietary formats, fixing such things again and again wastes alot
> of my time and keep me away from doing perhaps more important stuff.
And I'm helping you by doing all the postprocessing on ATRAC3+ :)
I could do all the work myself directly, but I choose to review your
patches so that in the future the postprocessing work is reduced while
you adapt a bit more to the modern libav style and workflow. If we
get to that point, things will go much more smoothly.
I'll fix up and split/rebase this patchset later today.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
