[FFmpeg-devel] [PATCH 2/2] Add SIPR decoder for 5k0, 6k5 and 8k5 modes
Ronald S. Bultje
rsbultje
Mon Jan 4 16:47:25 CET 2010
Hi Vitor,
On Fri, Jan 1, 2010 at 6:52 AM, Vitor Sessak <vitor1001 at gmail.com> wrote:
> Ronald S. Bultje wrote:
>> (Also, why is the last one so low? Shouldn't they be ascending?)
>
> Because in the code it do not take he cos() of the last value, funny
> algorithm they use...
Is it possible to rescale every 9th member of all arrays so that that
multiplication isn't needed anymore? Or is that impossible because
Q-tables are shared between all members of the LSF arrays?
On your new path:
+static void dequant(float *out, const int *idx, const float *cbs[])
+{
+ int i;
+
+ int stride = 2;
+ int num_vec = 5;
This (between i/stride) newline isn't needed, and I'd in fact put all
assignments on the same line, but that's just me.
+static void lsf_decode_fp(float *lsfnew, float *lsf_history,
+ const SiprParameters *parm)
[..]
+ ff_set_min_dist_lsf(lsfnew, LSFQ_DIFF_MIN / 4000., LP_FILTER_ORDER - 1);
That value is only used once, you could just adjust the DIFF_MIN directly.
+ lsfnew[9] = FFMIN(lsfnew[LP_FILTER_ORDER - 1], 1.3);
Aha, so there is a max. value for the last one. I thought so. :-). OK,
so here's my suggestion: let's merge this into the ff_set_min_dist()
function. WMAVoice same, and so do others.
+ for (i = 0; i < LP_FILTER_ORDER - 1; i++)
+ lsfnew[i] = cos(M_PI * lsfnew[i]);
You're still multiplying the sf by M_PI (and yes, my WMAVoice code
does the same, although * 2 M_PI). I still think this can be in the
tables (sorry if my previous comment was unclear). It saves a few
dozen multiplications per frame, should be faster.
(Interesting to note that my WMAVoice has a min_diff of LSFQ_DIFF_MIN
/ 8000, i.e. after the multiplications, our distances are the same
(0.00625 * 2 * M_PI).)
+/**
+ * Extracts decoding parameters from the input bitstream.
+ * @param parms parameters structure
+ * @param pgb pointer to initialized GetbitContext structure
+ */
Get(B)itContext.
+static void lsp2lpc_sipr(const double *isp, float *Az)
isp -> lsp?
[..]
+ ff_lsp2polyf(isp , pa, lp_half_order );
+ ff_lsp2polyf(isp + 1, qa, lp_half_order - 1);
+
+ for (i = 1, j = LP_FILTER_ORDER - 1; i < lp_half_order; i++, j--) {
+ double paf= pa[i] *(1 + isp[LP_FILTER_ORDER - 1]);
+ double qaf= (qa[i] - qa[i-2])*(1 - isp[LP_FILTER_ORDER - 1]);
+ Az[i-1] = (paf + qaf)*0.5;
+ Az[j-1] = (paf - qaf)*0.5;
Spaces around the *?
Weird function (compared to the acelp version). (No comment here, just
noting that it's weird, maybe that's worth a doxy?)
+static void sipr_decode_lp(float *lsfnew, const float *lsfold, float *Az,
+ int num_subfr)
[..]
+ for (j = 0; j < LP_FILTER_ORDER; j++)
+ lsfint[j] = lsfold[j] * (1 - t) + t * lsfnew[j];
Isn't this what ff_weighted_vector_sumf() or ff_acelp_interpolatef()
(didn't check, guessing by name) is for?
+static void eval_ir(const float *Az, int pitch_lag, float *freq,
+ float pitch_sharp_factor)
[..]
+ memset(tmp1 + 11, 0, 37*sizeof(float));
Spaces around the *?
+static void decode_frame(SiprContext *ctx, SiprParameters *params,
+ float *out_data)
[..]
+ float *impulse_response = ir_buf + LP_FILTER_ORDER;
+ float *synth = ctx->synth_buf + 16; // 16 instead of LP_FILTER_ORDER for
+ // memory alignment
+
+ int t0_first = 0;
+ AMRFixed fixed_cb;
Another empty line there - I'd remove it...
+ ff_acelp_interpolatef(excitation, excitation - T0 + (T0_frac <= 0),
+ ff_b60_sinc, 6,
+ 2*((2 + T0_frac)%3 + 1), LP_FILTER_ORDER,
+ SUBFR_SIZE);
+
+
Two empty lines, spaces around operators.
+ ctx->dsp.vector_clipf(out_data, synth, -1, 32765./(1<<15), frame_size);
32765? Not 32767?
+static int sipr_decode_frame(AVCodecContext *avctx, void *datap,
+ int *data_size, AVPacket *avpkt)
[..]
+ if (avpkt->size < (ctx->m.bits_per_frame >> 3)) {
+ av_log(avctx, AV_LOG_ERROR,
+ "Error processing packet: packet size (%d) too small\n",
+ avpkt->size);
+
+ *data_size = 0;
+ return -1;
+ }
I didn't check closely, but just to be sure: is it guaranteed that
this amount of bits is read? can it be higher? You're not using
get_bits_left() anywhere so I'm wondering if there's some way to
overhead (which is bad) the input buffer.
+ if (*data_size < SUBFR_SIZE * ctx->m.subframe_count * 4) {
sizeof(float).
Ronald
More information about the ffmpeg-devel
mailing list