After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 353135 - Detuning/Transposing Xtal Strings
Detuning/Transposing Xtal Strings
Status: RESOLVED FIXED
Product: beast
Classification: Other
Component: plugins
SVN trunk
Other All
: Normal enhancement
: ---
Assigned To: Beast Maintainers
Beast Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-08-27 19:58 UTC by Stefan Westerfeld
Modified: 2006-08-29 17:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Song with example instrument (6.29 KB, text/plain)
2006-08-27 20:00 UTC, Stefan Westerfeld
  Details
Proposed XTalStrings patch (6.15 KB, patch)
2006-08-27 22:08 UTC, Stefan Westerfeld
none Details | Review
New patch for DavXTalStrings (6.25 KB, patch)
2006-08-29 15:29 UTC, Stefan Westerfeld
none Details | Review

Description Stefan Westerfeld 2006-08-27 19:58:58 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.
Comment 1 Stefan Westerfeld 2006-08-27 20:00:37 UTC
Created attachment 71719 [details]
Song with example instrument

The "Iron String" instrument included in this song illustrates the effect of detuning.
Comment 2 Tim Janik 2006-08-27 20:49:00 UTC
(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.
Comment 3 Stefan Westerfeld 2006-08-27 21:46:29 UTC
(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.
Comment 4 Tim Janik 2006-08-27 22:03:42 UTC
(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? ;)
Comment 5 Stefan Westerfeld 2006-08-27 22:08:25 UTC
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.
Comment 6 Stefan Westerfeld 2006-08-28 19:44:29 UTC
(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 7 Tim Janik 2006-08-29 00:20:20 UTC
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.
Comment 8 Stefan Westerfeld 2006-08-29 15:27:29 UTC
(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).
Comment 9 Stefan Westerfeld 2006-08-29 15:29:15 UTC
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.
Comment 10 Tim Janik 2006-08-29 15:33:35 UTC
thanks, looks good, please commit.
Comment 11 Stefan Westerfeld 2006-08-29 16:28:03 UTC
(In reply to comment #10)
> thanks, looks good, please commit.
> 

Committed.