[FFmpeg-devel] [PATCH] AAC decoder round 8
Robert Swain
robert.swain
Fri Aug 15 17:12:48 CEST 2008
2008/8/15 Michael Niedermayer <michaelni at gmx.at>:
> On Fri, Aug 15, 2008 at 09:04:24AM +0100, Robert Swain wrote:
>> 2008/8/15 Michael Niedermayer <michaelni at gmx.at>:
>> > On Fri, Aug 15, 2008 at 01:32:08AM +0100, Robert Swain wrote:
>> >> $subj
>> >>
>> >> There's not much left to commit now! :D
>> >
>> > ok
>>
>> All committed. Just to make it easier for me and/or you to keep track
>> of, here's another patch attached with the remaining hunks.
>>
>> Regards,
>> Rob
>
>> Index: libavcodec/aac.c
>> ===================================================================
>> --- libavcodec/aac.c (revision 14774)
>> +++ libavcodec/aac.c (working copy)
> [...]
>> @@ -605,6 +616,44 @@
>> }
>>
>> /**
>> + * Decode Temporal Noise Shaping data; reference: table 4.48.
>> + *
>> + * @return Returns error status. 0 - OK, !0 - error
>> + */
>> +static int decode_tns(AACContext * ac, TemporalNoiseShaping * tns,
>> + GetBitContext * gb, const IndividualChannelStream * ics) {
>
>> + int w, filt, i, coef_len, coef_res = 0, coef_compress;
>
> useless init ?
Subtle, but yes.
>> + const int is8 = ics->window_sequence[0] == EIGHT_SHORT_SEQUENCE;
>> + const int tns_max_order = is8 ? 7 : ac->m4ac.object_type == AOT_AAC_MAIN ? 20 : 12;
>> + for (w = 0; w < ics->num_windows; w++) {
>> + tns->n_filt[w] = get_bits(gb, 2 - is8);
>> +
>> + if (tns->n_filt[w])
>> + coef_res = get_bits1(gb) + 3;
>> +
>> + for (filt = 0; filt < tns->n_filt[w]; filt++) {
>> + tns->length[w][filt] = get_bits(gb, 6 - 2*is8);
>> +
>
>> + if ((tns->order[w][filt] = get_bits(gb, 5 - 2*is8)) <= tns_max_order) {
>> + tns->direction[w][filt] = get_bits1(gb);
>> + coef_compress = get_bits1(gb);
>> + coef_len = coef_res - coef_compress;
>> + tns->tmp2_map[w][filt] = tns_tmp2_map[2*coef_compress + coef_res - 3];
>
> the 3 can be moved to "coef_len = coef_res - coef_compress + 3"
But, unless I'm missing something, that will change the behaviour of
the code as more bits will be read in the loop you quoted just below.
>> +
>> + for (i = 0; i < tns->order[w][filt]; i++)
>> + tns->coef[w][filt][i] = get_bits(gb, coef_len);
>
> tns->coef is only used to index into tmp2_map thus
> tns->coef could already contain the values from tmp2_map
> this also would make the tmp2_map field unneeded in the struct
OK, I'll look into this.
>> + } else {
>> + av_log(ac->avccontext, AV_LOG_ERROR, "TNS filter order %d is greater than maximum %d.",
>> + tns->order[w][filt], tns_max_order);
>> + tns->order[w][filt] = 0;
>> + return -1;
>> + }
>
> if(... > tns_max_order){
> ...
> return -1
> }
> ...
>
> seems cleaner to me
Done.
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> +/**
>> * Decode Mid/Side data; reference: table 4.54.
>> *
>> * @param ms_present Indicates mid/side stereo presence. [0] mask is all 0s;
>
>> @@ -1067,6 +1116,71 @@
>> }
>>
>> /**
>> + * Decode Temporal Noise Shaping filter coefficients and apply all-pole filters; reference: 4.6.9.3.
>> + *
>> + * @param decode 1 if tool is used normally, 0 if tool is used in LTP.
>> + * @param coef spectral coefficients
>> + */
>> +static void apply_tns(float coef[1024], TemporalNoiseShaping * tns, IndividualChannelStream * ics, int decode) {
>> + const int mmm = FFMIN(ics->tns_max_bands, ics->max_sfb);
>> + int w, filt, m, i, ib;
>> + int bottom, top, order, start, end, size, inc;
>> + float tmp;
>> + float lpc[TNS_MAX_ORDER + 1], b[2 * TNS_MAX_ORDER];
>> +
>> + for (w = 0; w < ics->num_windows; w++) {
>> + bottom = ics->num_swb;
>> + for (filt = 0; filt < tns->n_filt[w]; filt++) {
>> + top = bottom;
>> + bottom = FFMAX( 0, top - tns->length[w][filt]);
>
>> + order = FFMIN(tns->order[w][filt], TNS_MAX_ORDER);
>
> useless?
Indeed. Removed. Should the 'order' variable be removed as well or is
accessing it faster than accessing order[][]?
>> + if (order == 0)
>> + continue;
>> +
>
>> + // tns_decode_coef
>> + lpc[0] = 1;
>> + for (m = 1; m <= order; m++) {
>> + lpc[m] = tns->tmp2_map[w][filt][tns->coef[w][filt][m - 1]];
>> + for (i = 1; i < m; i++)
>> + b[i] = lpc[i] + lpc[m] * lpc[m-i];
>> + for (i = 1; i < m; i++)
>> + lpc[i] = b[i];
>> + }
>
> looks a little like eval_coefs from ra144.c
> but later is fixedpoint, so this is more a random comment than anything
>
>
>> +
>> + start = ics->swb_offset[FFMIN(bottom, mmm)];
>> + end = ics->swb_offset[FFMIN( top, mmm)];
>> + if ((size = end - start) <= 0)
>> + continue;
>> + if (tns->direction[w][filt]) {
>> + inc = -1; start = end - 1;
>> + } else {
>> + inc = 1;
>> + }
>> + start += w * 128;
>> +
>> + // ar filter
>> + memset(b, 0, sizeof(b));
>> + ib = 0;
>
>> + for (m = 0; m < size; m++) {
>> + tmp = coef[start];
>> + if (decode) {
>> + for (i = 0; i < order; i++)
>> + tmp -= b[ib + i] * lpc[i + 1];
>> + } else { // encode
>> + for (i = 0; i < order; i++)
>> + tmp += b[i] * lpc[i + 1];
>> + }
>> + if (--ib < 0)
>> + ib = order - 1;
>> + b[ib] = b[ib + order] = tmp;
>> + coef[start] = tmp;
>> + start += inc;
>> + }
>
> decode is always 1
This is left from LTP. I'll remove it when submitting next.
> b is not truly needed, coef[] can be used i its place
> also this is likely relevant to overal codec speed so it
> should be written more with speed than compactness in mind
Does that mean you want me to rewrite it?
> [...]
>
>> @@ -232,6 +245,14 @@
>> /** @} */
>>
>> /**
>> + * @defgroup temporary aligned temporary buffers (We do not want to have these on the stack.)
>> + * @{
>> + */
>> + DECLARE_ALIGNED_16(float, buf_mdct[2048]);
>> + DECLARE_ALIGNED_16(float, revers[1024]);
>> + /** @} */
>
> ok, but if you can reduce the amount of arrays or optimize the imdct
> stuff that is welcome ...
That's high on my to do list.
> [...]
>> Index: libavcodec/aacdectab.h
>> ===================================================================
>> --- libavcodec/aacdectab.h (revision 14767)
>> +++ libavcodec/aacdectab.h (working copy)
>> +
>> +/* @name tns_tmp2_map
>> + * Tables of the tmp2[] arrays of LPC coefficients used for TNS.
>> + * The suffix _M_N[] indicate the values of coef_compress and coef_res
>> + * respectively.
>> + * @{
>> + */
>> +static const float tns_tmp2_map_1_3[TNS_MAX_ORDER] = {
>> + 0.00000000, 0.43388373, -0.64278758, -0.34202015,
>> + 0.97492790, 0.78183150, -0.64278758, -0.34202015,
>> + -0.43388373, -0.78183150, -0.64278758, -0.34202015,
>> + -0.78183150, -0.43388373, -0.64278758, -0.34202015,
>> + 0.78183150, 0.97492790, -0.64278758, -0.34202015
>> +};
>> +
>> +static const float tns_tmp2_map_0_3[TNS_MAX_ORDER] = {
>> + 0.00000000, 0.43388373, 0.78183150, 0.97492790,
>> + -0.98480773, -0.86602539, -0.64278758, -0.34202015,
>> + -0.43388373, -0.78183150, -0.97492790, -0.97492790,
>> + -0.98480773, -0.86602539, -0.64278758, -0.34202015,
>> + 0.78183150, 0.97492790, 0.97492790, 0.78183150
>> +};
>> +
>> +static const float tns_tmp2_map_1_4[TNS_MAX_ORDER] = {
>> + 0.00000000, 0.20791170, 0.40673664, 0.58778524,
>> + -0.67369562, -0.52643216, -0.36124167, -0.18374951,
>> + 0.99452192, 0.95105648, 0.86602539, 0.74314481,
>> + -0.67369562, -0.52643216, -0.36124167, -0.18374951,
>> + -0.20791176, -0.40673670, -0.58778530, -0.74314487
>> +};
>> +
>> +static const float tns_tmp2_map_0_4[TNS_MAX_ORDER] = {
>> + 0.00000000, 0.20791170, 0.40673664, 0.58778524,
>> + 0.74314481, 0.86602539, 0.95105654, 0.99452192,
>> + -0.99573416, -0.96182561, -0.89516330, -0.79801720,
>> + -0.67369562, -0.52643216, -0.36124167, -0.18374951,
>> + -0.20791176, -0.40673670, -0.58778530, -0.74314487
>> +};
>
> iam not sure if the code is correct but i think several of these
> elements can never be accessed
I'll check the code.
Rob
More information about the ffmpeg-devel
mailing list