GNOME Bugzilla – Bug 364041
Making the DavOrgan module testable
Last modified: 2006-10-21 23:47:26 UTC
I recently tried to develop a test for the DavOrgan module, and found out that the DavOrgan module doesn't reset its state between two notes in a song. This in turn yields to different output, which in turn makes the test I developed fail. The state consists of the phases of the various harmonics, so the missing state reset results in quasi-random phases; this is inaudible, and maybe even desired, because a real-world instrument would probably also not yield deterministic phases. However, as stated above, it makes tests fail. Attached is a patch which introduces a phase reset; applying it makes the test reproducable. If we however want to keep non-deterministic phases for the organ module I suggest the following (a) don't rely on implicit randomization of the phases, but use an explicit bse_*_rand*() call (b) allow disabling the randomization for tests, so that for tests the module will behave deterministic and have always zero-phase, as with my patch applied I'll attach the patch and test.
Created attachment 75159 [details] [review] Patch to remove quasi-random phases This is required to make the test pass every time.
Created attachment 75160 [details] The testsong Testsong; if you want to integrate it in the build system, add the following to tests/audio/Makefile.am: # DavOrgan module is a mono module, so we test only channel 0 (left) FEATURE_TESTS += davorgan-test EXTRA_DIST += davorgan.bse davorgan.ref davorgan-test: $(BSE2WAV) $(srcdir)/davorgan.bse $(@F).wav $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 0 --avg-spectrum --spectrum --avg-energy > $(@F).tmp $(BSEFCOMPARE) $(srcdir)/davorgan.ref $(@F).tmp --threshold 99.99 rm -f $(@F).tmp $(@F).wav
(In reply to comment #0) > The state consists of the phases of the various harmonics, so the missing state > reset results in quasi-random phases; this is inaudible, and maybe even > desired, because a real-world instrument would probably also not yield > deterministic phases. However, as stated above, it makes tests fail. > > Attached is a patch which introduces a phase reset; applying it makes the test > reproducable. If we however want to keep non-deterministic phases for the organ > module I suggest the following the patch looks good, except for a changelog entry that describs what it does and the additions mentioned below. > (a) don't rely on implicit randomization of the phases, but use an explicit > bse_*_rand*() call > (b) allow disabling the randomization for tests, so that for tests the module > will behave deterministic and have always zero-phase, as with my patch applied > > I'll attach the patch and test. you can use the g_rand_* family of functions to preserve the phase randomization. however for tests this needs to be done only conditionally, so use bse_main_args->allow_randomization to determine whether phases should be randomized. (In reply to comment #2) > Created an attachment (id=75160) [edit] > The testsong > > Testsong; if you want to integrate it in the build system, add the following to > tests/audio/Makefile.am: sure, we always apprechiate new tests. however the song needs to be renamed before adding it to SVN, since it's not a canonical davorgan BSE file. if you really can't come up with a good name, simply use something like organtest1.bse. and BSE2WAV will have to use --bse-disable-randomization to work with your patch.
Ok, I committed it now. (In reply to comment #3) > you can use the g_rand_* family of functions to preserve the phase > randomization. however for tests this needs to be done only conditionally, so > use bse_main_args->allow_randomization to determine whether phases should be > randomized. The version I committed checks for bse_main_args->allow_randomization. > sure, we always apprechiate new tests. however the song needs to be renamed > before adding it to SVN, since it's not a canonical davorgan BSE file. if you > really can't come up with a good name, simply use something like > organtest1.bse. Its called organsong.bse (similar to minisong.bse). > and BSE2WAV will have to use --bse-disable-randomization to work with your > patch. Yep. I put added that flag to BSE2WAV.