[FFmpeg-devel] [PATCH 2/2] Add SIPR decoder for 5k0, 6k5 and 8k5 modes
Michael Niedermayer
michaelni
Tue Dec 22 01:18:07 CET 2009
On Mon, Dec 21, 2009 at 03:43:02PM +0100, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> 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];
paf= pa[i] *(1+isp[LP_FILTER_ORDER - 1]);
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;
btw what value does isp[LP_FILTER_ORDER - 1] have?
its not 1 or -1 or something trivial ?
and above needs qa[-1]=0
>>> + }
>>> +
>>> + 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 ...
>
>
> I've tried before and just gave a second try. If I change the code to not
> take the -1 (I have to undo the last iteration), I have:
>
>> static void lsp2lpc_sipr(double *isp, float *Az)
>> {
>> int lp_half_order = LP_FILTER_ORDER >> 1;
>> double pa[lp_half_order+1], qa[lp_half_order+1];
>> int i,j;
>> ff_lsp2polyf(isp , pa, lp_half_order);
>> ff_lsp2polyf(isp + 1, qa, lp_half_order);
>> qa[1] += 2 * isp[LP_FILTER_ORDER-1];
>> for(j=2; j < lp_half_order; j++)
>> qa[j] -= - 2 * qa[j-1] * isp[LP_FILTER_ORDER-1] + qa[j-2];
>> for (i = lp_half_order-1; i > 1; i--)
>> qa[i] -= qa[i-2];
>> for (i = 0; i < lp_half_order; i++) {
>> pa[i] *= (1 + isp[LP_FILTER_ORDER-1]);
>> qa[i] *= (1 - isp[LP_FILTER_ORDER-1]);
>> }
>> for (i = 1, j = LP_FILTER_ORDER - 1; i < lp_half_order; i++, j--) {
>> Az[i-1] = (pa[i] + qa[i]) * 0.5;
>> Az[j-1] = (pa[i] - qa[i]) * 0.5;
>> }
>> 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];
>> }
>
> From which I don't see any obvious simplification.
:(
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20091222/cebdf6a2/attachment.pgp>
More information about the ffmpeg-devel
mailing list