[Ffmpeg-devel] [PATCH] updated LGPL AC-3 decoder
Michael Niedermayer
michaelni
Sat Feb 10 16:30:41 CET 2007
Hi
On Sat, Feb 10, 2007 at 01:02:20AM -0500, Justin Ruggles wrote:
> Hello,
>
> Here is what I have so far in my attempts to merge the AC-3 decoder
> written by Kartikey Mahendra BHATT for the Google Summer of Code. There
> are some things which still need work, and I have some enhancements
> planned for after it's included. Right now, this gives generally
> working results, but has some issues.
>
> The 256-point MDCT is not working correctly. I'm still trying to get
> that fixed. Also, the downmixing seems to not always work like it
> should. The code could use some clean-up as well.
>
> The patch I've attached is against current SVN. The ac3.c and
> ac3_decoder.c files go into libavcodec. When it's committed, ac3.c will
> be created by doing an svn copy of ac3enc.c and then patching it to what
> I've included here. I think I got the build system stuff right, but I'm
> not completely sure.
you should have diffed ac3.c against ac3enc.c then, also keep in mind
that code duplication (i dont know if theres any after the copy and patch) is
unacceptable
>
> Here is a summary of the changes to the decoder:
> - used the new av_random for dithering
> - used the common bit allocation code from ac3enc.c (via ac3.c)
> - replaced the exponent decoding, which was from liba52 (GPL), with an
> implementation I wrote for my own AC-3 parser, which is LGPL.
> - reused some of the macros from ac3.h and got rid of ac3_decoder.h
>
> I really want to fix the short block MDCT before merging this, but I
> thought I would go ahead and post what I have so it could be reviewed.
[...]
> #include "avcodec.h"
> #include "ac3.h"
> #include "ac3tab.h"
>
> static inline int calc_lowcomp1(int a, int b0, int b1)
> {
> if ((b0 + 256) == b1) {
> a = 384 ;
> } else if (b0 > b1) {
> a = a - 64;
> if (a < 0) a=0;
> }
> return a;
> }
calc_lowcomp1(int a, int b0, int b1, int C){
if ((b0 + 256) == b1) {
a= C;
} else if (b0 > b1) {
a= FFMAX(a - 64, 0);
}
return a;
>
> static inline int calc_lowcomp(int a, int b0, int b1, int bin)
> {
> if (bin < 7) {
> if ((b0 + 256) == b1) {
> a = 384 ;
> } else if (b0 > b1) {
> a = a - 64;
> if (a < 0) a=0;
> }
> } else if (bin < 20) {
> if ((b0 + 256) == b1) {
> a = 320 ;
> } else if (b0 > b1) {
> a= a - 64;
> if (a < 0) a=0;
> }
> } else {
> a = a - 128;
> if (a < 0) a=0;
> }
> return a;
> }
if (bin < 7) {
return calc_lowcomp1(a, b0, b1, 384);
} else if (bin < 20) {
return calc_lowcomp1(a, b0, b1, 320);
} else {
return FFMAX(a - 128, 0);
}
>
> /* AC3 bit allocation. The algorithm is the one described in the AC3
> spec. */
> void ac3_parametric_bit_allocation(AC3BitAllocParameters *s, uint8_t *bap,
not doxygen compatible comment
> int8_t *exp, int start, int end,
> int snroffset, int fgain, int is_lfe,
> int deltbae,int deltnseg,
> uint8_t *deltoffst, uint8_t *deltlen, uint8_t *deltba)
> {
> int bin,i,j,k,end1,v,v1,bndstrt,bndend,lowcomp,begin;
> int fastleak,slowleak,address,tmp;
> int16_t psd[256]; /* scaled exponents */
> int16_t bndpsd[50]; /* interpolated exponents */
> int16_t excite[50]; /* excitation */
> int16_t mask[50]; /* masking value */
>
> /* exponent mapping to PSD */
> for(bin=start;bin<end;bin++) {
> psd[bin]=(3072 - (exp[bin] << 7));
> }
>
> /* PSD integration */
> j=start;
> k=masktab[start];
> do {
> v=psd[j];
> j++;
> end1=bndtab[k+1];
> if (end1 > end) end1=end;
end1= FFMIN(end1, end);
> for(i=j;i<end1;i++) {
> int c,adr;
> /* logadd */
> v1=psd[j];
> c=v-v1;
> if (c >= 0) {
> adr=c >> 1;
> if (adr > 255) adr=255;
> v=v + latab[adr];
> } else {
> adr=(-c) >> 1;
> if (adr > 255) adr=255;
> v=v1 + latab[adr];
> }
adr= FFMIN(FFABS(v-v1)>>1, 255)
v= FFMAX(v,v1) + latab[adr];
> j++;
> }
> bndpsd[k]=v;
> k++;
> } while (end > bndtab[k]);
>
> /* excitation function */
> bndstrt = masktab[start];
> bndend = masktab[end-1] + 1;
>
> if (bndstrt == 0) {
> lowcomp = 0;
> lowcomp = calc_lowcomp1(lowcomp, bndpsd[0], bndpsd[1]) ;
> excite[0] = bndpsd[0] - fgain - lowcomp ;
> lowcomp = calc_lowcomp1(lowcomp, bndpsd[1], bndpsd[2]) ;
> excite[1] = bndpsd[1] - fgain - lowcomp ;
> begin = 7 ;
> for (bin = 2; bin < 7; bin++) {
> if (!(is_lfe && bin == 6))
> lowcomp = calc_lowcomp1(lowcomp, bndpsd[bin], bndpsd[bin+1]) ;
> fastleak = bndpsd[bin] - fgain ;
> slowleak = bndpsd[bin] - s->sgain ;
> excite[bin] = fastleak - lowcomp ;
> if (!(is_lfe && bin == 6)) {
> if (bndpsd[bin] <= bndpsd[bin+1]) {
> begin = bin + 1 ;
> break ;
> }
> }
> }
>
> end1=bndend;
> if (end1 > 22) end1=22;
>
> for (bin = begin; bin < end1; bin++) {
> if (!(is_lfe && bin == 6))
> lowcomp = calc_lowcomp(lowcomp, bndpsd[bin], bndpsd[bin+1], bin) ;
>
> fastleak -= s->fdecay ;
> v = bndpsd[bin] - fgain;
> if (fastleak < v) fastleak = v;
FFMAX
>
> slowleak -= s->sdecay ;
> v = bndpsd[bin] - s->sgain;
> if (slowleak < v) slowleak = v;
FFMAX
>
> v=fastleak - lowcomp;
> if (slowleak > v) v=slowleak;
v= FFMAX(fastleak - lowcomp, slowleak)
[...]
> for (k = i; k < end1; k++) {
> address = (psd[i] - v) >> 5 ;
> if (address < 0) address=0;
> else if (address > 63) address=63;
clip()
[...]
also improvements to existing files (ac3enc.c) must be commited seperately as
they are seperate from the actual ac3 decoder
[...]
> #define ALT_BITSTREAM_READER
why?
>
> #include "avcodec.h"
> #include "ac3.h"
> #include "bitstream.h"
> #include "dsputil.h"
> #include "random.h"
>
> static const int nfchans_tbl[8] = { 2, 1, 2, 3, 3, 4, 4, 5 };
uint8_t
[...]
> /* Adjustments in dB gain */
> #define LEVEL_MINUS_3DB 0.7071067811865476
add "// sqrt(2)/2"
> #define LEVEL_MINUS_4POINT5DB 0.5946035575013605
> #define LEVEL_MINUS_6DB 0.5000000000000000
> #define LEVEL_PLUS_3DB 1.4142135623730951
add "// sqrt(2)"
[...]
> #define AC3_INPUT_DUALMONO 0x00
> #define AC3_INPUT_MONO 0x01
> #define AC3_INPUT_STEREO 0x02
> #define AC3_INPUT_3F 0x03
> #define AC3_INPUT_2F_1R 0x04
> #define AC3_INPUT_3F_1R 0x05
> #define AC3_INPUT_2F_2R 0x06
> #define AC3_INPUT_3F_2R 0x07
this could be changed to a enum
>
> typedef struct {
> uint16_t crc1;
> uint8_t fscod;
>
> uint8_t acmod;
trailing whitespace
> uint8_t cmixlev;
> uint8_t surmixlev;
> uint8_t dsurmod;
>
> uint8_t blksw;
> uint8_t dithflag;
> uint8_t cplinu;
> uint8_t chincpl;
> uint8_t phsflginu;
> uint8_t cplbegf;
> uint8_t cplendf;
> uint8_t cplcoe;
> uint32_t cplbndstrc;
> uint8_t rematstr;
> uint8_t rematflg;
> uint8_t cplexpstr;
> uint8_t lfeexpstr;
> uint8_t chexpstr[FBW_CHANNELS];
> uint8_t halfratecod;
> uint8_t sdcycod;
> uint8_t fdcycod;
> uint8_t sgaincod;
> uint8_t dbpbcod;
> uint8_t floorcod;
> uint8_t csnroffst;
> uint8_t cplfsnroffst;
> uint8_t cplfgaincod;
> uint8_t fsnroffst[FBW_CHANNELS];
> uint8_t fgaincod[FBW_CHANNELS];
> uint8_t lfefsnroffst;
> uint8_t lfefgaincod;
> uint8_t cplfleak;
> uint8_t cplsleak;
> uint8_t cpldeltbae;
> uint8_t deltbae[FBW_CHANNELS];
> uint8_t cpldeltnseg;
> uint8_t cpldeltoffst[DBA_SEG_MAX];
> uint8_t cpldeltlen[DBA_SEG_MAX];
> uint8_t cpldeltba[DBA_SEG_MAX];
> uint8_t deltnseg[FBW_CHANNELS];
> uint8_t deltoffst[FBW_CHANNELS][DBA_SEG_MAX];
> uint8_t deltlen[FBW_CHANNELS][DBA_SEG_MAX];
> uint8_t deltba[FBW_CHANNELS][DBA_SEG_MAX];
>
> /* Derived Attributes. */
> int sampling_rate;
> int bit_rate;
> int frame_size;
>
> int nfchans; // number of channels
> int lfeon; // lfe channel in use
>
> float dynrng; // dynamic range gain
> float dynrng2; // dynamic range gain for 1+1 mode
> float chcoeffs[AC3_MAX_CHANNELS]; // normalized channel coefficients
> float cplco[FBW_CHANNELS][18]; // coupling coordinates
> int ncplbnd; // number of coupling bands
> int ncplsubnd; // number of coupling sub bands
> int cplstrtmant; // coupling start mantissa
> int cplendmant; // coupling end mantissa
> int endmant[FBW_CHANNELS]; // channel end mantissas
are all these wierd names from the ac3 spec? if no then id strongly vote
for more readable names cplstrtmant -> coupling_start_mantisse for example
if they are in the ac3 spec then iam undecided if these names should be keept
>
> uint8_t dcplexps[AC3_MAX_COEFS]; // decoded coupling exponents
> uint8_t dexps[FBW_CHANNELS][AC3_MAX_COEFS]; // decoded fbw channel exponents
> uint8_t dlfeexps[AC3_MAX_COEFS]; // decoded lfe channel exponents
> uint8_t cplbap[AC3_MAX_COEFS]; // coupling bit allocation pointers
> uint8_t bap[FBW_CHANNELS][AC3_MAX_COEFS]; // fbw channel bit allocation pointers
> uint8_t lfebap[AC3_MAX_COEFS]; // lfe channel bit allocation pointers
the comments are all not doxygen compatible (///<)
[...]
> static int16_t dither_int16(AVRandomState *state)
> {
> uint32_t y = av_random(state);
> return (int16_t)((y << 16) >> 16);
> }
both high and low 16bit should be used due to speed reasons ...
[...]
>
> /*
> * Generate quantizer tables.
> */
> static void generate_quantizers_table(int16_t quantizers[], int level, int length)
> {
> int i;
>
> for (i = 0; i < length; i++)
> quantizers[i] = ((2 * i - level + 1) << 15) / level;
> }
>
> static void generate_quantizers_table_1(int16_t quantizers[], int level, int length1, int length2, int size)
> {
> int i, j;
> int16_t v;
>
> for (i = 0; i < length1; i++) {
> v = ((2 * i - level + 1) << 15) / level;
> for (j = 0; j < length2; j++)
> quantizers[i * length2 + j] = v;
> }
>
> for (i = length1 * length2; i < size; i++)
> quantizers[i] = 0;
> }
>
> static void generate_quantizers_table_2(int16_t quantizers[], int level, int length1, int length2, int size)
> {
> int i, j;
> int16_t v;
>
> for (i = 0; i < length1; i++) {
> v = ((2 * (i % level) - level + 1) << 15) / level;
> for (j = 0; j < length2; j++)
> quantizers[i * length2 + j] = v;
> }
>
> for (i = length1 * length2; i < size; i++)
> quantizers[i] = 0;
>
> }
>
> static void generate_quantizers_table_3(int16_t quantizers[], int level, int length1, int length2, int size)
> {
> int i, j;
>
> for (i = 0; i < length1; i++)
> for (j = 0; j < length2; j++)
> quantizers[i * length2 + j] = ((2 * (j % level) - level + 1) << 15) / level;
>
> for (i = length1 * length2; i < size; i++)
> quantizers[i] = 0;
> }
replace the quadruplicated shit above by
generate_quantizers_table(int16_t quantizers[], int level, int length1, int length2, int size, int im, int jm){
int i, j;
for (i = 0; i < length1; i++)
for (j = 0; j < length2; j++)
quantizers[i * length2 + j] = ((2 * (j % jm + i % im) - level + 1) << 15) / level;
}
also note the = 0 is useless as static&global arrays are initalized to 0
[...]
> /* Parse the 'sync_info' from the ac3 bitstream.
> * This function extracts the sync_info from ac3 bitstream.
> * GetBitContext within AC3DecodeContext must point to
> * start of the synchronized ac3 bitstream.
> *
> * @param ctx AC3DecodeContext
> * @return Returns framesize, returns 0 if fscod, frmsizecod or bsid is not valid
almost all code in lav* returns negative values on failure id suggest that
ac3 does too
doxygen comments start with /** or various other forms but not "/* "
> static int ac3_parse_sync_info(AC3DecodeContext *ctx)
> {
> GetBitContext *gb = &ctx->gb;
> int frmsizecod, bsid;
>
> skip_bits(gb, 16); //skip the sync_word, sync_info->sync_word = get_bits(gb, 16);
> ctx->crc1 = get_bits(gb, 16);
> ctx->fscod = get_bits(gb, 2);
> if (ctx->fscod == 0x03)
> return 0;
> frmsizecod = get_bits(gb, 6);
> if (frmsizecod >= 38)
> return 0;
>
> /* we include it here in order to determine validity of ac3 frame */
> bsid = get_bits(gb, 5);
> if (bsid > 10)
> return 0;
> skip_bits(gb, 3); //skip the bsmod, bsi->bsmod = get_bits(gb, 3);
>
> ctx->halfratecod = FFMAX(0, bsid - 8);
> ctx->sampling_rate = ac3_freqs[ctx->fscod] >> ctx->halfratecod;
> ctx->bit_rate = ac3_bitratetab[frmsizecod >> 1];
>
> ctx->frame_size = 0;
> switch (ctx->fscod) {
> case 0x00:
> ctx->frame_size = 4 * ctx->bit_rate;
> break;
> case 0x01:
> ctx->frame_size = 2 * (320 * ctx->bit_rate / 147 + (frmsizecod & 1));
> break;
> case 0x02:
> ctx->frame_size = 6 * ctx->bit_rate;
> break;
> }
>
> ctx->bit_rate >>= ctx->halfratecod;
>
> return ctx->frame_size;
> }
this looks a little bit similar to what is in the avparser, if so it is
code duplication which is not ok (put the common code in a common function)
[...]
> /* Decodes the grouped exponents.
> * This function decodes the coded exponents according to exponent strategy
> * and stores them in the decoded exponents buffer.
> *
> * @param gb GetBitContext which points to start of coded exponents
> * @param expstr Exponent coding strategy
> * @param ngrps Number of grouped exponetns
> * @param absexp Absolute exponent
> * @param dexps Decoded exponents are stored in dexps
> */
> static void decode_exponents(GetBitContext *gb, int expstr, int ngrps,
> uint8_t absexp, uint8_t *dexps)
> {
> int i, j, grp, grpsize;
> int dexp[256];
> int aexp[256];
> int expacc, prevexp;
>
> // unpack groups
> grpsize = expstr;
> if(expstr == EXP_D45) grpsize = 4;
> for(grp=0; grp<ngrps; grp++) {
> expacc = get_bits(gb, 7);
> dexp[grp*3] = expacc / 25;
> expacc -= 25 * dexp[grp*3];
> dexp[(grp*3)+1] = expacc / 5;
> expacc -= 5 * dexp[(grp*3)+1];
> dexp[(grp*3)+2] = expacc;
what about using a table which store the /25 (%25)/5 %5 values? should be
faster ...
> }
>
> // convert to absolute exps
> prevexp = absexp;
> for(i=0; i<ngrps*3; i++) {
> aexp[i] = prevexp + (dexp[i]-2);
> aexp[i] = FFMIN(FFMAX(aexp[i], 0), 24);
clip()
> prevexp = aexp[i];
also id
prevexp += dexp[i]-2;
prevexp= clip(prevexp, ...)
aexp[i]= prevexp;
somehow that looks cleaner to me ...
[...]
> #define TRANSFORM_COEFF(tc, m, e, f) (tc) = (m) * (f)[(e)]
i hate macros which hide trivial arithmetic operations, please replace
all TRANSFORM_COEFF by what they acctually do, and feel free to add a comment
to all like // transforms coeff
>
> /* Get the transform coefficients for coupling channel and uncouple channels.
> * The coupling transform coefficients starts at the the cplstrtmant, which is
> * equal to endmant[ch] for fbw channels. Hence we can uncouple channels before
> * getting transform coefficients for the channel.
> */
> static int get_transform_coeffs_cpling(AC3DecodeContext *ctx, mant_groups *m)
> {
> GetBitContext *gb = &ctx->gb;
> int ch, start, end, cplbndstrc, bnd, gcode, tbap;
> float cplcos[5], cplcoeff;
> uint8_t *exps = ctx->dcplexps;
> uint8_t *bap = ctx->cplbap;
>
> cplbndstrc = ctx->cplbndstrc;
> start = ctx->cplstrtmant;
> bnd = 0;
>
> while (start < ctx->cplendmant) {
> end = start + 12;
> while (cplbndstrc & 1) {
> end += 12;
> cplbndstrc >>= 1;
> }
> cplbndstrc >>= 1;
> for (ch = 0; ch < ctx->nfchans; ch++)
> cplcos[ch] = ctx->chcoeffs[ch] * ctx->cplco[ch][bnd];
> bnd++;
>
> while (start < end) {
> tbap = bap[start];
> switch(tbap) {
> case 0:
> for (ch = 0; ch < ctx->nfchans; ch++)
> if (((ctx->chincpl) >> ch) & 1) {
> if ((ctx->dithflag >> ch) & 1) {
> TRANSFORM_COEFF(cplcoeff, dither_int16(&ctx->dith_state), exps[start], scale_factors);
> ctx->transform_coeffs[ch + 1][start] = cplcoeff * cplcos[ch] * LEVEL_MINUS_3DB;
> } else
> ctx->transform_coeffs[ch + 1][start] = 0;
> }
> start++;
> continue;
> case 1:
> if (m->l3ptr > 2) {
> gcode = get_bits(gb, 5);
> m->l3_quantizers[0] = l3_quantizers_1[gcode];
> m->l3_quantizers[1] = l3_quantizers_2[gcode];
> m->l3_quantizers[2] = l3_quantizers_3[gcode];
> m->l3ptr = 0;
> }
> TRANSFORM_COEFF(cplcoeff, m->l3_quantizers[m->l3ptr++], exps[start], scale_factors);
> break;
>
> case 2:
> if (m->l5ptr > 2) {
> gcode = get_bits(gb, 7);
> m->l5_quantizers[0] = l5_quantizers_1[gcode];
> m->l5_quantizers[1] = l5_quantizers_2[gcode];
> m->l5_quantizers[2] = l5_quantizers_3[gcode];
> m->l5ptr = 0;
> }
> TRANSFORM_COEFF(cplcoeff, m->l5_quantizers[m->l5ptr++], exps[start], scale_factors);
> break;
>
> case 3:
> TRANSFORM_COEFF(cplcoeff, l7_quantizers[get_bits(gb, 3)], exps[start], scale_factors);
> break;
>
> case 4:
> if (m->l11ptr > 1) {
> gcode = get_bits(gb, 7);
> m->l11_quantizers[0] = l11_quantizers_1[gcode];
> m->l11_quantizers[1] = l11_quantizers_2[gcode];
> m->l11ptr = 0;
> }
> TRANSFORM_COEFF(cplcoeff, m->l11_quantizers[m->l11ptr++], exps[start], scale_factors);
> break;
>
> case 5:
> TRANSFORM_COEFF(cplcoeff, l15_quantizers[get_bits(gb, 4)], exps[start], scale_factors);
> break;
>
> default:
> TRANSFORM_COEFF(cplcoeff, get_sbits(gb, qntztab[tbap]) << (16 - qntztab[tbap]),
> exps[start], scale_factors);
> }
all the TRANSFORM_COEFF are equal excpet the second argument so this could be simplified like
...
case 3:
coeff= l7_quantizers[get_bits(gb, 3)];
break;
case 4:
if (m->l11ptr > 1) {
gcode = get_bits(gb, 7);
m->l11_quantizers[0] = l11_quantizers_1[gcode];
m->l11_quantizers[1] = l11_quantizers_2[gcode];
m->l11ptr = 0;
}
coeff= m->l11_quantizers[m->l11ptr++];
break;
case 5:
coeff= l15_quantizers[get_bits(gb, 4)];
break;
}
TRANSFORM_COEFF(cplcoeff, coeff, exps[start], scale_factors);
> for (ch = 0; ch < ctx->nfchans; ch++)
> if ((ctx->chincpl >> ch) & 1)
maybe chincpl and dithflag should be changed to normal arrays? or maybe
not? dunno
[...]
> if (ch_index != -1) { /* fbw channels */
> dithflag = (ctx->dithflag >> ch_index) & 1;
> exps = ctx->dexps[ch_index];
> bap = ctx->bap[ch_index];
> coeffs = ctx->transform_coeffs[ch_index + 1];
> end = ctx->endmant[ch_index];
> } else if (ch_index == -1) {
that else if is silly it must be true if the first is false
> dithflag = 0;
> exps = ctx->dlfeexps;
> bap = ctx->lfebap;
> coeffs = ctx->transform_coeffs[0];
> end = 7;
> }
>
>
> for (i = 0; i < end; i++) {
> tbap = bap[i];
> switch (tbap) {
> case 0:
> if (!dithflag) {
> coeffs[i] = 0;
> continue;
> }
> else {
> TRANSFORM_COEFF(coeffs[i], dither_int16(&ctx->dith_state), exps[i], factors);
> coeffs[i] *= LEVEL_MINUS_3DB;
> continue;
> }
>
> case 1:
> if (m->l3ptr > 2) {
> gcode = get_bits(gb, 5);
> m->l3_quantizers[0] = l3_quantizers_1[gcode];
> m->l3_quantizers[1] = l3_quantizers_2[gcode];
> m->l3_quantizers[2] = l3_quantizers_3[gcode];
> m->l3ptr = 0;
> }
> TRANSFORM_COEFF(coeffs[i], m->l3_quantizers[m->l3ptr++], exps[i], factors);
> continue;
>
> case 2:
> if (m->l5ptr > 2) {
> gcode = get_bits(gb, 7);
> m->l5_quantizers[0] = l5_quantizers_1[gcode];
> m->l5_quantizers[1] = l5_quantizers_2[gcode];
> m->l5_quantizers[2] = l5_quantizers_3[gcode];
> m->l5ptr = 0;
> }
> TRANSFORM_COEFF(coeffs[i], m->l5_quantizers[m->l5ptr++], exps[i], factors);
> continue;
>
> case 3:
> TRANSFORM_COEFF(coeffs[i], l7_quantizers[get_bits(gb, 3)], exps[i], factors);
> continue;
>
> case 4:
> if (m->l11ptr > 1) {
> gcode = get_bits(gb, 7);
> m->l11_quantizers[0] = l11_quantizers_1[gcode];
> m->l11_quantizers[1] = l11_quantizers_2[gcode];
> m->l11ptr = 0;
> }
> TRANSFORM_COEFF(coeffs[i], m->l11_quantizers[m->l11ptr++], exps[i], factors);
> continue;
>
> case 5:
> TRANSFORM_COEFF(coeffs[i], l15_quantizers[get_bits(gb, 4)], exps[i], factors);
> continue;
>
> default:
> TRANSFORM_COEFF(coeffs[i], get_sbits(gb, qntztab[tbap]) << (16 - qntztab[tbap]), exps[i], factors);
> continue;
> }
> }
>
> return 0;
> }
duplicate code (see get_transform_coeffs_cpling)
[...]
> if (get_transform_coeffs_ch(ctx, -1, &m))
> return -1;
indention looks slightly wrong
[...]
> case AC3_INPUT_3F:
> switch (to) {
> case AC3_OUTPUT_MONO:
> nf = LEVEL_MINUS_3DB / (1.0 + clev);
> ctx->chcoeffs[0] *= (nf * LEVEL_MINUS_3DB);
> ctx->chcoeffs[2] *= (nf * LEVEL_MINUS_3DB);
> ctx->chcoeffs[1] *= ((nf * clev * LEVEL_MINUS_3DB) / 2.0);
nf = 0.5 / (1.0 + clev);
ctx->chcoeffs[0] *= nf;
ctx->chcoeffs[2] *= nf;
ctx->chcoeffs[1] *= nf * clev * 0.5;
[...]
> case AC3_INPUT_2F_1R:
> switch (to) {
> case AC3_OUTPUT_MONO:
> nf = 2.0 * LEVEL_MINUS_3DB / (2.0 + slev);
> ctx->chcoeffs[0] *= (nf * LEVEL_MINUS_3DB);
> ctx->chcoeffs[1] *= (nf * LEVEL_MINUS_3DB);
> ctx->chcoeffs[2] *= (nf * slev * LEVEL_MINUS_3DB);
nf = 1.0 / (2.0 + slev);
ctx->chcoeffs[0] *= nf;
ctx->chcoeffs[1] *= nf;
ctx->chcoeffs[2] *= nf * slev;
(LEVEL_MINUS_3DB * LEVEL_MINUS_3DB = 0.5)
[...]
> /*********** BEGIN DOWNMIX FUNCTIONS ***********/
id put these in their own file
> static inline void mix_dualmono_to_mono(AC3DecodeContext *ctx)
> {
> int i;
> float (*output)[AC3_BLOCK_SIZE] = ctx->output;
all these functions seem to just use ctx->output so that should be passed
as argument instead of AC3DecodeContext
[...]
> static inline void upmix_mono_to_stereo(AC3DecodeContext *ctx)
> {
> int i;
> float (*output)[AC3_BLOCK_SIZE] = ctx->output;
>
> for (i = 0; i < 256; i++)
> output[2][i] = output[1][i];
memcpy()
> }
>
> static inline void mix_stereo_to_mono(AC3DecodeContext *ctx)
> {
> int i;
> float (*output)[AC3_BLOCK_SIZE] = ctx->output;
>
> for (i = 0; i < 256; i++)
> output[1][i] += output[2][i];
> memset(output[2], 0, sizeof(output[2]));
> }
duplicate of mix_dualmono_to_mono()
[...]
> static inline void mix_2f_1r_to_mono(AC3DecodeContext *ctx)
> {
> int i;
> float (*output)[AC3_BLOCK_SIZE] = ctx->output;
>
> for (i = 0; i < 256; i++)
> output[1][i] += (output[2][i] + output[3][i]);
> memset(output[2], 0, sizeof(output[2]));
> memset(output[3], 0, sizeof(output[3]));
duplicate of mix_3f_to_mono
>
> }
>
> static inline void mix_2f_1r_to_stereo(AC3DecodeContext *ctx)
> {
> int i;
> float (*output)[AC3_BLOCK_SIZE] = ctx->output;
>
> for (i = 0; i < 256; i++) {
> output[1][i] += output[2][i];
> output[2][i] += output[3][i];
> }
> memset(output[3], 0, sizeof(output[3]));
duplicate of mix_3f_to_stereo
[...]
> static inline void mix_2f_2r_to_mono(AC3DecodeContext *ctx)
> {
> int i;
> float (*output)[AC3_BLOCK_SIZE] = ctx->output;
>
> for (i = 0; i < 256; i++)
> output[1][i] = (output[2][i] + output[3][i] + output[4][i]);
> memset(output[2], 0, sizeof(output[2]));
> memset(output[3], 0, sizeof(output[3]));
> memset(output[4], 0, sizeof(output[4]));
duplictae of mix_3f_1r_to_mono
[...]
> for (k = 0; k < N / 8; k++)
> {
> o_ptr[2 * k] = -ptr1[k].im * w[2 * k] + d_ptr[2 * k] + 384.0;
> o_ptr[2 * k + 1] = ptr1[N / 8 - k - 1].re * w[2 * k + 1] + 384.0;
> o_ptr[N / 4 + 2 * k] = -ptr1[k].re * w[N / 4 + 2 * k] + d_ptr[N / 4 + 2 * k] + 384.0;
> o_ptr[N / 4 + 2 * k + 1] = ptr1[N / 8 - k - 1].im * w[N / 4 + 2 * k + 1] + d_ptr[N / 4 + 2 * k + 1] + 384.0;
> d_ptr[2 * k] = ptr2[k].re * w[k / 2 - 2 * k - 1];
> d_ptr[2 * k + 1] = -ptr2[N / 8 - k - 1].im * w[N / 2 - 2 * k - 2];
> d_ptr[N / 4 + 2 * k] = ptr2[k].im * w[N / 4 - 2 * k - 1];
> d_ptr[N / 4 + 2 * k + 1] = -ptr2[N / 8 - k - 1].re * w[N / 4 - 2 * k - 2];
> }
what a mess ...
[...]
> if (get_bits1(gb)) { /* dynamic range */
> dynrng = get_sbits(gb, 8);
> ctx->dynrng = ((((dynrng & 0x1f) | 0x20) << 13) * scale_factors[3 - (dynrng >> 5)]);
> }
>
> if (acmod == 0x00 && get_bits1(gb)) { /* dynamic range 1+1 mode */
> dynrng = get_sbits(gb, 8);
> ctx->dynrng2 = ((((dynrng & 0x1f) | 0x20) << 13) * scale_factors[3 - (dynrng >> 5)]);
> }
dynrng/dynrng2 -> dynrng[] and
for(i=0; i<= !acmod; i++){
if(get_bits1(gb)){
a= get_sbits(gb, 3);
b= get_bits(gb, 5);
ctx->dynrng[i]= ((b | 0x20) << 13) * scale_factors[3 - a];
}
}
[...]
> if (!(ctx->cplinu) || ctx->cplbegf > 2)
> for (rbnd = 0; rbnd < 4; rbnd++)
> ctx->rematflg |= get_bits1(gb) << rbnd;
> if (ctx->cplbegf > 0 && ctx->cplbegf <= 2 && ctx->cplinu)
> for (rbnd = 0; rbnd < 3; rbnd++)
> ctx->rematflg |= get_bits1(gb) << rbnd;
> if (ctx->cplbegf == 0 && ctx->cplinu)
> for (rbnd = 0; rbnd < 2; rbnd++)
> ctx->rematflg |= get_bits1(gb) << rbnd;
rbnd_len= 4;
if(ctx->cplinu)
rbnd_len-= (ctx->cplbegf==0) + (ctx->cplbegf<=2);
for (rbnd = 0; rbnd < rbnd_len; rbnd++)
ctx->rematflg |= get_bits1(gb) << rbnd;
[...]
> av_log(NULL, AV_LOG_ERROR, "coupling delta bit allocation strategy reserved\n");
av_log(NULL -> av_log(avctx
yes all av_log not just this one
[...]
> if (ctx->cplinu)
> if (ctx->cpldeltbae == DBA_NEW) { /*coupling delta offset, len and bit allocation */
> ctx->cpldeltnseg = get_bits(gb, 3);
> for (seg = 0; seg <= ctx->cpldeltnseg; seg++) {
> ctx->cpldeltoffst[seg] = get_bits(gb, 5);
> ctx->cpldeltlen[seg] = get_bits(gb, 4);
> ctx->cpldeltba[seg] = get_bits(gb, 3);
> }
> }
>
> for (i = 0; i < nfchans; i++)
> if (ctx->deltbae[i] == DBA_NEW) {/*channel delta offset, len and bit allocation */
> ctx->deltnseg[i] = get_bits(gb, 3);
> for (seg = 0; seg <= ctx->deltnseg[i]; seg++) {
> ctx->deltoffst[i][seg] = get_bits(gb, 5);
> ctx->deltlen[i][seg] = get_bits(gb, 4);
> ctx->deltba[i][seg] = get_bits(gb, 3);
> }
> }
hmm the 2 loops above could be merged if cpldelt* where in the same arrays as
delt*[]
[...]
> static inline int16_t convert(int32_t i)
> {
> if (i > 0x43c07fff)
> return 32767;
> else if (i <= 0x43bf8000)
> return -32768;
> else
> return (i - 0x43c00000);
> }
duplicate see a52dec/ac3dec.c
>
> //static int frame_count = 0;
hmm, forgotten debug code?
>
> /* Decode ac3 frame.
> *
> * @param avctx Pointer to AVCodecContext
> * @param data Pointer to pcm smaples
> * @param data_size Set to number of pcm samples produced by decoding
> * @param buf Data to be decoded
> * @param buf_size Size of the buffer
> */
> static int ac3_decode_frame(AVCodecContext * avctx, void *data, int *data_size, uint8_t *buf, int buf_size)
> {
> AC3DecodeContext *ctx = (AC3DecodeContext *)avctx->priv_data;
> int frame_start;
> int16_t *out_samples = (int16_t *)data;
> int i, j, k, start;
> int32_t *int_ptr[6];
>
> for (i = 0; i < 6; i++)
> int_ptr[i] = (int32_t *)(&ctx->output[i]);
>
> //av_log(NULL, AV_LOG_INFO, "decoding frame %d buf_size = %d\n", frame_count++, buf_size);
>
> //Synchronize the frame.
> frame_start = ac3_synchronize(buf, buf_size);
as the ac3 bitstream, should have been sent through a parser there should be
no need to search for any synccode, either its there or theres an error
[...]
> Index: libavcodec/ac3tab.h
> ===================================================================
> --- libavcodec/ac3tab.h (revision 7906)
> +++ libavcodec/ac3tab.h (working copy)
> @@ -25,10 +25,10 @@
> */
>
> /* possible frequencies */
> -static const uint16_t ac3_freqs[3] = { 48000, 44100, 32000 };
> +const uint16_t ac3_freqs[3] = { 48000, 44100, 32000 };
>
> /* possible bitrates */
> -static const uint16_t ac3_bitratetab[19] = {
> +const uint16_t ac3_bitratetab[19] = {
> 32, 40, 48, 56, 64, 80, 96, 112, 128,
> 160, 192, 224, 256, 320, 384, 448, 512, 576, 640
> };
> @@ -36,7 +36,7 @@
> /* AC3 MDCT window */
>
> /* MDCT window */
> -static const int16_t ac3_window[256] = {
> +const int16_t ac3_window[256] = {
> 4, 7, 12, 16, 21, 28, 34, 42,
> 51, 61, 72, 84, 97, 111, 127, 145,
> 164, 184, 207, 231, 257, 285, 315, 347,
> @@ -165,41 +165,34 @@
> 15, 15, 15, 15,
> };
global tables need a ff_ prefix to avoid name clashed
[...]
> +/* delta bit allocation strategy */
> +#define DBA_NEW 0x01
> +#define DBA_NONE 0x02
> +#define DBA_RESERVED 0x03
> +#define DBA_REUSE 0x00
could be changed to an enum
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070210/36df6333/attachment.pgp>
More information about the ffmpeg-devel
mailing list