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 450490 - Support for bsewave chunk fine-tune xinfo (chunk pitch adjustment)
Support for bsewave chunk fine-tune xinfo (chunk pitch adjustment)
Status: RESOLVED FIXED
Product: beast
Classification: Other
Component: bse
SVN trunk
Other Linux
: High enhancement
: m0.7
Assigned To: Beast Maintainers
Beast Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-06-23 23:15 UTC by Stefan Westerfeld
Modified: 2007-11-15 20:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed implementation; adds "fine tune" xinfo (8.08 KB, patch)
2007-06-24 14:22 UTC, Stefan Westerfeld
none Details | Review
Updated "fine tune" xinfo implementation (8.31 KB, patch)
2007-08-24 17:24 UTC, Stefan Westerfeld
none Details | Review
[PATCH] Added necessary code for "fine-tune" xinfo, bug #450490. (6.67 KB, patch)
2007-10-31 21:40 UTC, Stefan Westerfeld
none Details | Review
[PATCH] * docs/architecture.doxi: Documented the new "fine-tune" xinfo. (1.40 KB, patch)
2007-10-31 21:40 UTC, Stefan Westerfeld
rejected Details | Review
[PATCH] * tests/misctests.cc: Added test for bse_cent_tune(). (981 bytes, patch)
2007-10-31 21:40 UTC, Stefan Westerfeld
rejected Details | Review
[PATCH] * tests/misctests.cc: Added test for bse_cent_tune(). (427 bytes, patch)
2007-10-31 21:40 UTC, Stefan Westerfeld
rejected Details | Review
[PATCH] Merge BSE_FINE_TUNE_FACTOR() and bse_cent_factor(), bug #450490. (7.85 KB, patch)
2007-10-31 21:40 UTC, Stefan Westerfeld
committed Details | Review
[PATCH] Use bse_cent_tune_fast() where appropriate, bug #450490. (1.69 KB, patch)
2007-10-31 21:40 UTC, Stefan Westerfeld
committed Details | Review
Updated implementation of fine-tune xinfo (9.02 KB, patch)
2007-11-05 18:03 UTC, Stefan Westerfeld
committed Details | Review

Description Stefan Westerfeld 2007-06-23 23:15:03 UTC
It has been discussed that it may be desirable to introduce a new xinfo for bsewave files that allows adjusting the pitch at which a particular chunk will be replayed.

This can be useful for instance when designing drum kits and only one tom sample is available, but a high and low tom shall be in the drum kit.

I'll attach the implementation I wrote later (as soon as I tested it).
Comment 1 Stefan Westerfeld 2007-06-24 14:22:42 UTC
Created attachment 90553 [details] [review]
Proposed implementation; adds "fine tune" xinfo

Implementation of the fine tune xinfo; this song:
  
  http://space.twc.de/~stefan/music/finetunetest.bse

contains a wave file with fine tune xinfos for achieving tuned toms; only one tom sample is actually there, but used with different fine tune settings.

Here is an example script which I used to build the corresponding multisample out of the retro material:

rm -f r.bsewave
tools/bsewavetool create r.bsewave 1
tools/bsewavetool add-chunk r.bsewave -m39 ~/share/music/samples/kit/retro_kit/bd_mittel016.wav
tools/bsewavetool add-chunk r.bsewave -m40 ~/share/music/samples/kit/retro_kit/tom_med_mittel041.wav
tools/bsewavetool xinfo r.bsewave -m40 fine-tune=-600.0
tools/bsewavetool add-chunk r.bsewave -m41 ~/share/music/samples/kit/retro_kit/tom_med_mittel041.wav
tools/bsewavetool xinfo r.bsewave -m41 fine-tune=-304.0
tools/bsewavetool add-chunk r.bsewave -m42 ~/share/music/samples/kit/retro_kit/tom_med_mittel041.wav
tools/bsewavetool add-chunk r.bsewave -m43 ~/share/music/samples/kit/retro_kit/tom_med_mittel041.wav
tools/bsewavetool xinfo r.bsewave -m43 fine-tune=300.0
tools/bsewavetool add-chunk r.bsewave -m44 ~/share/music/samples/kit/retro_kit/tom_med_mittel041.wav
tools/bsewavetool xinfo r.bsewave -m44 fine-tune=777.77
tools/bsewavetool add-chunk r.bsewave -m45 ~/share/music/samples/kit/retro_kit/sn_mittel019.wav
tools/bsewavetool clip r.bsewave --all-chunks -s 0.002
tools/bsewavetool downsample2 r.bsewave
tools/bsewavetool oggenc r.bsewave
Comment 2 Tim Janik 2007-06-25 17:42:46 UTC
Comment on attachment 90553 [details] [review]
Proposed implementation; adds "fine tune" xinfo

>Index: docs/architecture.doxi
>===================================================================
>--- docs/architecture.doxi	(revision 4341)
>+++ docs/architecture.doxi	(working copy)
>@@ -112,6 +112,13 @@ of the lowercase letters 'a'-'z', the di
> 	0.5 (play sample half as loud, corresponding to a -6dB adjustment) and
> 	0.25 (corresponds to a -12dB adjustment). The range of valid volume
> 	adjustments is: 0 < volume <= 1.0, any other value will be ignored.
>+@item @strong{fine-tune}
>+	The fine tune allows to change the speed and thus the pitch at which

i think this section is better if you simply get rid of "speed" in here. pitch describes it all.

>+	the sample is replayed in cent. This does not affect the raw/compressed
>+	audio data actually stored. The default is 0, which means no fine tune.
>+	Examples of valid values are 100, which would pitch the sample up one
>+	semitone (100 cents), 1200, which would pitch up the sample one octave
>+	and -53.28 which would pitch the sample about half a semitone down.

what?? half a semitone is of course exactly 50 cent, see cent_table201 in bsemathsignal.c, 2**(1/24.)==1.02930223664349 is the same as 2**(1/1200.*50.)==1.02930223664349202878 (latter value pasted from table).

> @item @strong{gus-patch-envelope} -
> 	Contains the envelope points for a GUS Patch envelope.
> @done
>Index: bse/gslwaveosc.c
>===================================================================
>--- bse/gslwaveosc.c	(revision 4341)
>+++ bse/gslwaveosc.c	(working copy)
>@@ -250,7 +250,7 @@ gsl_wave_osc_set_filter (GslWaveOscData 
>   if (UNLIKELY (!wosc->config.lookup_wchunk))
>     return;
> 
>-  wosc->step_factor = zero_padding * wosc->wchunk->mix_freq;
>+  wosc->step_factor = zero_padding * wosc->wchunk->mix_freq * wosc->wchunk->fine_tune_factor;
>   wosc->step_factor /= wosc->wchunk->osc_freq * wosc->mix_freq;
>   step = wosc->step_factor * play_freq;
>   istep = step * (FRAC_MASK + 1.) + 0.5;

i don't see how this is possibly going to work, you're using the fine_tune to "adjust" 1 out of 3 places in this file where mix_freq is used. instead of fiddling with mix_freq, you need to multiple osc_freq with the fine tune factor.

>Index: bse/ChangeLog
>===================================================================
>--- bse/ChangeLog	(revision 4344)
>+++ bse/ChangeLog	(working copy)
>@@ -1,3 +1,19 @@
>+Sun Jun 24 16:01:58 2007  Stefan Westerfeld  <stefan@space.twc.de>
>+
>+	* bsemathsignal.[hc]: Implemented bse_fine_tune_factor, which is
>+	similar to bse_cent_factor, but accepts arbitary doubles as input
>+	because it doesn't use a table internally.
>+
>+	* gsldatahandle.[hc]: Implemented gsl_data_handle_fine_tune, which
>+	returns a fine tuning measured in cent ("fine-tune" xinfo).

the cent scaling is designed to adjust pitches beyond the hearing threshold, so i don't really think adding a bse_fine_tune_factor function is neccessary, indexing with a rounded value into the cent table should be good enough.

>+
>+	* gslwavechunk.[hc]: Cache the fine tune factor here, so that
>+	GslWaveOsc can access it quickly.
>+
>+	* gslwaveosc.c (gsl_wave_osc_set_filter): Take fine tune factor of the
>+	chunk into account for step computation (and thus for filter
>+	computation).
>+
> Sun Jun 24 00:17:40 2007  Stefan Westerfeld  <stefan@space.twc.de>
> 
> 	* bsemath.h: Added ln (10)/20 constant for ArtsCompressor.
>Index: bse/bsemathsignal.c
>===================================================================
>--- bse/bsemathsignal.c	(revision 4341)
>+++ bse/bsemathsignal.c	(working copy)
>@@ -213,6 +213,12 @@ static const double cent_table201[100 + 
> };
> const double * const bse_cent_table = cent_table201 + 100;
> 
>+double
>+bse_fine_tune_factor (double fine_tune)
>+{
>+  return exp (fine_tune * BSE_LN_2_POW_1_DIV_1200_d);
>+}
>+

as mentioned above, i hope this function is not really neccessary.

> 
> /* --- musical tuning systems --- */
> #define SCALED_INTERVAL(scale, F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12)       \
>Index: bse/bsemathsignal.h
>===================================================================
>--- bse/bsemathsignal.h	(revision 4341)
>+++ bse/bsemathsignal.h	(working copy)
>@@ -402,6 +402,18 @@ double        bse_transpose_factor      
> extern const double * const bse_cent_table;
> #define	bse_cent_factor(index /* -100..+100 */)		(bse_cent_table[index])
> 
>+/**
>+ * @param fine_tune	fine tuning in cent
>+ * @return		a factor corresponding to this
>+ *
>+ * This function computes a factor which corresponds to a given fine tuning in
>+ * cent.  The result can be used as factor for the frequency or the play speed.
>+ * It is similar to the bse_cent_factor macro; however, it doesn't use a table
>+ * internally, so the input is not limited in any way (i.e. 777.777 is a valid
>+ * fine_tune argument).
>+ */

please use the .c files for function documentation, header files shouldn't be cluttered with inline docs.

>+double  bse_fine_tune_factor (double fine_tune);
>+
> /* --- implementation details --- */
> static inline double  G_GNUC_CONST
> bse_approx_atan1 (register double x)
>Index: bse/gslwavechunk.c
>===================================================================
>--- bse/gslwavechunk.c	(revision 4341)
>+++ bse/gslwavechunk.c	(working copy)
>@@ -638,7 +638,8 @@ gsl_wave_chunk_new (GslDataCache   *dcac
>   wchunk->open_count = 0;
>   wchunk->mix_freq = mix_freq;
>   wchunk->osc_freq = osc_freq;
>-  wchunk->volume_adjust = 0.0;  /* will be set in gsl_wave_chunk_open */
>+  wchunk->volume_adjust = 0.0;	    /* will be set in gsl_wave_chunk_open */
>+  wchunk->fine_tune_factor = 0.0;   /* will be set in gsl_wave_chunk_open */
>   wchunk->requested_loop_type = loop_type;
>   wchunk->requested_loop_first = loop_first;
>   wchunk->requested_loop_last = loop_last;
>@@ -697,6 +698,7 @@ gsl_wave_chunk_open (GslWaveChunk *wchun
>       wchunk->length *= wchunk->n_channels;
>       wchunk->n_pad_values = BSE_CONFIG (wave_chunk_padding) * wchunk->n_channels;
>       wchunk->volume_adjust = gsl_data_handle_volume (wchunk->dcache->dhandle);
>+      wchunk->fine_tune_factor = bse_fine_tune_factor (gsl_data_handle_fine_tune (wchunk->dcache->dhandle));
>       gsl_data_cache_open (wchunk->dcache);
>       gsl_data_handle_close (wchunk->dcache->dhandle);
>       g_return_val_if_fail (wchunk->dcache->padding >= wchunk->n_pad_values, BSE_ERROR_INTERNAL);
>@@ -751,6 +753,7 @@ gsl_wave_chunk_close (GslWaveChunk *wchu
>   wchunk->leave_end_norm = 0;
>   wchunk->tail_start_norm = 0;
>   wchunk->volume_adjust = 0.0;
>+  wchunk->fine_tune_factor = 0.0;
>   gsl_wave_chunk_unref (wchunk);
> }
>
Comment 3 Stefan Westerfeld 2007-06-25 20:35:40 UTC
(In reply to comment #2)
> (From update of attachment 90553 [details] [review] [edit])
> >Index: docs/architecture.doxi
> >===================================================================
> >--- docs/architecture.doxi	(revision 4341)
> >+++ docs/architecture.doxi	(working copy)
> >@@ -112,6 +112,13 @@ of the lowercase letters 'a'-'z', the di
> > 	0.5 (play sample half as loud, corresponding to a -6dB adjustment) and
> > 	0.25 (corresponds to a -12dB adjustment). The range of valid volume
> > 	adjustments is: 0 < volume <= 1.0, any other value will be ignored.
> >+@item @strong{fine-tune}
> >+	The fine tune allows to change the speed and thus the pitch at which
> 
> i think this section is better if you simply get rid of "speed" in here. pitch
> describes it all.

Well, there are different methods for changing the pitch, and they have different implications on the sound, and on the limits on what you can do. But maybe I am thinking too complicated for joe user.

> >+	the sample is replayed in cent. This does not affect the raw/compressed
> >+	audio data actually stored. The default is 0, which means no fine tune.
> >+	Examples of valid values are 100, which would pitch the sample up one
> >+	semitone (100 cents), 1200, which would pitch up the sample one octave
> >+	and -53.28 which would pitch the sample about half a semitone down.
> 
> what?? half a semitone is of course exactly 50 cent, see cent_table201 in
> bsemathsignal.c, 2**(1/24.)==1.02930223664349 is the same as
> 2**(1/1200.*50.)==1.02930223664349202878 (latter value pasted from table).

I didn't want to argue about the size of half a semitone (rather I wanted to illustrate that you can use floating point values if you want to). Maybe I choose the wrong words here, though. I used "about half a semitone" as in "approximately half a semitone", would this other way of saying it make my intention more clear to you?

> > @item @strong{gus-patch-envelope} -
> > 	Contains the envelope points for a GUS Patch envelope.
> > @done
> >Index: bse/gslwaveosc.c
> >===================================================================
> >--- bse/gslwaveosc.c	(revision 4341)
> >+++ bse/gslwaveosc.c	(working copy)
> >@@ -250,7 +250,7 @@ gsl_wave_osc_set_filter (GslWaveOscData 
> >   if (UNLIKELY (!wosc->config.lookup_wchunk))
> >     return;
> > 
> >-  wosc->step_factor = zero_padding * wosc->wchunk->mix_freq;
> >+  wosc->step_factor = zero_padding * wosc->wchunk->mix_freq * wosc->wchunk->fine_tune_factor;
> >   wosc->step_factor /= wosc->wchunk->osc_freq * wosc->mix_freq;
> >   step = wosc->step_factor * play_freq;
> >   istep = step * (FRAC_MASK + 1.) + 0.5;
> 
> i don't see how this is possibly going to work, you're using the fine_tune to
> "adjust" 1 out of 3 places in this file where mix_freq is used. instead of
> fiddling with mix_freq, you need to multiple osc_freq with the fine tune
> factor.

The code in question uses two kinds of mixing frequencies:

  wosc->wchunk->mix_freq - the chunk mix freq
  wosc->mix_freq         - the engine mix freq

So if you want to interpret my code as mixing frequency adjustment, you need to only adjust the chunk mixing frequency (and then the argument about the number of changes I made again makes sense). But that was not what I had in mind when I wrote it. I just saw that if I increase step_factor for pitching up, everything else (which is derived from step factor) will come out right.

Finally if you change osc_freq, you can produce an equally valid implementation. However, you need to take care that for positive fine tune, we commonly assume that cent factors are > 1. Then, however, the osc_freq you would achieve when multiplying it with the fine tune factor would be larger than before. So, in our interpretation of the osc_freq, your interpretation would claim that the note was recorded at a higher frequency than normally. But that would make beast replay the sample slower, to compensate for this. That is: positive fine tune would actually make beast pitch down.

Since that is not intuitive, you'd probably want to divide osc_freq by the fine tune factor. But then your implementation would be mathematically equivalent to the code I proposed, as both affect step_factor in exactly the same way.

> >Index: bse/ChangeLog
> >===================================================================
> >--- bse/ChangeLog	(revision 4344)
> >+++ bse/ChangeLog	(working copy)
> >@@ -1,3 +1,19 @@
> >+Sun Jun 24 16:01:58 2007  Stefan Westerfeld  <stefan@space.twc.de>
> >+
> >+	* bsemathsignal.[hc]: Implemented bse_fine_tune_factor, which is
> >+	similar to bse_cent_factor, but accepts arbitary doubles as input
> >+	because it doesn't use a table internally.
> >+
> >+	* gsldatahandle.[hc]: Implemented gsl_data_handle_fine_tune, which
> >+	returns a fine tuning measured in cent ("fine-tune" xinfo).
> 
> the cent scaling is designed to adjust pitches beyond the hearing threshold, so
> i don't really think adding a bse_fine_tune_factor function is neccessary,
> indexing with a rounded value into the cent table should be good enough.

The table doesn't work for fine-tune > 100 or fine-tune < - 100, which by the xinfo specification is perfectly legal. Actually I wouldn't have bothered to write the function if it existed with an unrestricted signed integer argument.

On the other hand, its implementation is one pretty straightforward line of code, so its not as if we're adding something really problematic here.

> >+
> >+	* gslwavechunk.[hc]: Cache the fine tune factor here, so that
> >+	GslWaveOsc can access it quickly.
> >+
> >+	* gslwaveosc.c (gsl_wave_osc_set_filter): Take fine tune factor of the
> >+	chunk into account for step computation (and thus for filter
> >+	computation).
> >+
> > Sun Jun 24 00:17:40 2007  Stefan Westerfeld  <stefan@space.twc.de>
> > 
> > 	* bsemath.h: Added ln (10)/20 constant for ArtsCompressor.
> >Index: bse/bsemathsignal.c
> >===================================================================
> >--- bse/bsemathsignal.c	(revision 4341)
> >+++ bse/bsemathsignal.c	(working copy)
> >@@ -213,6 +213,12 @@ static const double cent_table201[100 + 
> > };
> > const double * const bse_cent_table = cent_table201 + 100;
> > 
> >+double
> >+bse_fine_tune_factor (double fine_tune)
> >+{
> >+  return exp (fine_tune * BSE_LN_2_POW_1_DIV_1200_d);
> >+}
> >+
> 
> as mentioned above, i hope this function is not really neccessary.
> 
> > 
> > /* --- musical tuning systems --- */
> > #define SCALED_INTERVAL(scale, F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12)       \
> >Index: bse/bsemathsignal.h
> >===================================================================
> >--- bse/bsemathsignal.h	(revision 4341)
> >+++ bse/bsemathsignal.h	(working copy)
> >@@ -402,6 +402,18 @@ double        bse_transpose_factor      
> > extern const double * const bse_cent_table;
> > #define	bse_cent_factor(index /* -100..+100 */)		(bse_cent_table[index])
> > 
> >+/**
> >+ * @param fine_tune	fine tuning in cent
> >+ * @return		a factor corresponding to this
> >+ *
> >+ * This function computes a factor which corresponds to a given fine tuning in
> >+ * cent.  The result can be used as factor for the frequency or the play speed.
> >+ * It is similar to the bse_cent_factor macro; however, it doesn't use a table
> >+ * internally, so the input is not limited in any way (i.e. 777.777 is a valid
> >+ * fine_tune argument).
> >+ */
> 
> please use the .c files for function documentation, header files shouldn't be
> cluttered with inline docs.

Ok. Will do as soon as the fate of the function is decided.

> >+double  bse_fine_tune_factor (double fine_tune);
> >+
[...]
Comment 4 Stefan Westerfeld 2007-08-24 17:24:33 UTC
Created attachment 94280 [details] [review]
Updated "fine tune" xinfo implementation

I changed the following things:
* removed example for fractional fine tune, in favour of -50 == half a semitone example
* added an extra #include "bsemathsignal.h" (I don't know why this missing include didn't cause problems), since bse_fine_tune_factor would be undefined in bse/gslwavechunk.c otherwise
* moved bse_fine_tune_factor documentation from bsemathsignal.h to bsemathsignal.c
* mentioned that its less efficient than the table based macro (though that should be clear)
Comment 5 Tim Janik 2007-10-28 17:51:55 UTC
(In reply to comment #4)
> Created an attachment (id=94280) [edit]
> Updated "fine tune" xinfo implementation

>+ * It is similar to the bse_cent_factor macro; however, it doesn't use a table

you need to add parenthesis to functions and macros in docs for the docu extraction tools to recognize function names: bse_cent_factor().
and 'macro' shouldn't be mentioned here, the impl of bse_cent_factor() may very well be changed to a function at any point.

also, we can't introduce bse_fine_tune_factor() for fractional cents, when we have 
bse_cent_factor() and BSE_FINE_TUNE_FACTOR() for integer cent steps already.
finally, the patch misses a test, see bse/tests/misctests.cc where we e.g. have one for bse_cent_factor() already.

as for the API names, it looks like we're getting too many names for the same thing here, so we need some consolidation here. e.g.:
- rename bse_cent_factor() to bse_cent_tune_fast(), document that it uses ints.
- rename bse_fine_tune_factor() to bse_cent_tune(), document that it's slow but supports fractions.
- use bse_cent_tune_fast() instead of BSE_FINE_TUNE_FACTOR(), make sure bse_cent_tune_fast() performs the CLAMP-ing currently done by BSE_FINE_TUNE_FACTOR().
Comment 6 Stefan Westerfeld 2007-10-31 21:40:07 UTC
Created attachment 98277 [details] [review]
[PATCH] Added necessary code for "fine-tune" xinfo, bug #450490.


* bsemathsignal.[hc]: Implemented bse_cent_tune, which is similar to
bse_cent_factor, but accepts arbitary doubles as input because it
doesn't use a table internally.

* gsldatahandle.[hc]: Implemented gsl_data_handle_fine_tune, which
returns a fine tuning measured in cent ("fine-tune" xinfo).

* gslwavechunk.[hc]: Cache the fine tune factor here, so that
GslWaveOsc can access it quickly.

* gslwaveosc.c (gsl_wave_osc_set_filter): Take fine tune factor of
the chunk into account for step computation (and thus for filter
computation).
---
 bse/ChangeLog       |   18 ++++++++++++++++++
 bse/bsemathsignal.c |   16 ++++++++++++++++
 bse/bsemathsignal.h |    2 ++
 bse/gsldatahandle.c |   13 +++++++++++++
 bse/gsldatahandle.h |    1 +
 bse/gslwavechunk.c  |    6 +++++-
 bse/gslwavechunk.h  |    7 ++++---
 bse/gslwaveosc.c    |    2 +-
 8 files changed, 60 insertions(+), 5 deletions(-)
Comment 7 Stefan Westerfeld 2007-10-31 21:40:10 UTC
Created attachment 98278 [details] [review]
[PATCH] * docs/architecture.doxi: Documented the new "fine-tune" xinfo.

 ChangeLog              |    4 ++++
 docs/architecture.doxi |    7 +++++++
 2 files changed, 11 insertions(+), 0 deletions(-)
Comment 8 Stefan Westerfeld 2007-10-31 21:40:13 UTC
Created attachment 98279 [details] [review]
[PATCH] * tests/misctests.cc: Added test for bse_cent_tune().

 bse/tests/misctests.cc |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
Comment 9 Stefan Westerfeld 2007-10-31 21:40:16 UTC
Created attachment 98280 [details] [review]
[PATCH] * tests/misctests.cc: Added test for bse_cent_tune().

 bse/ChangeLog |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Comment 10 Stefan Westerfeld 2007-10-31 21:40:19 UTC
Created attachment 98281 [details] [review]
[PATCH] Merge BSE_FINE_TUNE_FACTOR() and bse_cent_factor(), bug #450490.


* bse/bseglobals.h:
* bsemathsignal.[hc]: Merge BSE_FINE_TUNE_FACTOR() and
bse_cent_factor() to bse_cent_tune_fast(). The resulting function
automatically clamps its input and uses the cent table to compute its
result.

* bse/bseinfo.c:
* bse/bsenote.cc:
* bse/bsepart.h:
* bse/bsesong.proc:
* bse/gsloscillator-aux.c:
* bse/tests/misctests.cc: Use bse_cent_tune_fast() where appropriate.
---
 bse/ChangeLog           |   17 +++++++++++++++++
 bse/bseglobals.h        |    1 -
 bse/bseinfo.c           |    2 +-
 bse/bsemathsignal.c     |    6 +++---
 bse/bsemathsignal.h     |   23 ++++++++++++++++++++---
 bse/bsenote.cc          |    2 +-
 bse/bsepart.h           |    2 +-
 bse/bsesong.proc        |    2 +-
 bse/gsloscillator-aux.c |    2 +-
 bse/tests/misctests.cc  |    8 ++++----
 10 files changed, 49 insertions(+), 16 deletions(-)
Comment 11 Stefan Westerfeld 2007-10-31 21:40:22 UTC
Created attachment 98282 [details] [review]
[PATCH] Use bse_cent_tune_fast() where appropriate, bug #450490.


* davxtalstrings.c:
* davorgan.c: Use bse_cent_tune_fast() which replaces
bse_cent_factor().
---
 plugins/ChangeLog        |    8 ++++++++
 plugins/davorgan.c       |    2 +-
 plugins/davxtalstrings.c |    2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)
Comment 12 Stefan Westerfeld 2007-11-05 15:31:51 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=94280) [edit]
> > Updated "fine tune" xinfo implementation
> 
> >+ * It is similar to the bse_cent_factor macro; however, it doesn't use a table
> 
> you need to add parenthesis to functions and macros in docs for the docu
> extraction tools to recognize function names: bse_cent_factor().
> and 'macro' shouldn't be mentioned here, the impl of bse_cent_factor() may very
> well be changed to a function at any point.

Ok.

> also, we can't introduce bse_fine_tune_factor() for fractional cents, when we
> have 
> bse_cent_factor() and BSE_FINE_TUNE_FACTOR() for integer cent steps already.
> finally, the patch misses a test, see bse/tests/misctests.cc where we e.g. have
> one for bse_cent_factor() already.

Ooops. I didn't know about the macro but you are right.

> as for the API names, it looks like we're getting too many names for the same
> thing here, so we need some consolidation here. e.g.:
> - rename bse_cent_factor() to bse_cent_tune_fast(), document that it uses ints.
> - rename bse_fine_tune_factor() to bse_cent_tune(), document that it's slow but
> supports fractions.
> - use bse_cent_tune_fast() instead of BSE_FINE_TUNE_FACTOR(), make sure
> bse_cent_tune_fast() performs the CLAMP-ing currently done by
> BSE_FINE_TUNE_FACTOR().

Yes, thats a good idea. I did implement your suggestions for cleaning up the API as proposed in the recent git-send-bugzilla upload.
Comment 13 Tim Janik 2007-11-05 16:05:25 UTC
(In reply to comment #10)
> Created an attachment (id=98281) [edit]
> [PATCH] Merge BSE_FINE_TUNE_FACTOR() and bse_cent_factor(), bug #450490.


thanks, looks good. this patch can go in, allthough the TSTART() name in misctests.cc could be shorter.
Comment 14 Tim Janik 2007-11-05 16:08:45 UTC
(In reply to comment #10)
> Created an attachment (id=98281) [edit]
> [PATCH] Merge BSE_FINE_TUNE_FACTOR() and bse_cent_factor(), bug #450490.

this one is also good to go in (and must be comitted once attachment (id=98281) is applied.
Comment 15 Stefan Westerfeld 2007-11-05 16:48:13 UTC
(In reply to comment #13)
> (In reply to comment #10)
> > Created an attachment (id=98281) [edit]
> > [PATCH] Merge BSE_FINE_TUNE_FACTOR() and bse_cent_factor(), bug #450490.
> 
> 
> thanks, looks good. this patch can go in, allthough the TSTART() name in
> misctests.cc could be shorter.
> 

Committed:

    Merge BSE_FINE_TUNE_FACTOR() and bse_cent_factor(), bug #450490.
    
    * bse/bseglobals.h:
    * bsemathsignal.[hc]: Merge BSE_FINE_TUNE_FACTOR() and
    bse_cent_factor() to bse_cent_tune_fast(). The resulting function
    automatically clamps its input and uses the cent table to compute its
    result.
    
    * bse/bseinfo.c:
    * bse/bsenote.cc:
    * bse/bsepart.h:
    * bse/bsesong.proc:
    * bse/gsloscillator-aux.c:
    * bse/tests/misctests.cc: Use bse_cent_tune_fast() where appropriate.
Comment 16 Stefan Westerfeld 2007-11-05 16:50:52 UTC
(In reply to comment #14)
> (In reply to comment #10)
> > Created an attachment (id=98281) [edit]
> > [PATCH] Merge BSE_FINE_TUNE_FACTOR() and bse_cent_factor(), bug #450490.
> 
> this one is also good to go in (and must be comitted once attachment (id=98281) [edit]
> is applied.
> 

I think you wanted to say that I should apply the patch which is necessary to make the thing build again after the merger, which I committed too:

    Use bse_cent_tune_fast() where appropriate, bug #450490.
    
    * davxtalstrings.c:
    * davorgan.c: Use bse_cent_tune_fast() which replaces
    bse_cent_factor().
Comment 17 Tim Janik 2007-11-05 17:16:39 UTC
thanks for comitting. the remeining patches need reworking though:
- [PATCH] Added necessary code for "fine-tune" xinfo, bug #450490.
  this one needs renaming
- [PATCH] * docs/architecture.doxi: Documented the new "fine-tune" xinfo.
  this one should be merged with previous.
- [PATCH] * tests/misctests.cc: Added test for bse_cent_tune().
  hrm, changelog updates as single patch/comment.
it is tedious enough already to refer to individual patches with bugzilla. please merge your patches and add recent updates before submitting them. for review, the number of patch sets should be minimized,each patch set should be self sustained and they should be grouped by feature. bug reports shouldn't be abused for requesting others to review all of your intermediate local history.
Comment 18 Stefan Westerfeld 2007-11-05 18:03:21 UTC
Created attachment 98580 [details] [review]
Updated implementation of fine-tune xinfo

Ok, here again as single patch the "fine-tune" xinfo code.
Comment 19 Tim Janik 2007-11-15 20:55:46 UTC
(In reply to comment #18)
> Created an attachment (id=98580) [edit]
> Updated implementation of fine-tune xinfo
> 
> Ok, here again as single patch the "fine-tune" xinfo code.

thanks, committed with some additional docu fixes.