[FFmpeg-devel] [PATCH] NellyMoser audio decoder v2
Michael Niedermayer
michaelni
Tue Oct 2 00:48:29 CEST 2007
Hi
On Mon, Oct 01, 2007 at 02:25:59PM +0200, Benjamin Larsson wrote:
> Benjamin Larsson wrote:
>> [...]
>> Hi, I had some time yesterday and looked closer at the code. While there
>> is no doubt that the code is a TDAC implementation it doesn't seem like
>> the code can be replaced by the mdct code in ffmpeg. So I guess that the
>> fft replacement is what can be clean up for now. I'll sort out the table
>> generation for the rotations and the window.
if its not IMDCT then what is it?
[...]
> +static const int ff_band_sizes_table[23] = {
> +2, 2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 4, 4, 5, 6, 6, 7, 8, 9, 10, 12, 14, 15
> +};
fits in uint8_t
> +
> +static const float ff_nelly_signal_table[64] = {
> +0.1250000000, 0.1249623969, 0.1248494014, 0.1246612966, 0.1243980974,
> +0.1240599006, 0.1236471012, 0.1231596991, 0.1225982010, 0.1219628006,
> +0.1212539002, 0.1204719990, 0.1196174994, 0.1186909974, 0.1176929995,
> +0.1166241020, 0.1154849008, 0.1142762005, 0.1129987016, 0.1116530001,
> +0.1102401987, 0.1087609008, 0.1072160974, 0.1056066975, 0.1039336994,
> +0.1021981016, 0.1004009023, 0.0985433012, 0.0966262966, 0.0946511030,
> +0.0926188976, 0.0905309021, 0.0883883014, 0.0861926004, 0.0839449018,
> +0.0816465989, 0.0792991966, 0.0769039020, 0.0744623989, 0.0719759986,
> +0.0694463030, 0.0668746978, 0.0642627999, 0.0616123006, 0.0589246005,
> +0.0562013984, 0.0534444004, 0.0506552011, 0.0478353985, 0.0449868999,
> +0.0421111993, 0.0392102003, 0.0362856016, 0.0333391018, 0.0303725004,
> +0.0273876991, 0.0243862998, 0.0213702004, 0.0183412991, 0.0153013002,
> +0.0122520998, 0.0091955997, 0.0061335000, 0.0030677000
> +};
this is cos(i/128.0*M_PI)/8
which should be mentioned in a comment at least, also the name should be
changed to reflect what it is
[...]
> +static void antialias(float *buf, float *audio)
> +{
> + int i, end, mid_hi, mid_lo;
> + float a, b, c, d, e, f;
> +
> + end = NELLY_BUF_LEN-1;
> + mid_hi = NELLY_BUF_LEN/2;
> + mid_lo = mid_hi-1;
> +
> + for (i = 0; i < NELLY_BUF_LEN/4; i++) {
> + a = buf[end-2*i];
> + b = buf[2*i];
> + c = buf[2*i+1];
> + d = buf[end-2*i-1];
> +
> + e = tcos[i];
> + f = tsin[i];
> +
> + audio[2*i] = b*e-a*f;
> + audio[2*i+1] = -(a*e+b*f);
> +
> + a = tsin[mid_lo-i];
> + b = tcos[mid_lo-i];
> +
> + audio[end-2*i-1] = b*d-a*c;
> + audio[end-2*i] = -(b*c+a*d);
> + }
i think it would be more readable if instead of efab
the tcos/tsin would be stored in variables with the names s and c or
wr and wi maybe?
[...]
> + audio[bot] = audio[mid_up]*t[bot]+state[bot]*t[top];
> + audio[top] = state[bot]*t[bot]-audio[mid_up]*t[top];
> + state[bot] = -audio[mid_down];
> +
> + audio[mid_down] = s_top*t[mid_down]+state[mid_down]*t[mid_up];
> + audio[mid_up] = state[mid_down]*t[mid_down]-s_top*t[mid_up];
> + state[mid_down] = -s_bot;
audio[bot] = audio[mid_up]*t[bot]+state[bot ]*t[top];
audio[top] = state[bot ]*t[bot]-audio[mid_up]*t[top];
state[bot] = -audio[mid_down];
audio[mid_down] = s_top *t[mid_down]+state[mid_down]*t[mid_up];
audio[mid_up ] = state[mid_down]*t[mid_down]-s_top *t[mid_up];
state[mid_down] = -s_bot;
[...]
> + bitsum = sum_bits(sbuf, shift_saved, off);
> +
> + if (bitsum != NELLY_DETAIL_BITS) {
> + shift = 0;
> + diff = bitsum - NELLY_DETAIL_BITS;
> +
> + while (FFABS(diff) <= 16383) {
> + shift++;
> + diff *= 2;
> + }
> +
> + diff = (diff * NELLY_BASE_OFF) >> 15;
> + shift = shift_saved-(NELLY_BASE_SHIFT+shift-15);
> +
> + diff = signed_shift(diff, shift);
> +
> + for (j = 1; j < 20; j++) {
> + tmp = off;
> + off += diff;
> + last_bitsum = bitsum;
> +
> + bitsum = sum_bits(sbuf, shift_saved, off);
> +
> + if ((bitsum-NELLY_DETAIL_BITS) * (last_bitsum-NELLY_DETAIL_BITS) <= 0)
> + break;
> + }
> +
> + if (bitsum != NELLY_DETAIL_BITS) {
> + if (bitsum > NELLY_DETAIL_BITS) {
> + big_off = off;
> + off = tmp;
> + big_bitsum=bitsum;
> + small_bitsum=last_bitsum;
> + } else {
> + big_off = tmp;
> + big_bitsum=last_bitsum;
> + small_bitsum=bitsum;
> + }
> +
> + while (bitsum != NELLY_DETAIL_BITS && j <= 19) {
> + diff = (big_off+off)>>1;
> + bitsum = sum_bits(sbuf, shift_saved, diff);
> + if (bitsum > NELLY_DETAIL_BITS) {
> + big_off=diff;
> + big_bitsum=bitsum;
> + } else {
> + off = diff;
> + small_bitsum=bitsum;
> + }
> + j++;
> + }
this is a binary search and i think the variables could be named slightly
better and the whole can be simplified alot unless its buggy (=not a
working binary search)
and maybe it should be moved into its own function
> +
> + if (abs(big_bitsum-NELLY_DETAIL_BITS) >=
> + abs(small_bitsum-NELLY_DETAIL_BITS)) {
are the abs needed? isnt the sign of these expresions always the same?
[...]
> + bitsum = NELLY_DETAIL_BITS;
> + while (i < NELLY_FILL_LEN) {
> + bits[i] = 0;
> + i++;
> + }
> + }
the bitsum= here makes no sense, its not used
[...]
> + antialias(buf, aptr);
> + ff_fft_permute(&fftc, (FFTComplex*)aptr);
> + ff_fft_calc(&fftc, (FFTComplex*)aptr);
> +
> + for (j = 0; j<64; j++) {
> + ((FFTComplex*)aptr)[j].im = -((FFTComplex*)aptr)[j].im;
> + }
> + complex2signal(aptr);
indention looks odd
[...]
> +void nelly_util_floats2shorts(float audio[256], short shorts[256])
> +{
> + int i;
> +
> + for (i = 0; i < 256; i++) {
> + if (audio[i] >= 32767.0)
> + shorts[i] = 32767;
> + else if (audio[i] <= -32768.0)
> + shorts[i] = -32768;
> + else
> + shorts[i] = (short)audio[i];
> + }
> +}
shouldnt this be using the optimized dsp code?
[...]
> + while (remaining_size >= NELLY_BLOCK_LEN) {
> + nelly_decode_block(s, buf, s->float_buf);
> + nelly_util_floats2shorts(s->float_buf, data);
> + data += NELLY_SAMPLE_SIZE;
> + *data_size += NELLY_SAMPLE_SIZE;
> + buf += NELLY_BLOCK_LEN;
> + remaining_size -= NELLY_BLOCK_LEN;
> + }
this looks like theres a AVParser missing to split it into packets
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20071002/bd3e3e6d/attachment.pgp>
More information about the ffmpeg-devel
mailing list