GNOME Bugzilla – Bug 353135
Detuning/Transposing Xtal Strings
Last modified: 2006-08-29 17:08:31 UTC
Recently, I created an instrument based on the idea of using two DavXtalString modules with slightly different parameter sets to make the instrument sound more "fat". Its easy to give the instrument a slightly different "tension" and so forth, however, when it comes to detuning the two modules in frequency, which would commonly be done in "cent", beast fails to provide the necessary logic. Note that similar to DavXtalStrings, this detuning (or even So in my demo (attached), I use a constant, set it to 0.5 Hz, and add that to the incoming frequency. However, my feeling is that this is discouraged for several reasons. (1) it is not as nice as "cent" detuning; there is no musical meaning attached to 0.5 Hz, and 0.5 Hz in particular also mean something different, depending on the input frequency; for lower frequencies, 0.5 Hz means more detuning than for higher frequencies (multiplying the input frequency by 1.00384 or something would fix this) (2) for big transposing - a whole octave, 7 half-tones, or some other musically meaningful distance two more problems arise (2a) it is not guaranteed that detuning will result in a frequency below 22000 Hz, so aliasing may be produced (2b) achieving a 7-half-tones detuning will require the user to fiddle with the calculator; when he finally finds a meaningful number (such as 1.7 - i didn't calculate it) the frequency must be multiplied with, he will find that the constant module is not capable of generating an 1.7-signal; he will need to add an 1 signal to a 0.7 signal, and then multiply the input frequency with that value; thats three modules for a mere 7-half-tones- transposition I would provide a patch, but I currently have two possible solutions in my mind, and I don't know which route to take: - copy the necessary frequency modifying properties to the XTalStrings module (upside: no extra modules; downside: the XtalStrings module, and other modules which are generators (create some kind of sound, like for instance DavOrgan) will become a lot fatter)) - write a seperate module with the dedicated task to take an input frequency and detune/transpose it according to the users wishes; it would also CLAMP it to the [0,1] range upon output (advantages: implementation is only required once; disadvantages: user will always require extra modules; for instance 2 modules for my instrument in case he wants to detune both XTalStrings to achieve symmetric detuning (detune one +2cent, one -2cent)) I'll attach a small test .bse file with my instrument, so you can hear that the detuning I applied to the XtalStrings module really improves the sound quality. The instrument could be included into the tarball once the "right way to deal with this" has been found, and if necessary, the code is in SVN.
Created attachment 71719 [details] Song with example instrument The "Iron String" instrument included in this song illustrates the effect of detuning.
(In reply to comment #0) > Note that similar to DavXtalStrings, this detuning (or even > you interrupted your sentence here... > - copy the necessary frequency modifying properties to the XTalStrings module > (upside: no extra modules; downside: the XtalStrings module, and other modules > which are generators (create some kind of sound, like for instance DavOrgan) > will become a lot fatter)) a patch for the xtal module to implement the detuning properties that e.g. the standard oscillator has would be appreciated. > - write a seperate module with the dedicated task to take an input frequency > and detune/transpose it according to the users wishes; it would also CLAMP it > to the [0,1] range upon output > (advantages: implementation is only required once; disadvantages: user will > always require extra modules; for instance 2 modules for my instrument in case > he wants to detune both XTalStrings to achieve symmetric detuning (detune one > +2cent, one -2cent)) such a module could reasonably also be added to beast, this should be discussed in a seperate bug report though. as for the standard oscillator, i think detuning fnuctionality also makes sense for the xtal string module, regardless of the possible addition of a detuning module.
(In reply to comment #2) > (In reply to comment #0) > > > Note that similar to DavXtalStrings, this detuning (or even > > Strange. It should continue as follows: transposition) is probably helpful for any generator-like module that we'll have. For now, I suppose that's only the wave oscillator and the DavOrgan.
(In reply to comment #3) > Note that similar to DavXtalStrings, this detuning (or even > transposition) is probably helpful for any generator-like module that we'll > have. For now, I suppose that's only the wave oscillator and the DavOrgan. is that volounteering to also submit a patch for DavOrgan detuning? ;)
Created attachment 71725 [details] [review] Proposed XTalStrings patch Mon Aug 28 00:02:24 2006 Stefan Westerfeld <stefan@space.twc.de> * davxtalstrings.[hc]: Add transpose and fine_tune settings, just like they can be found in the oscillator code. Fixed a few minor i18n bugs in property translation markup.
(In reply to comment #4) > (In reply to comment #3) > > Note that similar to DavXtalStrings, this detuning (or even > > transposition) is probably helpful for any generator-like module that we'll > > have. For now, I suppose that's only the wave oscillator and the DavOrgan. > > is that volounteering to also submit a patch for DavOrgan detuning? ;) > Ok, when you're done reviewing the XTalStrings patch, so that it can to into SVN, I'll create a similar one for DavOrgan.
Comment on attachment 71725 [details] [review] Proposed XTalStrings patch >@@ -270,7 +296,7 @@ typedef struct { > guint count; > gfloat *string; > gfloat last_trigger_level; >- gfloat last_trigger_freq; >+ gfloat last_transposed_trigger_freq; note that the saved last-freq value is stored as a float. > DavXtalStringsParams tparams; > } XtalStringsModule; > >@@ -278,13 +304,19 @@ typedef struct { > */ > static inline void > xmod_trigger (XtalStringsModule *xmod, >- gfloat trigger_freq) >+ gdouble untransposed_trigger_freq) and here you change freq precision to double, allthough it will always be passed in as a float. also, you did this without notice/explanation. this may seem like a subtle unimportant change, but if at some later point, untransposed_trigger_freq is compared against a value from the signal stream, it will allmost always be false because float!=duoble in general. > { > guint i; > guint pivot; >- >+ >+ /* calculate transposed trigger frequency; the "real frequency" that will >+ * be played, including detuning and transpose operations >+ */ >+ gdouble trigger_freq = untransposed_trigger_freq; >+ trigger_freq *= bse_transpose_factor (xmod->tparams.transpose); >+ trigger_freq *= bse_cent_factor (xmod->tparams.fine_tune); so here, calculation of transposed freq changed to double. > trigger_freq = CLAMP (trigger_freq, 27.5, 4000.0); >- xmod->last_trigger_freq = trigger_freq; >+ xmod->last_transposed_trigger_freq = trigger_freq; but here, your casting the freq back to a float. i think the code should be consistent in what it does, i.e. use either float or double for the freq. looking at the oscillator code, it does use double for transposed frequencies, so chaging the precision here also seems in order. that means, you need to fix up last_transposed_trigger_freq to double in XtalStringsModule, and -most importantly- announce this change explicitely in the changelog. > > xmod->pos = 0; > xmod->count = 0; >@@ -325,7 +357,7 @@ xmod_process (BseModule *module, > guint real_freq_256, actual_freq_256; > guint i; > >- real_freq_256 = (int) (xmod->last_trigger_freq * 256); >+ real_freq_256 = (int) (xmod->last_transposed_trigger_freq * 256); > actual_freq_256 = (int) (bse_engine_sample_freq() * 256. / xmod->size); > > if (BSE_MODULE_ISTREAM (module, DAV_XTAL_STRINGS_ICHANNEL_FREQ).connected) >@@ -339,7 +371,7 @@ xmod_process (BseModule *module, > if (G_UNLIKELY (BSE_SIGNAL_RAISING_EDGE (last_trigger_level, trigger_in[i]))) > { > xmod_trigger (xmod, freq_in ? BSE_SIGNAL_TO_FREQ (freq_in[i]) : xmod->tparams.freq); >- real_freq_256 = (int) (xmod->last_trigger_freq * 256); >+ real_freq_256 = (int) (xmod->last_transposed_trigger_freq * 256); > actual_freq_256 = (int) (bse_engine_sample_freq() * 256. / xmod->size); > } > last_trigger_level = trigger_in[i]; >@@ -389,7 +421,7 @@ xmod_reset (BseModule *module) > xmod->count = 0; > xmod->a = 0.0; > xmod->damping_factor = 0.0; >- xmod->last_trigger_freq = 440.0; /* just _some_ valid freq for the stepping */ >+ xmod->last_transposed_trigger_freq = 440.0; /* just _some_ valid freq for the stepping */ > xmod->last_trigger_level = 0; > } other than the float vs. double issue, the patch generally looks good and can go in.
(In reply to comment #7) > (From update of attachment 71725 [details] [review] [edit]) > >@@ -270,7 +296,7 @@ typedef struct { > > guint count; > > gfloat *string; > > gfloat last_trigger_level; > >- gfloat last_trigger_freq; > >+ gfloat last_transposed_trigger_freq; > > note that the saved last-freq value is stored as a float. Yes. > > DavXtalStringsParams tparams; > > } XtalStringsModule; > > > >@@ -278,13 +304,19 @@ typedef struct { > > */ > > static inline void > > xmod_trigger (XtalStringsModule *xmod, > >- gfloat trigger_freq) > >+ gdouble untransposed_trigger_freq) > > and here you change freq precision to double, allthough it will always be > passed in as a float. > also, you did this without notice/explanation. this may seem like a subtle > unimportant > change, but if at some later point, untransposed_trigger_freq is compared > against a > value from the signal stream, it will allmost always be false because > float!=duoble in general. Yes, but the purpose of this function is to trigger the module at a given frequency; and not to store untransposed_trigger_freq into some data member. If a float value is passed, and computations follow, then passing untransposed_trigger_freq as float would be sufficient. If a double value is passed, then the value is needlessly narrowed down to float, but then double calculations follow. I did increase the precision of the datatype to state my intension to the compiler: that this is a value that I want to be calculated with the best possible precision. > > { > > guint i; > > guint pivot; > >- > >+ > >+ /* calculate transposed trigger frequency; the "real frequency" that will > >+ * be played, including detuning and transpose operations > >+ */ > >+ gdouble trigger_freq = untransposed_trigger_freq; > >+ trigger_freq *= bse_transpose_factor (xmod->tparams.transpose); > >+ trigger_freq *= bse_cent_factor (xmod->tparams.fine_tune); > > so here, calculation of transposed freq changed to double. Yes, again, I am stating my intension to the compiler here: that the calculation itself, which involves a few floating point multiplications, is done with the best precision available. > > trigger_freq = CLAMP (trigger_freq, 27.5, 4000.0); > >- xmod->last_trigger_freq = trigger_freq; > >+ xmod->last_transposed_trigger_freq = trigger_freq; > > but here, your casting the freq back to a float. > i think the code should be consistent in what it does, i.e. use either > float or double for the freq. looking at the oscillator code, it does > use double for transposed frequencies, so chaging the precision here > also seems in order. that means, you need to fix up > last_transposed_trigger_freq > to double in XtalStringsModule, and -most importantly- announce this change > explicitely in the changelog. I think calculating as double and backcasting to float *is* a consistent strategy. I don't want any unneccessary precision loss to be caused by the multiplications, so I instruct the compiler to perform double multiplications, not floating point multiplications. Since multiplied values are stored then into data fields, there is no "comparing the value with the original value"; it will not match as soon as tuning or transposition is not 0, so there is no need that both are floats, and never up/downconverted inbetween. > > xmod->pos = 0; > > xmod->count = 0; > >@@ -325,7 +357,7 @@ xmod_process (BseModule *module, > > guint real_freq_256, actual_freq_256; > > guint i; > > > >- real_freq_256 = (int) (xmod->last_trigger_freq * 256); > >+ real_freq_256 = (int) (xmod->last_transposed_trigger_freq * 256); > > actual_freq_256 = (int) (bse_engine_sample_freq() * 256. / xmod->size); > > > > if (BSE_MODULE_ISTREAM (module, DAV_XTAL_STRINGS_ICHANNEL_FREQ).connected) > >@@ -339,7 +371,7 @@ xmod_process (BseModule *module, > > if (G_UNLIKELY (BSE_SIGNAL_RAISING_EDGE (last_trigger_level, trigger_in[i]))) > > { > > xmod_trigger (xmod, freq_in ? BSE_SIGNAL_TO_FREQ (freq_in[i]) : xmod->tparams.freq); > >- real_freq_256 = (int) (xmod->last_trigger_freq * 256); > >+ real_freq_256 = (int) (xmod->last_transposed_trigger_freq * 256); > > actual_freq_256 = (int) (bse_engine_sample_freq() * 256. / xmod->size); > > } > > last_trigger_level = trigger_in[i]; > >@@ -389,7 +421,7 @@ xmod_reset (BseModule *module) > > xmod->count = 0; > > xmod->a = 0.0; > > xmod->damping_factor = 0.0; > >- xmod->last_trigger_freq = 440.0; /* just _some_ valid freq for the stepping */ > >+ xmod->last_transposed_trigger_freq = 440.0; /* just _some_ valid freq for the stepping */ > > xmod->last_trigger_level = 0; > > } > > other than the float vs. double issue, the patch generally looks good and can > go in. Well, as you see, I don't have the same opinion as you have; however, if by changing the precision of the data field to gdouble, we can resolve the issue, and the patch can go in, then I'll do it (new patch follows).
Created attachment 71838 [details] [review] New patch for DavXTalStrings Mon Aug 28 00:02:24 2006 Stefan Westerfeld <stefan@space.twc.de> * davxtalstrings.[hc]: Add transpose and fine_tune settings, just like they can be found in the oscillator code. Fixed a few minor i18n bugs in property translation markup. Use gdouble precision to store transposed frequency within the engine module data structure.
thanks, looks good, please commit.
(In reply to comment #10) > thanks, looks good, please commit. > Committed.