jamoma / jamoma2

A header-only C++ library for building dynamic and reflexive systems with an emphasis on audio and media.
MIT License
30 stars 6 forks source link

Review new sine-tone example #81

Closed nwolek closed 8 years ago

nwolek commented 8 years ago

@tap - Did a lot of on the Oscillator class today and actually built a second example project that produces an A440 (src/sine-tone-example). Please review and comment.

A few specific things:

  1. I am not happy that I ended up with duplicate frequency and initialphase parameters in both the Oscillator and its mSync. If you can think of a way to consolidate, please advise.
  2. Should we provide a way to query the current phase?
  3. I would value your input on how to implement interpolation options (just defaults to Linear for now).
  4. Same for waveshape options (sine, cosine, sawtooth, triangle, etc).

Because of the new tests and projects, you will have to do a fresh build.

tap commented 8 years ago

Awesome.

When I run the sine example I hear some crackles in the sound. Do you hear that too? I also hear in the noise example. I've not noticed that before... (shuffle, shuffle) So switching my Apogee to 96K sample-rate eliminated the crackles, it must be that the sample-rate conversion or buffering related to it or ??? But that's weird because this example is hard-coded to 44.1K ???

Getting the phase would be useful, but even more useful would be setting the phase. Or at least resetting to the initial-phase. It could be that mPhase is not needed and you could just read and write to the phase parameter, but I'm not sure what the performance penalty for that is going to be... (just pondering out loud here). This little wrinkle probably deserves it's own ticket as it is a research project that may cover multiple classes, not just this one.

I guess what you are asking regarding the duplicated parameters is whether or not the Oscillator class should be a subclass that extends the Sync class. I think it would be a good experiment to refactor this way since we have no other cases of that (yet). Again that could be a separate ticket though.

In the constructor, there is this line:

            mSync.gain = tableSize; // ramp from 0 to tableSize

Is that correct? Or should it be

            mSync.gain = tableSize-1; // ramp from 0 to tableSize

?

nwolek commented 8 years ago

I do not hear crackles. I was just using my internal speakers, so maybe it is the sound card issue. That is strange given hard coded 44.1 kHz.

smacks forehead - Oscillator subclassing Sync. Not sure why I did not think to go this route!

I'll look at the tableSize issue as well. We may not hear the effect here, because the interpolation just uses a value of 0.0 when the index is out of bounds. In this case, the first sample happens to also be 0.0.

I won't get back to this until tomorrow or Thursday, so let me know if you have other thoughts before then.

nwolek commented 8 years ago

Progress report on issue #5: