GNOME.org
 

Re: Party Monster audio test

On Thu, 25 May 2006, Stefan Westerfeld wrote:


  Hi!




Index: plugins/bsenoise.cc
===================================================================
RCS file: /cvs/gnome/beast/plugins/bsenoise.cc,v
retrieving revision 1.2
diff -u -p -r1.2 bsenoise.cc
--- plugins/bsenoise.cc	7 Mar 2005 07:40:39 -0000	1.2
+++ plugins/bsenoise.cc	24 May 2006 21:51:44 -0000
@@ -53,7 +53,8 @@ class Noise : public NoiseBase {
    {
      g_return_if_fail (n_values <= block_size()); /* paranoid */

-      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() % (noise_data->size() - n_values)]);
+      //ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() % (noise_data->size() - n_values)]);
+      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[0]);
    }
  };
public:
@@ -66,8 +67,12 @@ public:
    const int N_NOISE_BLOCKS = 20;
    noise_data.resize (block_size() * N_NOISE_BLOCKS);

+    GRand *random_generator = g_rand_new_with_seed (0xdeadbeaf);
+
    for (vector<gfloat>::iterator ni = noise_data.begin(); ni != noise_data.end(); ni++)
-      *ni = 1.0 - rand() / (0.5 * RAND_MAX);  // FIXME: should have class noise
+      *ni = g_rand_double_range (random_generator, -1, 1);
+
+    g_rand_free (random_generator);
  }


to do something like this, we need:
- a new bse option --static-random (or similar name) to switch off real random
  numbers and use deterministic streams instead
- a new framework that allowes to open up several new random data streams
  and get data from those
- use seperate random data streams everywhere we could possibly need
  deterministic results at some point (i.e. bsenoise and davxtalstrings.c,
  possibly other modules)
- possible future randomized behaviour (e.g.l implementation of a humanizer)
  will also have to honour --static-random


  void
  reset1()
Index: plugins/davsyndrum.c
===================================================================
RCS file: /cvs/gnome/beast/plugins/davsyndrum.c,v
retrieving revision 1.29
diff -u -p -r1.29 davsyndrum.c
--- plugins/davsyndrum.c	16 May 2006 10:20:47 -0000	1.29
+++ plugins/davsyndrum.c	24 May 2006 21:51:44 -0000
@@ -259,7 +259,7 @@ dmod_process (BseModule *module,
          /* trigger drum */
          dmod_trigger (dmod,
                        freq_in ? BSE_FREQ_FROM_VALUE (freq_in[i]) : dmod->params.freq,
-                        ratio_in ? ratio_in[1] : 1.0);
+                        ratio_in ? ratio_in[i] : 1.0); /* FIXME: <- bug? */
          spring_vel = dmod->spring_vel;
          env = dmod->env;
          freq_rad = dmod->freq_rad;


could you be bothered to elabortate here?
all i can gather from your mail is "bug?" and "I think I found a bug here,
and the fix can be committed right away". i'm sorry, but for me that isn't
particularly telling, so i'd apprechiate an explanation of why you think
this is buggy (and possible investigation of source history to figure how/
if a bug was introduced here). and why you think you know what the fix would
be.


Index: plugins/davxtalstrings.c
===================================================================
RCS file: /cvs/gnome/beast/plugins/davxtalstrings.c,v
retrieving revision 1.33
diff -u -p -r1.33 davxtalstrings.c
--- plugins/davxtalstrings.c	8 May 2006 01:37:22 -0000	1.33
+++ plugins/davxtalstrings.c	24 May 2006 21:51:44 -0000
@@ -303,11 +303,17 @@ xmod_trigger (XtalStringsModule *xmod,
  /* Add some snap. */
  for (i = 0; i < xmod->size; i++)
    xmod->string[i] = pow (xmod->string[i], xmod->tparams.snap_factor * 10.0 + 1.0);
+

+  GRand *random_generator = g_rand_new_with_seed (0xdeadbeaf);
+
  /* Add static to displacements. */
  for (i = 0; i < xmod->size; i++)
    xmod->string[i] = (xmod->string[i] * (1.0F - xmod->tparams.metallic_factor) +
-		       (bse_rand_bool () ? -1.0F : 1.0F) * xmod->tparams.metallic_factor);
+		       (g_rand_boolean(random_generator) ? -1.0F : 1.0F) * xmod->tparams.metallic_factor);
+
+  g_rand_free (random_generator);
+
  /* Set velocity. */
  for (i = 0; i < xmod->size; i++)
    xmod->string[i] *= xmod->tparams.trigger_vel;
Index: slowtests/audio/Makefile.am
===================================================================
RCS file: /cvs/gnome/beast/slowtests/audio/Makefile.am,v
retrieving revision 1.20
diff -u -p -r1.20 Makefile.am
--- slowtests/audio/Makefile.am	21 May 2006 18:04:12 -0000	1.20
+++ slowtests/audio/Makefile.am	24 May 2006 21:51:45 -0000
@@ -80,3 +80,13 @@ velocity-test:
	$(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum --spectrum --avg-energy >> $(@F).tmp
	$(BSEFCOMPARE) $(srcdir)/velocity.ref $(@F).tmp --threshold 99.99
	rm -f $(@F).tmp $(@F).wav
+
+# the BEAST demo song
+FEATURE_TESTS += partymonster-test
+EXTRA_DIST += partymonster.ref
+partymonster-test:
+	$(BSE2WAV) $(top_srcdir)/library/demo/partymonster.bse $(@F).wav
+	$(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 0 --avg-spectrum --spectrum --avg-energy --end-time  > $(@F).tmp
+	$(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum --spectrum --avg-energy --end-time >> $(@F).tmp
+	$(BSEFCOMPARE) $(srcdir)/partymonster.ref $(@F).tmp --threshold 99.99
+	rm -f $(@F).tmp $(@F).wav


below, you're saying 99.99% is too tight to be matched in the presence of
real noise. wouldn't something like 99.8% make sense here then, until
something like --static-random is implemented?


Index: tools/bsefextract.cc
===================================================================
RCS file: /cvs/gnome/beast/tools/bsefextract.cc,v
retrieving revision 1.28
diff -u -p -r1.28 bsefextract.cc
--- tools/bsefextract.cc	21 May 2006 01:24:11 -0000	1.28
+++ tools/bsefextract.cc	24 May 2006 21:51:54 -0000
@@ -176,16 +176,25 @@ struct Feature
  const char *description;
  bool        extract_feature;      /* did the user enable this feature with --feature? */

+  string
+  double_to_string (double data) const
+  {
+    char *x = g_strdup_printf ("%.17g", data);
+    string s = x;
+    g_free (x);
+    return s;
+  }
+
  void print_value (const string& value_name, double data) const
  {
-    fprintf (options.output_file, "%s = %f;\n", value_name.c_str(), data);
+    fprintf (options.output_file, "%s = %s;\n", value_name.c_str(), double_to_string (data).c_str());
  }

  void print_vector (const string& vector_name, const vector<double>& data) const
  {
    fprintf (options.output_file, "%s[%ld] = {", vector_name.c_str(), data.size());
    for (vector<double>::const_iterator di = data.begin(); di != data.end(); di++)
-      fprintf (options.output_file, " %f", *di);
+      fprintf (options.output_file, " %s", double_to_string (*di).c_str());
    fprintf (options.output_file, " };\n");
  }

@@ -210,7 +219,7 @@ struct Feature
	const vector<double>& line = *mi;

	for (vector<double>::const_iterator li = line.begin(); li != line.end(); li++)
-	  fprintf (options.output_file, " %f", *li);
+	  fprintf (options.output_file, " %s", double_to_string (*li).c_str());
	fprintf (options.output_file, " }\n");
      }
    fprintf (options.output_file, "};\n");


i think you can commit that right away.


Some comments on the patch:
* eliminating random data in bsenoise.cc doesn't change the similarity
  score too much, but is of course necessary for 100% matches
* davsyndrum.c: I think I found a bug here, and the fix can be
  committed right away
* the main randomness problem really is in the davxtalstrings.c module,
  after eliminating this source of random, my compare runs were much
  more similar than before

As for the last change, I think it might be useful to increase the
precision with which bsefextract outputs its extracted features. As a
user trying to read the feature file with an editor, I dislike using the
%.17g format, because its much harder to read than %f.


i can't say i agree here. i much prefer 1.3e-69 (%g) over
  0.0000000000000000000000000000000000000000000000000000000000000000000013
(%f). and for figures with smaller exponent, %g equals %f anyway.


The other thing
that I dislike about it as a casual user is that matrix features
(--spectrum) are not aligned any more, that is, the numbers of two
different lines no longer start at the same column.


huh? but that can easily be fixed, have you tried something like %15.9g?


However, as a developer, I see that %f and %.17g features are so
different that changing the bsefextract output from one to another
actually breaks some existing tests. This leads me to the conclusion
that %f's implicit quantization could have the same effect in some
real world scenarios (i.e. 0.000003 vs. 0.000002 in a spectrum feature
could bring similarity below the threshold, although the "real" distance
- the "double" distance before writing the feature file - between both
extracted features might be much smaller than 0.000001).


%f can't be used to properly represent a double. at least not with much
less than 600 character strings. that's why our serialization functions
which can't allow for data loss are using:
  if (g_option_check (hints, "f"))      /* float hint */
    gstring_puts (gstring, g_ascii_formatd (numbuf, G_ASCII_DTOSTR_BUF_SIZE, "%.7g", sfi_value_get_real (value)));
  else
    gstring_puts (gstring, g_ascii_formatd (numbuf, G_ASCII_DTOSTR_BUF_SIZE, "%.17g", sfi_value_get_real (value)));
and why we have extra API to write out flots to data streams:
  sfi_wstore_putf();
  sfi_wstore_putd();
speaking of which, you need to change your code to use those anyway, because currently the reference files will break in non-C locales. 


  Cu... Stefan


---
ciaoTJ
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]