[FFmpeg-devel] [PATCH] AAC Decoder round 4

[Robert Swain]

2008/7/30 Robert Swain <robert.swain at gmail.com>:
> 2008/7/27 Michael Niedermayer <michaelni at gmx.at>:
>> On Fri, Jul 18, 2008 at 03:13:38PM +0100, Robert Swain wrote:

[...]

>>> +    global_gain = get_bits(gb, 8);
>>> +
>>> +    if (!common_window && !scale_flag) {
>>> +        if (decode_ics_info(ac, gb, 0, ics) < 0)
>>> +            return -1;
>>> +    }
>>> +
>>> +    if (decode_section(ac, gb, ics, sce->band_type, sce->band_type_run_end) < 0)
>>> +        return -1;
>>> +    if (decode_scalefactors(ac, gb, sce->mixing_gain, global_gain, ics, sce->band_type, sce->band_type_run_end, sce->sf) < 0)
>>> +        return -1;
>>> +
>>> +    if (!scale_flag) {
>>> +        if ((pulse.present = get_bits1(gb))) {
>>> +            if (ics->window_sequence == EIGHT_SHORT_SEQUENCE) {
>>> +                av_log(ac->avccontext, AV_LOG_ERROR, "Pulse tool not allowed in eight short sequence.\n");
>>> +                return -1;
>>> +            }
>>> +            decode_pulses(ac, gb, &pulse);
>>> +        }
>>> +        if ((tns->present = get_bits1(gb)))
>>> +            decode_tns(ac, gb, ics, tns);
>>> +        if (get_bits1(gb)) {
>>> +            av_log(ac->avccontext, AV_LOG_ERROR, "SSR not supported.\n");
>>> +            return -1;
>>> +        }
>>> +    }
>>> +
>>> +    if (decode_spectrum(ac, gb, ics, sce->band_type, icoeffs) < 0)
>>> +        return -1;
>>> +    if (pulse.present)
>>> +        add_pulses(ac, ics, &pulse, icoeffs);
>>
>> it seems pulse.present is not used by anything outside this function, if true
>> it could be a normal local variable (and the field removed from the struct)
>
> Done.

[...]

>>> +/**
>>> + * Decode a channel_pair_element; reference: table 4.4.
>>> + *
>>> + * @param   tag Identifies the instance of a syntax element.
>>> + * @return  Returns error status. 0 - OK, !0 - error
>>> + */
>>> +static int decode_cpe(AACContext * ac, GetBitContext * gb, int tag) {
>>> +    int i, ret;
>>> +    ChannelElement * cpe;
>>> +
>>> +    cpe = ac->che[ID_CPE][tag];
>>
>>> +    cpe->common_window = get_bits1(gb);
>>
>> This variable does not seem used outside this function thus could be a local
>> variable (and the one in the struct of course removed)
>
> Done.

Kostya has said that he would like these changes reverted as he uses
these elements of these data structures in his code.

Also he would like tables to be put in aactab.c, table declarations in
aactab.h and structures and definitions in aac.h so I've added this to
my to do before next submission list.

Regards,
Rob