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

[Michael Niedermayer]

On Wed, Jul 30, 2008 at 01:54:16PM +0100, Robert Swain wrote:
> 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.

He can just put these 2 fields in the 2 structs back with his encoder patch.
But as long as they are not used i will not approve unused variables, its
far too hard to keep track of what might be used one day and then check back
if that actually happened or if we ended up with some permamently unused
fields in structs.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080730/fbd7242e/attachment.pgp>