[FFmpeg-devel] [PATCH 2/2] Add SIPR decoder for 5k0, 6k5 and 8k5 modes
Michael Niedermayer
michaelni
Sun Dec 20 03:53:02 CET 2009
On Sat, Dec 19, 2009 at 03:17:37PM +0100, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> On Fri, Dec 18, 2009 at 06:07:45PM +0100, Vitor Sessak wrote:
>>> Diego Biurrun wrote:
>>>> On Fri, Dec 18, 2009 at 05:52:13PM +0100, Vitor Sessak wrote:
>
> [...]
>
>>> Gave a try to a different formatting.
>>>
>>> Here is a new patch with these and a few other minor cosmetics fixed.
>> [...]
>>> +typedef struct {
>>> + const char *mode_name;
>>> + uint16_t bits_per_frame;
>>> + uint8_t subframe_size;
>> constant?
>
> Not for 16k mode, but removed anyway.
>
>>> + uint8_t subframe_count;
>>> + uint8_t frames_per_packet;
>>> + float treshpit;
>> treshpit ?
>
> Indeed, bad name. Renamed.
>
>>> +
>>> + /* bitstream parameters */
>>> + uint8_t number_of_fc_indexes;
>>> +
>>> + /** size in bits of the i-th stage vector of quantizer */
>>> + uint8_t vq_indexes_bits[5];
>>> +
>>> + /** size in bits of the adaptive-codebook index for every subframe
>>> */
>>> + uint8_t pitch_delay_bits[5];
>>> +
>>> + uint8_t gp_index_bits;
>>> + uint8_t fc_index_bits[10]; ///< size in bits of the fixed codebook
>>> indexes
>>> + uint8_t gc_index_bits; ///< size in bits of the gain codebook
>>> indexes
>> always 7 ?
>
> Not for 16k.
>
>> [...]
>>> +static void dequant(float *out, const int *idx, const float *cbs[])
>>> +{
>>> + int i;
>>> +
>>> + int stride = 2;
>>> + int num_vec = 5;
>>> +
>>> + for (i = 0; i < num_vec; i++)
>>> + memcpy(out + stride*i, cbs[i] + stride*idx[i],
>>> stride*sizeof(float));
>> you dont need local vars for constants
>
> They differ for 16k and this way of writing it minimize the patch to add
> it. I agree that normally this is not ok, but AFAIK only this, the previous
> change and fc_index_bits been constant are the only things affected by it,
> so I think it is worth it.
ok
>
>> [...]
>>> +static void sipr_decode_lp(float *lsfnew, const float *lsfold, float
>>> *Az,
>>> + int num_subfr, int filter_order)
>>> +{
>>> + float *pAz = Az;
>>> + double lsfint[filter_order];
>>> + int i,j;
>>> + float t, t0 = 1.0 / num_subfr;
>>> +
>>> + t = t0/2;
>> *0.5 divide is slow and gcc not trustworthy
>> filter_order seems constant so why pass it into the func?
>
> Agreed and fixed both (and elsewhere too).
>
>>> + for (i = 0; i < num_subfr; i++) {
>>> + for (j = 0; j < filter_order; j++)
>>> + lsfint[j] = lsfold[j] * (1 - t) + t * lsfnew[j];
>>> +
>>> + lsp2lpc_sipr(lsfint, pAz, filter_order);
>>> + pAz += filter_order;
>>> + t += t0;
>>> + }
>>> +}
>>> +
>>> +/**
>>> + * Evaluate the adaptative impulse response
>>> + */
>>> +static void eval_ir(const float *Az, int pitch_lag, float *freq,
>>> + float treshpit, int length)
>>> +{
>>> + float tmp1[L_SUBFR_SIPR+1], tmp2[LP_FILTER_ORDER+1];
>>> + int i;
>>> +
>>> + tmp1[0] = 1.;
>>> + for (i = 0; i < LP_FILTER_ORDER; i++) {
>>> + tmp1[i+1] = Az[i] * ff_pow_0_55[i];
>>> + tmp2[i ] = Az[i] * ff_pow_0_7 [i];
>>> + }
>>> + memset(tmp1 + 11, 0, 37*sizeof(float));
>>> +
>>> + ff_celp_lp_synthesis_filterf(freq, tmp2, tmp1, L_SUBFR_SIPR,
>>> + LP_FILTER_ORDER);
>>> +
>>> + pitch_sharpening(length, pitch_lag, treshpit, freq);
>> [...]
>>> + for (i = 0; i < LP_FILTER_ORDER; i++) {
>>> + lpc_d[i] = lpc[i] * ff_pow_0_75[i];
>>> + lpc_n[i] = lpc[i] * pow_0_5 [i];
>>> + };
>>> +
>>> + memcpy(pole_out - LP_FILTER_ORDER, ctx->postfilter_mem,
>>> + LP_FILTER_ORDER*sizeof(float));
>>> +
>>> + ff_celp_lp_synthesis_filterf(pole_out, lpc_d, samples, L_SUBFR_SIPR,
>>> + LP_FILTER_ORDER);
>> looks somewhat similar, dunno if it can be factorized ...
>
> No good idea from me neither. I hate these similar, but not really easily
> mergeable, chunks...
>
>> [...]
>>> +static float lsf_cb5[32][2] = {
>> const
>> [...]
>>> +static float pred[4] = {
>> const
>
> Done everywhere.
>
>> and i assume you checked that these tables are not duplicates from other
>> similar codecs?
>
> I checked AMR, which was the biggest source of "inspiration" for SIPR. Any
> other ideas?
>
> New patch attached.
[...]
> +static void lsp2lpc_sipr(const double *isp, float *Az)
> +{
> + int lp_half_order = LP_FILTER_ORDER >> 1;
> + double pa[lp_half_order+1], qa[lp_half_order];
> + int i,j;
> +
> + ff_lsp2polyf(isp , pa, lp_half_order );
> + ff_lsp2polyf(isp + 1, qa, lp_half_order - 1);
> +
> + for (i = lp_half_order-1; i > 1; i--)
> + qa[i] -= qa[i-2];
> +
> + for (i = 1, j = LP_FILTER_ORDER - 1; i < lp_half_order; i++, j--) {
> + float paf = (pa[i] + qa[i]) * 0.5;
> + float qaf = (pa[i] - qa[i]) * 0.5;
> +
> + Az[i-1] = paf + qaf * isp[LP_FILTER_ORDER - 1];
> + Az[j-1] = qaf + paf * isp[LP_FILTER_ORDER - 1];
> + }
> +
> + Az[lp_half_order - 1] = (1.0 + isp[LP_FILTER_ORDER - 1]) *
> + pa[lp_half_order] * 0.5;
> +
> + Az[LP_FILTER_ORDER - 1] = isp[LP_FILTER_ORDER - 1];
> +}
have you checked if this can be simplified?
that is do you understand the difference from it to ff_acelp_lspd2lpc() ?
especially what effect the -1 has in ff_lsp2polyf() / that is if the
iteration less isnt just merged into the following code ...
> +
> +static void sipr_decode_lp(float *lsfnew, const float *lsfold, float *Az,
> + int num_subfr)
> +{
> + float *pAz = Az;
> + double lsfint[LP_FILTER_ORDER];
> + int i,j;
> + float t, t0 = 1.0 / num_subfr;
> +
> + t = t0 * 0.5;
> + for (i = 0; i < num_subfr; i++) {
> + for (j = 0; j < LP_FILTER_ORDER; j++)
> + lsfint[j] = lsfold[j] * (1 - t) + t * lsfnew[j];
dont we have some function for this somewhere? (no i dont know its just
i faintly remember something like this but it could have been integer too)
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Thouse who are best at talking, realize last or never when they are 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/20091220/a555c905/attachment.pgp>
More information about the ffmpeg-devel
mailing list