Friday, October 14, 2011

Tests did their job!

Debugging a problem with a sustain pedal going down too early made me realize that I had changed the definition of the value of a signal before the first sample from zero to the first sample.  So that had the counter-intuitive effect that a pedal beginning at a certain time would actually be counted as always being down if it didn't explicitly start with zero.  So there are two solutions to that, either change the pedal call so it puts a zero sample at time 0 if there isn't already a previous sample, or change the various sample functions so a signal is considered to be 0 before the first sample.  The first way is clearly ad-hoc and gross and just delaying the problem in some other call when I've forgotten about this behaviour, so it's time to change the signal definition yet again.

I seemed to recall I had added that "take the first value" behaviour at some point for some specific reason, and some fiddling with darcs annotate I found a couple of patches from more than a year ago that fixed bugs when I forgot to update everything, but couldn't find the patch that originally introduced it.  Stupid.  I should always document choices like that.  Turns out the documentation for the Signal module was several design changes out of date, and still documented the previous zero behaviour.

While I didn't find the rationale, checking the patches meant I had the set of functions that would have to be changed in mind, so I was more confident I wouldn't miss something.  So I made the change and ran the tests.

This sort of subtle interpretation change is pretty scary because it yields no type errors, and no crashes, but just that the derivation will be incorrect in some subtle way, such as the original sustain pedal problem.  But fortunately, every time I track down some subtle derivation bug I added a test for it, and when I ran the tests, sure enough the pitch signal sharing function failed its test.

Two notes overlapping but with different pitches have different pitch curves, as expected.  If those pitch curves are parallel, they can share a channel.  However, if a pitch signal is considered to start with the note, and signals are zero before the first sample, it looks like the second note jumps from pitch 0 to its real pitch, which is not parallel!

But why does the overlap check even look at the pitch curve before the beginning of the second note?  It turns out there's a bit of time inserted called the "control lead time" so controls can be set a little in advance of the note start.  MIDI has no concept of simultaneity and a pitch bend sent at the same time as a note on will cause an audible artifact.  There are tests explicitly verifying that notes that are overlapping after taking the control lead time into account still can't share channels so it was definitely intentional, and it seems to still makes sense, but I think adding the lead time to the end of the decay of the previous note rather than subtracting from the start of the current note would achieve the same effect without having to look at the pitch signal before the note begins.

So, give it a lot of thought, make the two line change of moving an addition from one expression to another, write a comment explaining why, rerun the tests.  This is exactly the kind of subtle issue that without the presence of the tests there is absolutely no way I would have foreseen.

And I have to say, a 13 line function with 22 lines of comments, 7 arguments, and a long history of minor tweaks is definitely a sore point.  I've thought a lot at various points about how to make it easier to explain, clearer, less error-prone, but failed every time.

As an aside, I have a new method of debugging derivation issues lately.  I wrote a function to dump the score from the UI in the simplified haskell literal format that the test functions use to construct a test score.  Then copy and paste that into a test, and see if running it there reproduces the problem.  I write a little predicate to characterize the problem (e.g. this particular note is on the wrong channel) or just eyeball it, and start cutting out score to see what happens to the result.  It seems to work ok but is still more clumsy than I want.  Hopefully I can think of something better later.

Since often the problem is that something *sounds* wrong, and has to be turned into a series of messages that *look* wrong, I also wrote a synthesizer state emulator.  Given a list of messages and their times, it can signal things like stuck notes, who is overlapping who, who has pitch bends where, etc.  I haven't actually found this one very useful yet, but even if I don't use it much for testing it should eventually become part of the process to turn recorded MIDI back into high level score.

And so, the final practical result of all that debugging is that I can play the piece back and the pedal is finally in the right place.  But music being what it is, I've gotten rather used to the pedal going down earlier.  Is it better there or did I just get used to it?  Who knows...

No comments:

Post a Comment