[FFmpeg-devel] [PATCH] HE-AAC v1 try 5
Michael Niedermayer
michaelni
Thu Mar 4 23:20:34 CET 2010
On Wed, Mar 03, 2010 at 10:07:12PM -0500, Alex Converse wrote:
> Here is what has changed since last time:
> * Optimizations to sbr_hf_assemble():
> ** Y is calculated incrementally. This takes advantage of values known
> to be zero, omitting them from the calculation
> ** Some structure values are read into const locals.
> * Optimizations to the synthesis filterbank: 32 calls to memove() per
> channel per frame are now replaced with occasional calls to memcpy()
> * .init() no longer overwrites AVCodec context fields to zero
> * avcodec.h minor bump
>
> Also attached is a patch to libavformat that adds a workaround to set
> container parameters that are unreliable in the case of backwards
> compatible signalling to zero. I question my ability to audit each
> demuxer manually in a timely manner.
> avformat.h | 2 +-
> utils.c | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
> 4948a33990bbe6fca0e4bda563022b3e2e03d49f sbr_avformat.patch
> From bfc85e165f3716abe08356a584dc8ac6e5593369 Mon Sep 17 00:00:00 2001
> From: Alex Converse <alex.converse at gmail.com>
> Date: Wed, 3 Mar 2010 05:17:59 -0500
> Subject: [PATCH] Add a workaround for backwards compatible HE-AAC signaling.
> MIME-Version: 1.0
> Content-Type: multipart/mixed; boundary="------------1"
>
> This is a multi-part message in MIME format.
> --------------1
> Content-Type: text/plain; charset=UTF-8; format=fixed
> Content-Transfer-Encoding: 8bit
>
>
> The sample rate, frame size, and channel count from the container are
> not reliable when backwards compatible signaling is used.
> ---
> libavformat/avformat.h | 2 +-
> libavformat/utils.c | 5 +++++
> 2 files changed, 6 insertions(+), 1 deletions(-)
>
>
> --------------1
> Content-Type: text/x-patch; name="0001-Add-a-workaround-for-backwards-compatible-HE-AAC-sig.patch"
> Content-Transfer-Encoding: 8bit
> Content-Disposition: attachment; filename="0001-Add-a-workaround-for-backwards-compatible-HE-AAC-sig.patch"
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index e7426aa..a46d564 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -22,7 +22,7 @@
> #define AVFORMAT_AVFORMAT_H
>
> #define LIBAVFORMAT_VERSION_MAJOR 52
> -#define LIBAVFORMAT_VERSION_MINOR 54
> +#define LIBAVFORMAT_VERSION_MINOR 55
> #define LIBAVFORMAT_VERSION_MICRO 0
>
> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 2c0f4bb..b71e7cb 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2080,6 +2080,11 @@ int av_find_stream_info(AVFormatContext *ic)
>
> for(i=0;i<ic->nb_streams;i++) {
> st = ic->streams[i];
> + if (st->codec->codec_id == CODEC_ID_AAC) {
> + st->codec->sample_rate = 0;
> + st->codec->frame_size = 0;
> + st->codec->channels = 0;
> + }
> if(st->codec->codec_type == CODEC_TYPE_VIDEO){
> /* if(!st->time_base.num)
> st->time_base= */
>
> --------------1--
ok if tested
[...]
> @@ -1651,7 +1640,21 @@ static int decode_extension_payload(AACContext *ac, GetBitContext *gb, int cnt)
> case EXT_SBR_DATA_CRC:
> crc_flag++;
> case EXT_SBR_DATA:
> - res = decode_sbr_extension(ac, gb, crc_flag, cnt);
> + if (!che) {
> + av_log(ac->avccontext, AV_LOG_ERROR, "SBR was found before the first channel element.\n");
> + return res;
> + } else if (!ac->m4ac.sbr) {
> + av_log(ac->avccontext, AV_LOG_ERROR, "SBR signaled to be not-present but was found in the bitstream.\n");
> + skip_bits_long(gb, 8 * cnt - 4);
> + return res;
> + } else if (ac->m4ac.sbr == -1 && ac->output_configured == OC_LOCKED) {
> + av_log(ac->avccontext, AV_LOG_ERROR, "Implicit SBR was found with a first occurrence after the first frame.\n");
> + skip_bits_long(gb, 8 * cnt - 4);
> + return res;
> + } else {
> + ac->m4ac.sbr = 1;
> + }
> + res = ff_decode_sbr_extension(ac, &che->sbr, gb, crc_flag, cnt, elem_type);
> break;
> case EXT_DYNAMIC_RANGE:
> res = decode_dynamic_range(&ac->che_drc, gb, cnt);
> @@ -1833,7 +1836,7 @@ static void apply_independent_coupling(AACContext *ac,
> const float *src = cce->ch[0].ret;
> float *dest = target->ret;
>
> - for (i = 0; i < 1024; i++)
> + for (i = 0; i < 2048; i++)
> dest[i] += gain * (src[i] - bias);
> }
>
does this now always has 2x as much data or is half of this unneeded/wrong?
[...]
> +
> +static VLC vlc_sbr[10];
> +static const int8_t vlc_sbr_lav[10] =
> + { 60, 60, 24, 24, 31, 31, 12, 12, 31, 12 };
> +static DECLARE_ALIGNED_16(float, analysis_cos_pre)[64];
> +static DECLARE_ALIGNED_16(float, analysis_sin_pre)[64];
> +static DECLARE_ALIGNED_16(float, analysis_cossin_post)[32][2];
> +static DECLARE_ALIGNED_16(float, zero64)[64];
const
[...]
> +static inline void remove_table_element_int16(int16_t *table, uint8_t *last_el,
> + int el)
> +{
> + memmove(table + el, table + el + 1, (*last_el - el)*sizeof(int16_t));
> + (*last_el)--;
> +}
please use variable names that make sense, that is size or length not last_el
and el should be index
> +
> +static inline int in_table_int16(int16_t *table, int last_el, int16_t needle)
> +{
> + int i;
> + for (i = 0; i <= last_el; i++)
> + if (table[i] == needle)
> + return 1;
> + return 0;
> +}
> +
> +/// Limiter Frequency Band Table (14496-3 sp04 p198)
> +static void sbr_make_f_tablelim(SpectralBandReplication *sbr)
> +{
> + int k;
> + if (sbr->bs_limiter_bands > 0) {
> + static const float lim_bands_per_octave[3] = {1.2, 2, 3};
> + int16_t patch_borders[5];
> +
> + patch_borders[0] = sbr->k[4];
> + for (k=1; k <= sbr->num_patches; k++)
> + patch_borders[k] = patch_borders[k-1] + sbr->patch_num_subbands[k-1];
> +
> + memcpy( sbr->f_tablelim, sbr->f_tablelow,
> + (sbr->n[0] + 1) * sizeof(sbr->f_tablelow[0]));
please dont place whitespace completely randomly
> + if (sbr->num_patches > 1)
> + memcpy(&sbr->f_tablelim[sbr->n[0] + 1], &patch_borders[1],
> + (sbr->num_patches - 1) * sizeof(patch_borders[0]));
i think x+n is cleaner than &x[n]
> +
> + qsort(sbr->f_tablelim, sbr->num_patches + sbr->n[0],
> + sizeof(sbr->f_tablelim[0]),
> + qsort_comparison_function_int16);
> +
> + k = 1;
> + sbr->n_lim = sbr->n[0] + sbr->num_patches - 1;
> + while (k <= sbr->n_lim) {
> + // if ( nOctaves * limBands >= 0.49) ...
> + if (log2(sbr->f_tablelim[k] / (float)sbr->f_tablelim[k-1]) *
> + lim_bands_per_octave[sbr->bs_limiter_bands - 1] >= 0.49) {
> + k++;
> + continue;
> + }
X*L >= 0.49
X >= 0.49/L
X >= L' (which is 0.49 merged into the table)
log2(a/b) >= L'
a/b >= exp2(L')
a >= b*L'' (which is exp2 merged into the table)
> + if (sbr->f_tablelim[k] == sbr->f_tablelim[k-1] ||
> + !in_table_int16(patch_borders, sbr->num_patches, sbr->f_tablelim[k]))
> + remove_table_element_int16(sbr->f_tablelim, &sbr->n_lim, k);
> + else if (!in_table_int16(patch_borders, sbr->num_patches, sbr->f_tablelim[k-1]))
> + remove_table_element_int16(sbr->f_tablelim, &sbr->n_lim, k-1);
> + else
> + k++;
> + }
this code seems somewhat obfuscated
we have 2 tables A and B, they are combined into table C and sorted
then starting from 0 to size all elements from C are removed that are
too close to their previous element unless they where in B
this can trivially be done in O(|C|) time your code though needs
O(|C|*|C|) besides that the code seems messy which probably is the
bigger issue.
note, iam not asking you to make it faster if speed is irrelevant but
it could be made less messy i think
> + } else {
> + sbr->f_tablelim[0] = sbr->f_tablelow[0];
> + sbr->f_tablelim[1] = sbr->f_tablelow[sbr->n[0]];
> + sbr->n_lim = 1;
> + }
> +}
> +
> +static unsigned int sbr_header(SpectralBandReplication *sbr, GetBitContext *gb)
> +{
> + unsigned int cnt = get_bits_count(gb);
> + uint8_t bs_header_extra_1;
> + uint8_t bs_header_extra_2;
> + int old_bs_limiter_bands = sbr->bs_limiter_bands;
> +
> + sbr->start = 1;
> +
> + // Save last spectrum parameters variables to compare to new ones
> + memcpy(&sbr->spectrum_params[0], &sbr->spectrum_params[1], sizeof(SpectrumParameters));
spectrum_params[0] can be a local variable
[...]
> +static int array_min_int16(int16_t *array, int nel)
const int16_t *array
> +{
> + int i, min = array[0];
> + for (i = 1; i < nel; i++)
> + if (array[i] < min)
> + min = array[i];
min= FFMIN(min, array[i]);
also i seriously doubt that the whole ac^k needs to be scaned for the
minimum element, at the least you can stop once elements are more than 1 or
2 larger than the minimum, and that assumes rounding has a significant
effect
[...]
> +
> +/// Master Frequency Band Table (14496-3 sp04 p194)
> +static int sbr_make_f_master(AACContext *ac, SpectralBandReplication *sbr,
> + SpectrumParameters *spectrum)
> +{
> + unsigned int temp;
> + unsigned int start_min, stop_min;
> + int k;
> + const uint8_t *sbr_offset_ptr;
> + int16_t stop_dk[13];
> +
> + if (sbr->sample_rate < 32000) {
> + temp = 3000;
> + } else if (sbr->sample_rate < 64000) {
> + temp = 4000;
> + } else
> + temp = 5000;
> +
> + start_min = ((temp << 7) + (sbr->sample_rate >> 1)) / sbr->sample_rate;
> + stop_min = ((temp << 8) + (sbr->sample_rate >> 1)) / sbr->sample_rate;
> +
> + switch (sbr->sample_rate) {
> + case 16000:
> + sbr_offset_ptr = sbr_offset[0];
> + break;
> + case 22050:
> + sbr_offset_ptr = sbr_offset[1];
> + break;
> + case 24000:
> + sbr_offset_ptr = sbr_offset[2];
> + break;
> + case 32000:
> + sbr_offset_ptr = sbr_offset[3];
> + break;
> + case 44100: case 48000: case 64000:
> + sbr_offset_ptr = sbr_offset[4];
> + break;
> + case 88200: case 96000: case 128000: case 176400: case 192000:
> + sbr_offset_ptr = sbr_offset[5];
> + break;
> + default:
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Unsupported sample rate for SBR: %d\n", sbr->sample_rate);
> + return -1;
> + }
> +
> + sbr->k[0] = start_min + sbr_offset_ptr[spectrum->bs_start_freq];
> +
> + if (spectrum->bs_stop_freq < 14) {
> + sbr->k[2] = stop_min;
> + make_bands(stop_dk, stop_min, 64, 13);
> + qsort(stop_dk, 13, sizeof(stop_dk[0]), qsort_comparison_function_int16);
just confirming, this array is not guranteed to be sorted already?
> + for (k = 0; k < spectrum->bs_stop_freq; k++)
> + sbr->k[2] += stop_dk[k];
> + } else if (spectrum->bs_stop_freq == 14) {
> + sbr->k[2] = 2*sbr->k[0];
> + } else if (spectrum->bs_stop_freq == 15) {
> + sbr->k[2] = 3*sbr->k[0];
> + } else {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Invalid bs_stop_freq: %d\n", spectrum->bs_stop_freq);
> + return -1;
> + }
> + sbr->k[2] = FFMIN(64, sbr->k[2]);
> +
> + if (!spectrum->bs_freq_scale) {
> + unsigned int dk;
> + int k2diff;
> +
> + if (!spectrum->bs_alter_scale) {
> + dk = 1;
> + sbr->n_master = (sbr->k[2] - sbr->k[0]) & ~0x01;
> + } else {
> + dk = 2;
> + sbr->n_master = ((sbr->k[2] - sbr->k[0] + 2) >> 2) << 1;
> + }
maybe silly idea but
dk = 1 + spectrum->bs_alter_scale;
sbr->n_master = (sbr->k[2] - sbr->k[0] + (2&dk))>>dk;
sbr->n_master<<=1;
does that with less code
> +
> + for (k = 1; k <= sbr->n_master; k++)
> + sbr->f_master[k] = dk;
> +
> + k2diff = sbr->k[2] - sbr->k[0] - sbr->n_master * dk;
> + if (k2diff) {
> + int incr;
> + if (k2diff < 0) {
> + incr = 1;
> + k = 1;
> + } else {
> + incr = -1;
> + k = sbr->n_master;
> + }
> +
> + while (k2diff) {
> + sbr->f_master[k] -= incr;
> + k += incr;
> + k2diff += incr;
> + }
if(k2diff < 0){
sbr->f_master[1]--;
sbr->f_master[2]-= (k2diff < 1);
}else{
sbr->f_master[sbr->n_master]++;
}
> + }
> +
> + sbr->f_master[0] = sbr->k[0];
> + for (k = 1; k <= sbr->n_master; k++)
> + sbr->f_master[k] += sbr->f_master[k - 1];
> +
> + // Requirements (14496-3 sp04 p205)
> + if (sbr->n_master <= 0) {
> + av_log(ac->avccontext, AV_LOG_ERROR, "Invalid n_master: %d\n", sbr->n_master);
> + return -1;
> + }
> + } else {
> + int half_bands = 7 - spectrum->bs_freq_scale; // bs_freq_scale = {1,2,3}
> + int two_regions, num_bands_0;
> + int vdk0_max, vdk1_min;
> + int16_t vk0[49];
> +
> + if (sbr->k[2] / (float)sbr->k[0] > 2.2449f) {
if (sbr->k[2] > 2.2449f*sbr->k[0]) {
or maybe
if (49*sbr->k[2] > 110*sbr->k[0]) {
(but i did not check that this is correct for the input range)
> + two_regions = 1;
> + sbr->k[1] = 2 * sbr->k[0];
> + } else {
> + two_regions = 0;
> + sbr->k[1] = sbr->k[2];
> + }
> +
> + num_bands_0 = lrintf(half_bands * log2f(sbr->k[1] / (float)sbr->k[0])) * 2;
> +
> + if (num_bands_0 <= 0) { // Requirements (14496-3 sp04 p205)
> + av_log(ac->avccontext, AV_LOG_ERROR, "Invalid num_bands_0: %d\n", num_bands_0);
> + return -1;
> + }
> +
> + vk0[0] = 0;
> +
> + make_bands(vk0+1, sbr->k[0], sbr->k[1], num_bands_0);
> +
> + qsort(vk0 + 1, num_bands_0, sizeof(vk0[1]), qsort_comparison_function_int16);
sort needed?
[...]
> + // temp == max number of QMF subbands
> + if (sbr->sample_rate <= 32000) {
> + temp = 48;
> + } else if (sbr->sample_rate == 44100) {
> + temp = 35;
> + } else if (sbr->sample_rate >= 48000)
> + temp = 32;
> +
> + if (sbr->k[2] - sbr->k[0] > temp) {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Invalid bitstream, too many QMF subbands: %d\n", sbr->k[2] - sbr->k[0]);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/// High Frequency Generation - Patch Construction (14496-3 sp04 p216 fig. 4.46)
> +static int sbr_hf_calc_npatches(AACContext *ac, SpectralBandReplication *sbr)
> +{
> + int i, k, sb = 0;
> + int msb = sbr->k[0];
> + int usb = sbr->k[4];
what is usb
what is msb
> + int goal_sb = ((1000 << 11) + (sbr->sample_rate >> 1)) / sbr->sample_rate;
> +
> + sbr->num_patches = 0;
> +
> + if (goal_sb < sbr->k[4] + sbr->m[1]) {
> + for (k = 0; sbr->f_master[k] < goal_sb; k++) ;
> + } else
> + k = sbr->n_master;
> +
> + do {
> + int odd = 0;
> + for (i = k; i == k || sb > (sbr->k[0] - 1 + msb - odd); i--) {
> + sb = sbr->f_master[i];
> + odd = (sb - 2 + sbr->k[0]) & 1;
the -2 is useless
and the condition can probably be written wthout odd
maybe:
sb > ((msb - 1)&~1) + sbr->k[0]
works?
> + }
> +
> + sbr->patch_num_subbands[sbr->num_patches] = FFMAX(sb - usb, 0);
> + sbr->patch_start_subband[sbr->num_patches] = sbr->k[0] - odd - sbr->patch_num_subbands[sbr->num_patches];
> +
> + if (sbr->patch_num_subbands[sbr->num_patches] > 0) {
> + usb = sb;
> + msb = sb;
> + sbr->num_patches++;
> + } else
> + msb = sbr->k[4];
> +
> + if (sbr->f_master[k] - sb < 3)
> + k = sbr->n_master;
> + } while (sb != sbr->k[4] + sbr->m[1]);
> +
> + if ((sbr->patch_num_subbands[sbr->num_patches-1] < 3) && (sbr->num_patches > 1))
> + sbr->num_patches--;
superflous ()
[...]
> +static int sbr_grid(AACContext *ac, SpectralBandReplication *sbr,
> + GetBitContext *gb, SBRData *ch_data)
> +{
> + int i;
> +
> + ch_data->bs_freq_res[0] = ch_data->bs_freq_res[ch_data->bs_num_env[1]];
> + ch_data->bs_num_env[0] = ch_data->bs_num_env[1];
> + ch_data->bs_amp_res = sbr->bs_amp_res_header;
> +
> + switch (ch_data->bs_frame_class = get_bits(gb, 2)) {
> + case FIXFIX:
> + ch_data->bs_num_env[1] = 1 << get_bits(gb, 2);
> + if (ch_data->bs_num_env[1] == 1)
> + ch_data->bs_amp_res = 0;
> +
> + ch_data->bs_freq_res[1] = get_bits1(gb);
> + for (i = 1; i < ch_data->bs_num_env[1]; i++)
> + ch_data->bs_freq_res[i + 1] = ch_data->bs_freq_res[1];
> + break;
> + case FIXVAR:
> + ch_data->bs_var_bord[1] = get_bits(gb, 2);
> + ch_data->bs_num_rel[1] = get_bits(gb, 2);
> + ch_data->bs_num_env[1] = ch_data->bs_num_rel[1] + 1;
> +
> + for (i = 0; i < ch_data->bs_num_rel[1]; i++)
> + ch_data->bs_rel_bord[1][i] = 2 * get_bits(gb, 2) + 2;
> +
> + ch_data->bs_pointer = get_bits(gb, ceil_log2[ch_data->bs_num_env[1]]);
> +
> + for (i = 0; i < ch_data->bs_num_env[1]; i++)
> + ch_data->bs_freq_res[ch_data->bs_num_env[1] - i] = get_bits1(gb);
> + break;
> + case VARFIX:
> + ch_data->bs_var_bord[0] = get_bits(gb, 2);
> + ch_data->bs_num_rel[0] = get_bits(gb, 2);
> + ch_data->bs_num_env[1] = ch_data->bs_num_rel[0] + 1;
> +
> + for (i = 0; i < ch_data->bs_num_rel[0]; i++)
> + ch_data->bs_rel_bord[0][i] = 2 * get_bits(gb, 2) + 2;
> +
> + ch_data->bs_pointer = get_bits(gb, ceil_log2[ch_data->bs_num_env[1]]);
> +
> + for (i = 0; i < ch_data->bs_num_env[1]; i++)
> + ch_data->bs_freq_res[i + 1] = get_bits1(gb);
> + break;
> + case VARVAR:
> + ch_data->bs_var_bord[0] = get_bits(gb, 2);
> + ch_data->bs_var_bord[1] = get_bits(gb, 2);
> + ch_data->bs_num_rel[0] = get_bits(gb, 2);
> + ch_data->bs_num_rel[1] = get_bits(gb, 2);
> + ch_data->bs_num_env[1] = ch_data->bs_num_rel[0] + ch_data->bs_num_rel[1] + 1;
> +
> + for (i = 0; i < ch_data->bs_num_rel[0]; i++)
> + ch_data->bs_rel_bord[0][i] = 2 * get_bits(gb, 2) + 2;
> + for (i = 0; i < ch_data->bs_num_rel[1]; i++)
> + ch_data->bs_rel_bord[1][i] = 2 * get_bits(gb, 2) + 2;
> +
> + ch_data->bs_pointer = get_bits(gb, ceil_log2[ch_data->bs_num_env[1]]);
> +
> + for (i = 0; i < ch_data->bs_num_env[1]; i++)
> + ch_data->bs_freq_res[i + 1] = get_bits1(gb);
> + break;
> + }
this can be done simpler and cleaner
> +
> + if (ch_data->bs_frame_class == FIXFIX && ch_data->bs_num_env[1] > 4) {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Invalid bitstream, too many SBR envelopes in FIXFIX type SBR frame: %d\n",
> + ch_data->bs_num_env[1]);
> + return -1;
> + }
> + if (ch_data->bs_frame_class == VARVAR && ch_data->bs_num_env[1] > 5) {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "Invalid bitstream, too many SBR envelopes in VARVAR type SBR frame: %d\n",
> + ch_data->bs_num_env[1]);
> + return -1;
> + }
> +
> + ch_data->bs_num_noise = (ch_data->bs_num_env[1] > 1) + 1;
> +
> + return 0;
> +}
> +
> +static void sbr_grid_copy(SBRData *dst, const SBRData *src) {
> + //These variables are saved from the previous frame rather than copied
> + dst->bs_freq_res[0] = dst->bs_freq_res[dst->bs_num_env[1]];
> + dst->bs_num_env[0] = dst->bs_num_env[1];
> +
> + //These variables are read from the bitstream and therefore copied
> + memcpy(dst->bs_freq_res+1, src->bs_freq_res+1, sizeof(dst->bs_freq_res)-1);
> + memcpy(dst->bs_num_env+1, src->bs_num_env+1, sizeof(dst->bs_num_env)-1);
the len is only correct when its a (u)int8_t
> + memcpy(dst->bs_var_bord, src->bs_var_bord, sizeof(dst->bs_var_bord));
> + memcpy(dst->bs_rel_bord, src->bs_rel_bord, sizeof(dst->bs_rel_bord));
> + memcpy(dst->bs_num_rel, src->bs_num_rel, sizeof(dst->bs_rel_bord));
> + dst->bs_amp_res = src->bs_amp_res;
> + dst->bs_num_noise = src->bs_num_noise;
> + dst->bs_pointer = src->bs_pointer;
> + dst->bs_frame_class = src->bs_frame_class;
> +}
> +
> +static void sbr_dtdf(SpectralBandReplication *sbr, GetBitContext *gb,
> + SBRData *ch_data)
i wonder what dtdf means
> +{
> + int i;
> +
> + for (i = 0; i < ch_data->bs_num_env[1]; i++)
> + ch_data->bs_df_env[i] = get_bits1(gb);
> + for (i = 0; i < ch_data->bs_num_noise; i++)
> + ch_data->bs_df_noise[i] = get_bits1(gb);
> +}
> +
> +static void sbr_invf(SpectralBandReplication *sbr, GetBitContext *gb,
> + SBRData *ch_data)
> +{
> + int i;
> +
> + memcpy(ch_data->bs_invf_mode[1], ch_data->bs_invf_mode[0], 5 * sizeof(uint8_t));
> + for (i = 0; i < sbr->n_q; i++)
> + ch_data->bs_invf_mode[0][i] = get_bits(gb, 2);
> +}
> +
> +static void sbr_envelope(SpectralBandReplication *sbr, GetBitContext *gb,
> + SBRData *ch_data, int ch)
> +{
> + int bits, max_depth;
> + int i, j;
> + VLC_TYPE (*t_huff)[2], (*f_huff)[2];
> + int t_lav, f_lav;
> +
> + if (sbr->bs_coupling && ch) {
> + max_depth = 2;
> + if (ch_data->bs_amp_res) {
> + bits = 5;
> + t_huff = vlc_sbr[T_HUFFMAN_ENV_BAL_3_0DB].table;
> + t_lav = vlc_sbr_lav[T_HUFFMAN_ENV_BAL_3_0DB];
> + f_huff = vlc_sbr[F_HUFFMAN_ENV_BAL_3_0DB].table;
> + f_lav = vlc_sbr_lav[F_HUFFMAN_ENV_BAL_3_0DB];
can be vertically aligned
> + } else {
> + bits = 6;
> + t_huff = vlc_sbr[T_HUFFMAN_ENV_BAL_1_5DB].table;
> + t_lav = vlc_sbr_lav[T_HUFFMAN_ENV_BAL_1_5DB];
> + f_huff = vlc_sbr[F_HUFFMAN_ENV_BAL_1_5DB].table;
> + f_lav = vlc_sbr_lav[F_HUFFMAN_ENV_BAL_1_5DB];
> + }
> + } else {
> + max_depth = 3;
> + if (ch_data->bs_amp_res) {
> + bits = 6;
> + t_huff = vlc_sbr[T_HUFFMAN_ENV_3_0DB].table;
> + t_lav = vlc_sbr_lav[T_HUFFMAN_ENV_3_0DB];
> + f_huff = vlc_sbr[F_HUFFMAN_ENV_3_0DB].table;
> + f_lav = vlc_sbr_lav[F_HUFFMAN_ENV_3_0DB];
> + } else {
> + bits = 7;
> + t_huff = vlc_sbr[T_HUFFMAN_ENV_1_5DB].table;
> + t_lav = vlc_sbr_lav[T_HUFFMAN_ENV_1_5DB];
> + f_huff = vlc_sbr[F_HUFFMAN_ENV_1_5DB].table;
> + f_lav = vlc_sbr_lav[F_HUFFMAN_ENV_1_5DB];
> + }
> + }
> +
> + for (i = 0; i < ch_data->bs_num_env[1]; i++) {
> + if (!ch_data->bs_df_env[i]) {
> + ch_data->bs_data_env[i][0] = get_bits(gb, bits); // bs_env_start_value_balance
> + for (j = 1; j < sbr->n[ch_data->bs_freq_res[i + 1]]; j++)
> + ch_data->bs_data_env[i][j] = get_vlc2(gb, f_huff, 9, max_depth) - f_lav;
> + } else {
> + for (j = 0; j < sbr->n[ch_data->bs_freq_res[i + 1]]; j++)
> + ch_data->bs_data_env[i][j] = get_vlc2(gb, t_huff, 9, max_depth) - t_lav;
max_depth should be a constant, setting it to the maximum is faster
than if it is variable
[...]
> +static void sbr_sinusoidal_coding(SpectralBandReplication *sbr,
> + GetBitContext *gb, SBRData *ch_data)
> +{
> + int i;
> + for (i = 0; i < sbr->n[1]; i++)
> + ch_data->bs_add_harmonic[i] = get_bits1(gb);
> +}
i guess this can be consdered a duplicate of all the other identical
cases that read a single linear array of n bits
[...]
> +static void sbr_single_channel_element(AACContext *ac,
> + SpectralBandReplication *sbr,
> + GetBitContext *gb)
> +{
> + if (get_bits1(gb)) // bs_data_extra
> + skip_bits(gb, 4); // bs_reserved
> +
> + sbr_grid(ac, sbr, gb, &sbr->data[0]);
> + sbr_dtdf(sbr, gb, &sbr->data[0]);
> + sbr_invf(sbr, gb, &sbr->data[0]);
> + sbr_envelope(sbr, gb, &sbr->data[0], 0);
> + sbr_noise(sbr, gb, &sbr->data[0], 0);
> +
> + if ((sbr->data[0].bs_add_harmonic_flag = get_bits1(gb)))
> + sbr_sinusoidal_coding(sbr, gb, &sbr->data[0]);
> +}
please name functions that primarely read bitstream data consistently
like read_sbr_grid()
so they can be distinguished from functions performing bitstream reader
independant calculations like apply_mdct()
> +
> +static void sbr_channel_pair_element(AACContext *ac,
> + SpectralBandReplication *sbr,
> + GetBitContext *gb)
> +{
> + if (get_bits1(gb)) // bs_data_extra
> + skip_bits(gb, 8); // bs_reserved
> +
> + if ((sbr->bs_coupling = get_bits1(gb))) {
> + sbr_grid(ac, sbr, gb, &sbr->data[0]);
> + sbr_grid_copy(&sbr->data[1], &sbr->data[0]);
> + sbr_dtdf(sbr, gb, &sbr->data[0]);
> + sbr_dtdf(sbr, gb, &sbr->data[1]);
> + sbr_invf(sbr, gb, &sbr->data[0]);
> + memcpy(sbr->data[1].bs_invf_mode[1], sbr->data[1].bs_invf_mode[0], sizeof(sbr->data[1].bs_invf_mode[0]));
> + memcpy(sbr->data[1].bs_invf_mode[0], sbr->data[0].bs_invf_mode[0], sizeof(sbr->data[1].bs_invf_mode[0]));
> + sbr_envelope(sbr, gb, &sbr->data[0], 0);
> + sbr_noise(sbr, gb, &sbr->data[0], 0);
> + sbr_envelope(sbr, gb, &sbr->data[1], 1);
> + sbr_noise(sbr, gb, &sbr->data[1], 1);
> + } else {
> + sbr_grid(ac, sbr, gb, &sbr->data[0]);
> + sbr_grid(ac, sbr, gb, &sbr->data[1]);
> + sbr_dtdf(sbr, gb, &sbr->data[0]);
> + sbr_dtdf(sbr, gb, &sbr->data[1]);
> + sbr_invf(sbr, gb, &sbr->data[0]);
> + sbr_invf(sbr, gb, &sbr->data[1]);
> + sbr_envelope(sbr, gb, &sbr->data[0], 0);
> + sbr_envelope(sbr, gb, &sbr->data[1], 1);
> + sbr_noise(sbr, gb, &sbr->data[0], 0);
> + sbr_noise(sbr, gb, &sbr->data[1], 1);
> + }
can be simplified by factorizing things out
[...]
> +/// SBR Envelope and Noise Floor Decoding (14496-3 sp04 p201)
> +static void sbr_env_noise_floors(SpectralBandReplication *sbr, SBRData *ch_data,
> + int ch)
> +{
> + int delta = (ch == 1 && sbr->bs_coupling == 1) + 1;
> + int i, k, L;
> + const int temp = sbr->n[1] & 1;
> + for (L = 0; L < ch_data->bs_num_env[1]; L++) {
> + if (ch_data->bs_df_env[L]) {
> + // bs_freq_res[0] == bs_freq_res[bs_num_env[1]] from prev frame
> + if (ch_data->bs_freq_res[L + 1] == ch_data->bs_freq_res[L]) {
> + for (k = 0; k < sbr->n[ch_data->bs_freq_res[L + 1]]; k++)
> + ch_data->env_facs[L + 1][k] = ch_data->env_facs[L][k] + delta * ch_data->bs_data_env[L][k];
> + } else if (ch_data->bs_freq_res[L + 1]) {
> + for (k = 0; k < sbr->n[ch_data->bs_freq_res[L + 1]]; k++) {
> + i = (k + temp) >> 1; // find i such that f_tablelow[i] <= f_tablehigh[k] < f_tablelow[i + 1]
> + ch_data->env_facs[L + 1][k] = ch_data->env_facs[L][i] + delta * ch_data->bs_data_env[L][k];
> + }
> + } else {
> + for (k = 0; k < sbr->n[ch_data->bs_freq_res[L + 1]]; k++) {
> + i = k ? 2*k - temp : 0; // find i such that f_tablehigh[i] == f_tablelow[k]
> + ch_data->env_facs[L + 1][k] = ch_data->env_facs[L][i] + delta * ch_data->bs_data_env[L][k];
> + }
> + }
> + } else {
> + ch_data->env_facs[L + 1][0] = delta * ch_data->bs_data_env[L][0];
> + for (k = 1; k < sbr->n[ch_data->bs_freq_res[L + 1]]; k++)
> + ch_data->env_facs[L + 1][k] = ch_data->env_facs[L + 1][k - 1] + delta * ch_data->bs_data_env[L][k];
> + }
> + }
please dont use l as variable name it looks like 1 in many fonts and
makes the code harder to read.
It is not reasonable to expect everyone who wishes to read the code to
switch fonts or run a search and replace over the code, like i just did
so i can review the code.
Back to the actual code, cant this be merged with reading the data from
the bitstream? it seems the intermediate bs_data_env could at least be
a local array
[...]
> +/// Dequantization and stereo decoding (14496-3 sp04 p203)
> +static void sbr_dequant(SpectralBandReplication *sbr, int id_aac)
> +{
> + int k, L;
> + int ch;
> +
> + if (id_aac == TYPE_CPE && sbr->bs_coupling) {
> + float alpha = sbr->data[0].bs_amp_res ? 1.0f : 0.5f;
> + float pan_offset = sbr->data[0].bs_amp_res ? 12.0f : 24.0f;
vertical align
> + for (L = 1; L <= sbr->data[0].bs_num_env[1]; L++) {
> + for (k = 0; k < sbr->n[sbr->data[0].bs_freq_res[L]]; k++) {
> + float temp1 = exp2f(sbr->data[0].env_facs[L][k] * alpha + 7.0f);
> + float temp2 = exp2f((pan_offset - sbr->data[1].env_facs[L][k]) * alpha);
isnt env_facs integer here? i so this can probably be done more
efficiently with a LUT
[...]
> +/** High Frequency Generation (14496-3 sp04 p214+) and Inverse Filtering
> + * (14496-3 sp04 p214)
> + * Warning: This routine does not seem numerically stable.
> + */
> +static void sbr_hf_inverse_filter(float (*alpha0)[2], float (*alpha1)[2],
> + float X_low[32][40][2], int k0)
const X_low ?
> +{
> + int i, j, k, n;
> + for (k = 0; k < k0; k++) {
> + float phi[3][2][2], dk;
> +
> + for (i = 0; i < 3; i++) {
> + for (j = 0; j < 2; j++) {
> + unsigned int idxtmp1 = ENVELOPE_ADJUSTMENT_OFFSET - i;
> + unsigned int idxtmp2 = ENVELOPE_ADJUSTMENT_OFFSET - (j + 1);
> +
> + phi[i][j][0] = 0.0f;
> + phi[i][j][1] = 0.0f;
these can be put inside the if() below
> +
> + if (i <= j + 1)
> + for (n = 0; n < 16 * 2 + 6; n++) {
> + unsigned int idx1 = n + idxtmp1;
> + unsigned int idx2 = n + idxtmp2;
> + phi[i][j][0] += X_low[k][idx1][0] * X_low[k][idx2][0] +
> + X_low[k][idx1][1] * X_low[k][idx2][1];
> + if (i != j + 1) { // imaginary part
> + phi[i][j][1] += X_low[k][idx1][1] * X_low[k][idx2][0] -
> + X_low[k][idx1][0] * X_low[k][idx2][1];
> + }
> + }
> + }
> + }
if i dont misread this then the implementation is not ideal
you calcuate
X[i0+n] * X[i1+n] for n=0..38
and i0,i1 in 0,1 0,2 1,1 1,2 2,2
here 1,1/2,2 and 0,1/1,2 calculate many multiplies twice
also the if() and all the multidimendimensional array deref in the inner loop
might have a bad effect on speed (depending on compiler)
[...]
> +/// Chirp Factors (14496-3 sp04 p214)
> +static void sbr_chirp(SpectralBandReplication *sbr, SBRData *ch_data)
> +{
> + int i;
> + float new_bw;
> + float temp_bw;
> +
> + for (i = 0; i < sbr->n_q; i++) {
> + switch (ch_data->bs_invf_mode[0][i]) {
> + case 0:
> + if (ch_data->bs_invf_mode[1][i] == 1) {
> + new_bw = 0.6f;
> + } else
> + new_bw = 0.0f;
> + break;
> + case 1:
> + if (!ch_data->bs_invf_mode[1][i]) {
> + new_bw = 0.6f;
> + } else
> + new_bw = 0.75f;
> + break;
> + case 2:
> + new_bw = 0.9f;
> + break;
> + default:
> + new_bw = 0.98f;
> + break;
> + }
if(ch_data->bs_invf_mode[0][i]+ch_data->bs_invf_mode[1][i] == 1){
new_bw = 0.6f;
}else{
new_bw = table[ ch_data->bs_invf_mode[0][i] ];
}
> +
> + if (new_bw < ch_data->bw_array[1][i]) {
> + temp_bw = 0.75f * new_bw + 0.25f * ch_data->bw_array[1][i];
> + } else
> + temp_bw = 0.90625f * new_bw + 0.09375f * ch_data->bw_array[1][i];
> + ch_data->bw_array[0][i] = temp_bw < 0.015625f ? 0.0f : temp_bw;
> + }
> +
> + // update previous bw_array values
> + memcpy(ch_data->bw_array[1], ch_data->bw_array[0], 5 * sizeof(float));
why are there 2 arrays ? seems like you use just one
> +}
> +
> +static inline int find_freq_subband(uint16_t *table, int nel, int needle)
> +{
> + int i;
> + for (i = 0; i < nel; i++) {
> + if (needle >= table[i] && needle < table[i + 1])
> + return i;
> + }
if i assume the table is sorted then you need just one of the 2 compares
and you also seem to restart searching sequential elements from the start
> + return -1;
> +}
> +
> +/// Generate the subband filtered lowband
> +static int sbr_lf_gen(AACContext *ac, SpectralBandReplication *sbr,
> + float X_low[32][40][2], float W[2][32][32][2]) {
nothing being const looks suspicous
> + int k, L;
> + const int t_HFGen = 8;
> + const int l_f = 32;
> + memset(X_low, 0, 32*sizeof(*X_low));
> + for (k = 0; k < sbr->k[4]; k++) {
> + for (L = t_HFGen; L < l_f + t_HFGen; L++) {
> + X_low[k][L][0] = W[1][L - t_HFGen][k][0];
> + X_low[k][L][1] = W[1][L - t_HFGen][k][1];
> + }
> + }
> + for (k = 0; k < sbr->k[3]; k++) {
> + for (L = 0; L < t_HFGen; L++) {
> + X_low[k][L][0] = W[0][L + l_f - t_HFGen][k][0];
> + X_low[k][L][1] = W[0][L + l_f - t_HFGen][k][1];
> + }
> + }
the memset above seems to set some elements to 0 that are never read
also cant this copying around be avoided and "W" be accessed directly
by the code that uses X_low?
> + return 0;
> +}
> +
> +/// High Frequency Generator (14496-3 sp04 p215)
> +static int sbr_hf_gen(AACContext *ac, SpectralBandReplication *sbr,
> + float X_high[64][40][2], float X_low[32][40][2], float (*alpha0)[2],
> + float (*alpha1)[2], float bw_array[2][5], uint8_t *t_env,
> + int bs_num_env)
> +{
> + int i, x, L;
> + int k = sbr->k[4];
> + for (i = 0; i < sbr->num_patches; i++) {
> + for (x = 0; x < sbr->patch_num_subbands[i]; x++, k++) {
> + const int g = find_freq_subband(sbr->f_tablenoise, sbr->n_q + 1, k);
> + const int p = sbr->patch_start_subband[i] + x;
> +
> + if (g < 0) {
> + av_log(ac->avccontext, AV_LOG_ERROR,
> + "ERROR : no subband found for frequency %d\n", k);
> + return -1;
> + }
> +
> + for (L = 2 * t_env[0]; L < 2 * t_env[bs_num_env]; L++) {
> + const int idx = L + ENVELOPE_ADJUSTMENT_OFFSET;
> + X_high[k][idx][0] =
> + (X_low[p][idx - 2][0] * alpha1[p][0] -
> + X_low[p][idx - 2][1] * alpha1[p][1]) * bw_array[0][g] * bw_array[0][g] +
> + (X_low[p][idx - 1][0] * alpha0[p][0] -
> + X_low[p][idx - 1][1] * alpha0[p][1]) * bw_array[0][g] +
> + X_low[p][idx][0];
> + X_high[k][idx][1] =
> + (X_low[p][idx - 2][1] * alpha1[p][0] +
> + X_low[p][idx - 2][0] * alpha1[p][1]) * bw_array[0][g] * bw_array[0][g] +
> + (X_low[p][idx - 1][1] * alpha0[p][0] +
> + X_low[p][idx - 1][0] * alpha0[p][1]) * bw_array[0][g] +
> + X_low[p][idx][1];
> + }
bw_array can be multiplied into alpha cuting 6 multiplies away from this
loop
> + }
> + }
> + if (k < sbr->m[1] + sbr->k[4])
> + memset(X_high + k, 0, (sbr->m[1] + sbr->k[4] - k) * sizeof(*X_high));
> +
> + return 0;
> +}
> +
> +/// Generate the subband filtered lowband
> +static int sbr_x_gen(SpectralBandReplication *sbr,
> + float X[2][32][64], float X_low[32][40][2], float Y[2][38][64][2], int ch) {
> + int k, L;
> + const int l_f = 32;
> + const int l_Temp = FFMAX(2*sbr->data[ch].t_env_num_env_old - l_f, 0);
> + memset(X, 0, 2*sizeof(*X));
> + for (k = 0; k < sbr->k[3]; k++) {
> + for (L = 0; L < l_Temp; L++) {
> + X[0][L][k] = X_low[k][L + ENVELOPE_ADJUSTMENT_OFFSET][0];
> + X[1][L][k] = X_low[k][L + ENVELOPE_ADJUSTMENT_OFFSET][1];
> + }
> + }
> + for (; k < sbr->k[3] + sbr->m[0]; k++) {
> + for (L = 0; L < l_Temp; L++) {
> + X[0][L][k] = Y[0][L + l_f][k][0];
> + X[1][L][k] = Y[0][L + l_f][k][1];
> + }
> + }
> +
> + for (k = 0; k < sbr->k[4]; k++) {
> + for (L = l_Temp; L < l_f; L++) {
> + X[0][L][k] = X_low[k][L + ENVELOPE_ADJUSTMENT_OFFSET][0];
> + X[1][L][k] = X_low[k][L + ENVELOPE_ADJUSTMENT_OFFSET][1];
> + }
> + }
> + for (; k < sbr->k[4] + sbr->m[1]; k++) {
> + for (L = l_Temp; L < l_f; L++) {
> + X[0][L][k] = Y[1][L][k][0];
> + X[1][L][k] = Y[1][L][k][1];
> + }
> + }
> + return 0;
> +}
more copying code ....
can that be avoided?
and before you reply that the *dct needs it in that order in the next
function, question is why is it not in that order to begin with?
[...]
> + for (i = ilb; i < iub; i++) {
> + sum += X_high[m + sbr->k[4]][i][0] * X_high[m + sbr->k[4]][i][0] +
> + X_high[m + sbr->k[4]][i][1] * X_high[m + sbr->k[4]][i][1];
> + }
dont we have a optimized funtion for taking the dot product of a
real valued float vector alraedy?
> + e_curr[L][m] = sum * recip_env_size;
> + }
> + }
> + } else {
> + int k, p;
> +
> + for (L = 0; L < ch_data->bs_num_env[1]; L++) {
> + const int env_size = 2 * (ch_data->t_env[L + 1] - ch_data->t_env[L]);
> + int ilb = ch_data->t_env[L] * 2 + ENVELOPE_ADJUSTMENT_OFFSET;
> + int iub = ch_data->t_env[L + 1] * 2 + ENVELOPE_ADJUSTMENT_OFFSET;
> + const uint16_t *table = ch_data->bs_freq_res[L + 1] ? sbr->f_tablehigh : sbr->f_tablelow;
> +
> + for (p = 0; p < sbr->n[ch_data->bs_freq_res[L + 1]]; p++) {
> + float sum = 0.0f;
> + const int den = env_size * (table[p + 1] - table[p]);
> +
> + for (k = table[p]; k < table[p + 1]; k++) {
> + for (i = ilb; i < iub; i++) {
> + sum += X_high[k][i][0] * X_high[k][i][0] +
> + X_high[k][i][1] * X_high[k][i][1];
> + }
duplicate
[...]
> + static const int8_t phi[2][4] = {
> + { 1, 0, -1, 0}, // real
> + { 0, 1, 0, -1}, // imaginary
> + };
> + float (*g_temp)[48] = ch_data->g_temp, (*q_temp)[48] = ch_data->q_temp;
> + int indexnoise = ch_data->f_indexnoise;
> + int indexsine = ch_data->f_indexsine;
> + memcpy(Y[0], Y[1], sizeof(Y[0]));
> +
> + if (sbr->reset) {
> + for (i = 0; i < h_SL; i++) {
> + memcpy(g_temp[i + 2*ch_data->t_env[0]], sbr->gain[0], m_max * sizeof(sbr->gain[0][0]));
> + memcpy(q_temp[i + 2*ch_data->t_env[0]], sbr->q_m[0], m_max * sizeof(sbr->q_m[0][0]));
> + }
> + } else if (h_SL) {
> + memcpy(g_temp[2*ch_data->t_env[0]], g_temp[2*ch_data->t_env_num_env_old], 4*sizeof(g_temp[0]));
> + memcpy(q_temp[2*ch_data->t_env[0]], q_temp[2*ch_data->t_env_num_env_old], 4*sizeof(q_temp[0]));
> + }
> +
> + for (L = 0; L < ch_data->bs_num_env[1]; L++) {
> + for (i = 2 * ch_data->t_env[L]; i < 2 * ch_data->t_env[L + 1]; i++) {
> + memcpy(g_temp[h_SL + i], sbr->gain[L], m_max * sizeof(sbr->gain[0][0]));
> + memcpy(q_temp[h_SL + i], sbr->q_m[L], m_max * sizeof(sbr->q_m[0][0]));
> + }
> + }
> +
> + for (L = 0; L < ch_data->bs_num_env[1]; L++) {
> + for (i = 2 * ch_data->t_env[L]; i < 2 * ch_data->t_env[L + 1]; i++) {
> + int phi_sign = (1 - 2*(kx & 1));
> +
> + if (h_SL && L != l_a[0] && L != l_a[1]) {
> + for (m = 0; m < m_max; m++) {
> + const int idx1 = i + h_SL;
> + float g_filt = 0.0f;
> + for (j = 0; j <= h_SL; j++)
> + g_filt += g_temp[idx1 - j][m] * h_smooth[j];
> + Y[1][i][m + kx][0] =
> + X_high[m + kx][i + ENVELOPE_ADJUSTMENT_OFFSET][0] * g_filt;
> + Y[1][i][m + kx][1] =
> + X_high[m + kx][i + ENVELOPE_ADJUSTMENT_OFFSET][1] * g_filt;
> + }
> + } else {
> + for (m = 0; m < m_max; m++) {
> + const float g_filt = g_temp[i + h_SL][m];
> + Y[1][i][m + kx][0] =
> + X_high[m + kx][i + ENVELOPE_ADJUSTMENT_OFFSET][0] * g_filt;
> + Y[1][i][m + kx][1] =
> + X_high[m + kx][i + ENVELOPE_ADJUSTMENT_OFFSET][1] * g_filt;
> + }
> + }
> +
> + if (L != l_a[0] && L != l_a[1]) {
> + for (m = 0; m < m_max; m++) {
> + indexnoise = (indexnoise + 1) & 0x1ff;
> + if (sbr->s_m[L][m]) {
> + Y[1][i][m + kx][0] +=
> + sbr->s_m[L][m] * phi[0][indexsine];
> + Y[1][i][m + kx][1] +=
> + sbr->s_m[L][m] * (phi[1][indexsine] * phi_sign);
> + } else {
> + float q_filt;
> + if (h_SL) {
> + const int idx1 = i + h_SL;
> + q_filt = 0.0f;
> + for (j = 0; j <= h_SL; j++)
> + q_filt += q_temp[idx1 - j][m] * h_smooth[j];
> + } else {
> + q_filt = q_temp[i][m];
> + }
> + Y[1][i][m + kx][0] +=
> + q_filt * sbr_noise_table[indexnoise][0];
> + Y[1][i][m + kx][1] +=
> + q_filt * sbr_noise_table[indexnoise][1];
> + }
> + phi_sign = -phi_sign;
> + }
> + } else {
> + indexnoise = (indexnoise + m_max) & 0x1ff;
> + for (m = 0; m < m_max; m++) {
> + Y[1][i][m + kx][0] +=
> + sbr->s_m[L][m] * phi[0][indexsine];
> + Y[1][i][m + kx][1] +=
> + sbr->s_m[L][m] * (phi[1][indexsine] * phi_sign);
> + phi_sign = -phi_sign;
> + }
> + }
> + indexsine = (indexsine + 1) & 3;
all the multiplies by phi* are 0,1,-1
half of them are by 0
[...]
> +
> +/**
> + * Spectral Band Replication header - spectrum parameters that invoke a reset if they differ from the previous header.
> + */
> +typedef struct {
> + uint8_t bs_start_freq;
> + uint8_t bs_stop_freq;
> + uint8_t bs_xover_band;
> +
> + /**
> + * @defgroup bs_header_extra_1 Variables associated with bs_header_extra_1
> + * @{
> + */
> + uint8_t bs_freq_scale;
> + uint8_t bs_alter_scale;
> + uint8_t bs_noise_bands;
> + /** @} */
> +} SpectrumParameters;
> +
> +#define SBR_SYNTHESIS_BUF_SIZE ((1280-128)*2)
> +
> +/**
> + * Spectral Band Replication per channel data
> + */
> +typedef struct {
> + /**
> + * @defgroup bitstream Main bitstream data variables
> + * @{
> + */
> + uint8_t bs_frame_class;
> + uint8_t bs_add_harmonic_flag;
> + uint8_t bs_num_env[2];
> + uint8_t bs_freq_res[7];
> + uint8_t bs_var_bord[2];
> + uint8_t bs_num_rel[2];
> + uint8_t bs_rel_bord[2][3];
> + uint8_t bs_pointer;
> + uint8_t bs_num_noise;
> + uint8_t bs_df_env[5];
> + uint8_t bs_df_noise[2];
> + uint8_t bs_invf_mode[2][5];
> + int32_t bs_data_env[7][32];
> + int32_t bs_data_noise[2][5];
> + uint8_t bs_add_harmonic[48];
> + uint8_t bs_amp_res;
> + /** @} */
> +
> + /**
> + * @defgroup state State variables
> + * @{
> + */
> + DECLARE_ALIGNED(16, float, synthesis_filterbank_samples)[SBR_SYNTHESIS_BUF_SIZE];
> + DECLARE_ALIGNED(16, float, analysis_filterbank_samples) [1312];
> + int synthesis_filterbank_samples_offset;
> + int l_a[2];
> + float bw_array[2][5];
> + float W[2][32][32][2];
> + float Y[2][38][64][2];
> + float g_temp[42][48];
> + float q_temp[42][48];
> + uint8_t s_indexmapped[8][48];
> + float env_facs[6][48];
> + float noise_facs[3][5];
> + uint8_t t_env[8];
> + uint8_t t_env_num_env_old;
> + uint8_t t_q[3];
> + uint16_t f_indexnoise;
> + uint8_t f_indexsine;
> + /** @} */
> +} SBRData;
> +
> +/**
> + * Spectral Band Replication
> + */
> +typedef struct {
> + int32_t sample_rate;
> + uint8_t start;
> + uint8_t reset;
> + SpectrumParameters spectrum_params[2];
> + uint8_t bs_amp_res_header;
> + /**
> + * @defgroup bs_header_extra_2 variables associated with bs_header_extra_2
> + * @{
> + */
> + uint8_t bs_limiter_bands;
> + uint8_t bs_limiter_gains;
> + uint8_t bs_interpol_freq;
> + uint8_t bs_smoothing_mode;
> + /** @} */
> + uint8_t bs_coupling;
> + uint8_t k[5]; ///< k0, k1, k2, kx', and kx respectively
this seems never used as an array, why is it one?
also why are so many variables fixed small sizes? this can cause a
speed loss on some architectures, and even x86 with gcc seems to have a
dislike for such things from what ive seen recently in h264 optims
> + uint8_t m[2]; ///< M' and M respectively
> + uint8_t n_master;
> + SBRData data[2];
> + uint8_t n[2]; ///< n_low and n_high respectively
> + uint8_t n_q;
> + uint8_t n_lim;
> + uint16_t f_master[49];
> + uint16_t f_tablelow[25];
> + uint16_t f_tablehigh[49];
> + uint16_t f_tablenoise[6];
> + uint16_t f_tablelim[29];
[...]
> + float X_low[32][40][2];
> + float X_high[64][40][2];
> + DECLARE_ALIGNED(16, float, X)[2][32][64];
> + float alpha0[64][2];
> + float alpha1[64][2];
> + float e_origmapped[7][48];
> + float q_mapped[7][48];
> + uint8_t s_mapped[7][48];
> + float e_curr[7][48];
> + float q_m[7][48];
> + float s_m[7][48];
It would tremendously help code readability and thus also me reviewing
if these variables where things that made sense and did not require one
to look _each_ up in a spec that isnt freely available.
A little like FOSS encrypted with a key one has to pay for ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20100304/ae056cdd/attachment.pgp>
More information about the ffmpeg-devel
mailing list