[FFmpeg-devel] [PATCH] Common ACELP routines (2/3) - filters
Vladimir Voroshilov
voroshil
Mon Apr 28 05:41:03 CEST 2008
On Mon, Apr 28, 2008 at 1:35 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> On Mon, Apr 28, 2008 at 01:02:32AM +0700, Vladimir Voroshilov wrote:
[...]
> > > > + -pitch_delay = -(6*pitch_delay_int+pitch_delay_frac) =
> > > > +
> > > > + = -6*pitch_delay_int - pitch_delay_frac =
> > > > +
> > > > + / -6*(pitch_delay_int) - pitch_delay_frac, pitch_delay_frac < 0
> > > > + =<
> > > > + \ -6*(pitch_delay_int+1) + (6-pitch_delay_frac), pitch_delay_frac >= 0
> > > > + */
> > > > +
> > > > + /* Compute negative value of fractional delay */
> > > > + pitch_delay_frac = - pitch_delay_frac;
> > > > +
> > >
> > > > + /* Now make sure that pitch_delay_frac will always be positive */
> > >
> > > > + if(pitch_delay_frac < 0)
> > > > + {
> > > > + pitch_delay_frac += 6;
> > > > + pitch_delay_int++;
> > > > + }
> > >
> > > Can it really be >= 0 and <0 ?
> >
> > Didn't understand you.
> > According to current pitch delay decode routine pitch_delay_frac
> > belongs to [-2; 3]
> > -pitch_delay_frac belongs to [-3; 2]
>
> In the latest code i have it is:
>
> + pitch_delay_3x = 3 * ctx->pitch_delay_int_prev + 1;
> + if(!i)
> + pitch_delay_1st_int = pitch_delay_3x / 3;
> + }
> + else if (!i)
> + {
> + // Decoding of the adaptive-codebook vector delay for first subframe (4.1.3)
> + pitch_delay_3x = parm->ac_index[i] + 59;
> + if(pitch_delay_3x > 255)
> + pitch_delay_3x = 3 * pitch_delay_3x - 512;
> + pitch_delay_1st_int = pitch_delay_3x / 3;
> + }
> + else
> + {
> + // Decoding of the adaptive-codebook vector delay for second subframe (4.1.3)
> + int pitch_delay_min_3k = 3 * av_clip(
> + ctx->pitch_delay_int_prev-5,
> + PITCH_LAG_MIN,
> + PITCH_LAG_MAX-9);
> +
> + pitch_delay_3x = parm->ac_index[i] + pitch_delay_min_3k - 1;
> + }
> ...
> + ff_acelp_interpolate_excitation(pitch_delay_3x, ctx->exc + i*ctx->subframe_size, ctx->subframe_size);
>
> In this code pitch_delay_3x should be >0 thus the if will always be true and
> can be simplified out! The same applies to the *(-1) before it.
Michael, you messed up two different versions.
"if" in quoted code applies to "1-pitch_delay_3x%3", not to "pitch_delay_3x"
thus argument of "if" can be negative obviously.
Here are two versions.
One (packed int/frac):
decode_lag()
{
return int*3+frac+1 (frac = [-1; 1])
}
interp(pitch_delay_3_x)
{
frac = 1-pitch_delay_3x%3
if(frac<0)
{
frac+=3;
int++;
}
}
Second (not packed, used in my latest patches, G.729
and AMR reference codes):
decode_lag(*int, *frac)
{
*int=
*frac= (frac = [-1; 1])
}
interp(int, frac)
{
frac = -frac
if(frac<0)
{
frac+=3;
int++;
}
}
I cant see how those "if" can be eliminated without
complicating this and decode_lag code.
[...]
> > > > +void ff_acelp_high_pass_filter(
> > > > + int16_t* out,
> > >
> > > > + int16_t* hpf_z,
> > > > + int* hpf_f,
> > > > + const int16_t* in,
> > >
> > > > + int length)
> > > > +{
> > > > + int i;
> > > > +
> > > > + for(i=0; i<length; i++)
> > > > + {
> > > > + memmove(hpf_z + 1, hpf_z, 2 * sizeof(hpf_z[0]));
> > > > + hpf_z[0] = in[i];
> > >
> > > > +
> > > > + hpf_f[0] = MULL(hpf_f[1], 15836);
> > > > + hpf_f[0] += MULL(hpf_f[2], -7667);
> > > > +
> > > > + hpf_f[0] += 7699 * (hpf_z[0] - 2*hpf_z[1] + hpf_z[2]);
> > > > +
> > > > + /* Clippin is required to pass G.729 OVERFLOW test */
> > >
> > > > + if(hpf_f[0] >= 0xfffffff)
> > > > + {
> > > > + out[i] = SHRT_MAX;
> > >
> > > > + hpf_f[0] = 0x3fffffff;
> > > > + }
> > > > + else if (hpf_f[0] <= -0x10000000)
> > > > + {
> > > > + out[i] = SHRT_MIN;
> > >
> > > > + hpf_f[0] = -0x40000000;
> > > > + }
> > > > + else
> > > > + {
> > >
> > > > + hpf_f[0] <<= 2;
> > >
> > > The shift is avoidable i think as already said
> >
> > Changed and added comment about bitexactness.
>
> I do think the bitexactness can be maintained.
I DO, or you do NOT ?
Assuming you DO, added #ifdef G729_BITEXACT :)
> > There is also some not clear thing for me..
> > ff_acelp_interpolate_excitation (according to math) is just an low-pass filter.
> > But it is intended to build adaptive-codebook (pitch) vector from
> > previous speech data.
> > I wonder should it be moved to something like acelp_vectors.c (along
> > with fixed codebook (innovation) vector decoding, fixed vector
> > updating and building excitation signal from pitch and innovation
> > vectors) ?
>
> Whichever way you prefer ...
Will be moved to acelp_vectors.c later,
but I want to finish discussion about it first.
> > +void ff_acelp_high_pass_filter(
> > + int16_t* out,
> > + int16_t* hpf_z,
> > + int* hpf_f,
> > + const int16_t* in,
> > + int length)
> > +{
> > + int i;
> > +
> > + for(i=0; i<length; i++)
> > + {
> > + memmove(hpf_z + 1, hpf_z, 2 * sizeof(hpf_z[0]));
> > + hpf_z[0] = in[i];
> > +
> > + /* Note: to make result bitexact with G.729 reference code,
> > + two least significant bits of each value returned by MULL
> > + should be cleared */
> > + hpf_f[0] = MULL(hpf_f[1], 15836); // (15.14) = (14.14) * (1.13)
> > + hpf_f[0] += MULL(hpf_f[2], -7667); // (14.14) = (14.14) * (0.13)
> > +
> > + hpf_f[0] += 30796 * (hpf_z[0] - 2*hpf_z[1] + hpf_z[2]); // (16.14) = (3.13) * (14.0)
> > +
>
> > + /* Clipping is required to pass G.729 OVERFLOW test */
> > + if(hpf_f[0] >= 0x3fffffff)
>
> > + {
> > + out[i] = SHRT_MAX;
> > + hpf_f[0] = 0x3fffffff;
> > + }
> > + else if (hpf_f[0] <= -0x40000000)
>
> > + {
> > + out[i] = SHRT_MIN;
> > + hpf_f[0] = -0x40000000;
> > + }
> > + else
> > + {
> > + /* Multiplication by 2 with rounding can cause short type overflow, thus
> > + additional clipping is required. */
> > + out[i] = av_clip_int16((hpf_f[0] + 0x2000) >> 14); /* (15.0) = 2 * (14.14) */
> > + }
>
> The cliping looks duplicated
Remove first clipping passes all test, thus removed.
--
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: acelp_filt_39.diff
Type: text/x-diff
Size: 12436 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080428/8d8d5fe7/attachment.diff>
More information about the ffmpeg-devel
mailing list