ssj71 / OSC2MIDI

A highly flexible and configurable OSC to JACK MIDI (and back) bridge
GNU General Public License v3.0
45 stars 7 forks source link

Fix various corner cases in conversions #7

Closed agraef closed 9 years ago

agraef commented 9 years ago

This pull request deals with various rather obscure but nevertheless important corner cases which I think aren't handled properly right now.

Commit 776e7c3 improves the handling of note commands, so that these are printed and matched properly. It does this by turning note commands with constant 4th argument into regular 3-argument noteon/noteoff commands, so that the cases of a variable or constant note command can be readily detected.

Commit cf7e3eb deals with arg conditioning with a zero scaling factor in rules like these:

/1/fader1 f, val*0+1 : ...

I'd argue that the mathematically correct way to interpret this is:

/1/fader1 f, 1 : ...

That is, a conditioned argument with a zero scale factor should be treated as a constant which equals the offset in the conditioning. This is what a MIDI->OSC conversion would give anyway, no matter what finite value val is bound to. Conversely, in OSC->MIDI conversions, the OSC argument ought to be matched against 1 and val isn't well-defined so it might as well be bound to any value or not be bound at all. Analogous reasoning also applies to the right-hand side of a rule, of course. Commit cf7e3eb implements this behavior (for both sides of a rule) and also emits a warning message when a zero scale factor is detected.

The remaining commits all deal with multiple occurrences of a variable on the left- or right-hand side of a rule.

Commit a4ca9f2 corrects OSC->MIDI conversions when there are multiple occurrences of a variable on the right-hand side of a rule, such as:

/1/fader1 f, val : noteon( 0, 127*val, 64*val );

Previously, the value of the variable would only be inserted into one of the instances on the right-hand side. Commit a4ca9f2 makes sure that we iterate over all instances of the same variable in the midi message and apply the proper conditioning to each of them.

Commit 0d895bc is a little cosmetic change which ensures that a left-hand side variable is mapped to its first (rather than the last) occurrence on the right-hand side. This was already the case for midi->osc mappings, so I just made sure that the osc->midi mappings behave the same.

Commit f3e2ddc is concerned with matching multiple occurrences of the same variable on either the left-hand side of a rule (in osc->midi conversions) or on the right-hand side (in midi->osc conversions). Consider a rule like this:

/2/xy1 ff, val, val : controlchange( 1, 127*val, 64*val );

In term rewriting we call these "non-linearities". Currently there isn't any check whether the two occurrences of val on either side are matched to the same value, so it's up to anyone's guess which of the two possible values val will be bound to when matching either the lhs against an OSC or the rhs against a MIDI message. IMHO the mathematically correct way to deal with this is to just check the values of both occurrences against each other and only match the rule if they are the same. This is what commit f3e2ddc implements.

Commit f3e2ddc, while it implements matching of multiple occurrences of the same variable in the mathematically correct way, might not be what we want, though. If you leave it out, multiple occurrences will be ignored for matching purposes, and the variable will be bound to the value at its first occurrence (commit 0d895bc above makes sure that it does this consistently for both osc->midi and midi->osc conversions). This also makes kind of sense and osc2midi users could certainly live with it if this behavior is properly documented IMHO.

ssj71 commented 9 years ago

Thanks for these good patches. I really like the approach to the non-linear arguments, we could maybe make this behavior switchable if someone wants it the other way.

Also, especially thanks for the clear documenting of what the changes are for and comments in the code, I wouldn't have time to review these and figure them out otherwise!

agraef commented 9 years ago

I really like the approach to the non-linear arguments, we could maybe make this behavior switchable if someone wants it the other way.

Good idea. Expect another pull request for that.