Fix crackle in sonify.time_frequency#255
Conversation
sonify.time_frequency|
@craffel I think this is good to go (on my end). |
| frequency = float(frequency) | ||
| # hack so that we can ensure an integer number of periods and samples | ||
| # rounds frequency to 1st decimal, s.t. 10 * frequency will be an int | ||
| frequency = np.round(frequency, 1) |
There was a problem hiding this comment.
Doesn't this mean that only integer frequencies are sonifiable?
There was a problem hiding this comment.
No -- but i'm rounding each frequency to the nearest 10s place, so it might have an audible effect on higher frequencies.
There was a problem hiding this comment.
offline conversation summary: might be better to make the number of decimals a parameter n_dec, and maintain a table of size 10**n_dec * fs so that a user can control the precision of the approximation.
craffel
left a comment
There was a problem hiding this comment.
Thanks for picking this up! Two small comments.
|
|
||
| # Sum into the aggregate output waveform | ||
| output[start:end] += wave[start:end] * gram[n, m] | ||
| weights = gram_interpolator(np.arange(start, end)) |
There was a problem hiding this comment.
Looks like weights is unused? Did you mean to replace gram_interpolator(np.arange(start, end)) with weights below?
| else: | ||
| # if there is only one point in time_centers, interpolator | ||
| # returns constant | ||
| gram_interpolator = _const_interpolator(gram[n, 0]) |
There was a problem hiding this comment.
Can you replace all this logic with
gram_interpolator = interp1d(
time_centers, gram[n, :],
kind='linear' if len(time_centers) > 1 else 'nearest',
bounds_error=False, fill_value=0.0)or maybe 'zero' instead of 'nearest'? Alternatively if you can provide a little more of a comment for special casing, would be helpful for understanding it.
|
@craffel I ran into a snag in the interpolation when Using it fails because when Using The interpolator always returns zero -- for example: What do you want the expected behavior to be? For a length |
|
Hmm, those are frustrating behaviors of interp1d. Oh well, you can go back to what you had! |
fix edge case where has only one point make n_dec a parameter
|
Thanks @rabitt ! |
* linear interpolation between frames * fix sinewave discontinuity; replace ramp with interpolator fix edge case where has only one point make n_dec a parameter
This addresses @craffel's comments on #224.
The main changes are:
_fast_synthesize. The line:n_samples = int(10.0 * fs / frequency)is meant to generate 10 periods of the wave at
f0=frequency, and the generated wave is copied. For non-integer values offrequency,10. * fs / frequencyis not necessarily an integer, so casting it to an integer results in generating less than 10 periods of the wave, and then there is a discontinuity when the wave is copied. I did a hack to fix this problem and forcen_samplesto always be an integer.If this is merged, #224 can be closed. cc @stefan-balke