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 364041 - Making the DavOrgan module testable
Making the DavOrgan module testable
Status: RESOLVED FIXED
Product: beast
Classification: Other
Component: plugins
v0.7.x
Other Linux
: Normal normal
: ---
Assigned To: Beast Maintainers
Beast Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-10-21 21:54 UTC by Stefan Westerfeld
Modified: 2006-10-21 23:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to remove quasi-random phases (947 bytes, patch)
2006-10-21 21:55 UTC, Stefan Westerfeld
none Details | Review
The testsong (8.10 KB, text/plain)
2006-10-21 21:57 UTC, Stefan Westerfeld
  Details

Description Stefan Westerfeld 2006-10-21 21:54:38 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.
Comment 1 Stefan Westerfeld 2006-10-21 21:55:41 UTC
Created attachment 75159 [details] [review]
Patch to remove quasi-random phases

This is required to make the test pass every time.
Comment 2 Stefan Westerfeld 2006-10-21 21:57:50 UTC
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
Comment 3 Tim Janik 2006-10-21 22:19:29 UTC
(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.
Comment 4 Stefan Westerfeld 2006-10-21 23:47:26 UTC
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.