[Bug 14717] resampled sound is horrible

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=14717

--- Comment #202 from Alexander E. Patrakov <patrakov at gmail.com> 2011-11-04 12:05:51 CDT ---
Yes, this is a bug in the original program, and PI -> 2 * PI won't solve it.
The invariant that should hold is that sin(M_PI * x * cutoff) == 0.

Here x = g_fir->size if one uses linear interpolation and correctly
interpolates one point beyond the end (you don't), or g_fir->size - 1 if one
uses the variant of linear interpolation as in your old patch that does not
invent an extra point (in this case, the argument to Kaiser function should be
modified as well). In other words, at the end of the FIR (including the
interpolated area) the sine should be zero and the argument of the Kaiser
function (Izero) should be zero. But this is impossible if one specifies the
cutoff frequency precisely, as this would require a FIR with a fractional
length.

With e.g. Gaussian window (but not Kaiser, because one cannot evaluate it one
point beyond its intended domain due to sqrt()) and a slightly different
implementation of the cubic interpolation (i.e. just refuse to interpolate in
the last segment instead of inventing a point like you do in line 221) one
would get x = g_fir->size - 2 and one extra point at the end of the FIR table
that guarantees the correct derivatives. With Kaiser window, I'd use cubic
interpolation everywhere except the last segment, where linear interpolation
would go. But honestly, I don't see much point in this Kaiser window. A
Gaussian window would work about just as well. BTW I also tried to
reverse-engineer a FIR used by Windows XP, and found that Kaiser and Gaussian
windows fit it equally well (not exactly, but much better than raised-cosine
family of windows) because at this length these two types of windows are very
similar.

And BTW your handling of boundary conditions is wrong for the case near firpos
== 0. You invent extra points by linear continuation, instead of using the
symmetry. I think it would be better to store both wings of the FIR explicitly
(concept: fir[-x] == fir[x]) instead of inserting ifs.

I solved the boundary condition problem locally (and yes, I am working on a
different implementation and intentionally duplicating your work so that we can
compare the results and spot missing cleanups, etc) with the following code:

  fprintf(stderr, "creating %s filter...\n", firparams[idx].name);
  fflush(stderr);

  g_fir->step = firparams[idx].step;
  g_fir->size = firparams[idx].width0 * g_fir->step / firparams[idx].passband +
1;
  cutoff = firparams[idx].width0 / (g_fir->size - 1);
  ALLOC(g_fir->wing, g_fir->size);

but this is obviously valid only for linear interpolation.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.