LilyIssues / Issues / #5119 [enhancement] midi2ly.py better note lengths, now supports meter changes

Colin Campbell - 2017-04-16

Christopher's explanation of his work:
This will reduce the fractions (n-1)/d, n/d, and (n+1)/d (where the
original fraction is n/d). If (n-1)/d or (n+1)/d reduces more, its
reduction replaces n/d, except in one technical case. For instance,
the fraction 29/120 is considered to be rounded off after some error
has been added. The possible values to be checked are 28/120 (7/30 in
lowest terms), 29/120 (which does not reduce), and 30/120 (1/4). Since
30/120 reduces to the smallest denominator, 29/120 gets replaced with
1/4.

Now the technical case: If n/d is in lowest terms, n is odd, and d is
even but not divisible by 2, then n/d won't reduce, but you will be
able to cancel a factor of 2 from (n-1)/d and (n+1)/d. Since there is
no way to break this tie, the original value n/d is kept. (Note this
is the only case where (n-1)/d and (n+1)/d will be reduced by the same
denominator. I've proven this.)

Note that this will not make changes like 4*1/4 into 16; that might
be in a future version. This would turn the example output into

r2. e16 r16 e8 r2. e16 r16 e8 r2. e16

Also, fractions which don't reduce but have large denominators might
be replaced with approximations with small denominators. (Continued
fractions allow you to calculate these.)

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Phil Holmes - 2017-04-23

status: New --> Accepted
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2017-05-27

Fixes to midi2ly.py: better note lengths, now supports meter changes

(1) Note lengths currently are fractions like 61/120. If the numerator
can be changed by 1 to get a better fraction, it is done. Quarter
notes are also replaced with eighths/sixteenths if appropriate.
For example: a duration of 461/120 is changed to 41/2 and then
to 8. (2) Current version of midi2ly does not support meter changes.
Patch does.

Resolves: #5119

http://codereview.appspot.com/325800043

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Description has changed:

Diff:

--- old
+++ new
@@ -9,37 +9,18 @@

 r2. e4*1/4 r4*1/4 e4*1/2 r2. e4*1/4 r4*1/4 e4*1/2 r2. e4*1/4

-I have written a patch to fix this. Inside of the class Duration (line
-129), replace the dump function with:
+I have written a patch to fix this. 

----
+--
+Fixes to midi2ly.py: better note lengths, now supports meter changes


-    def dump (self):
-        den = self.den; num = self.num;
-        # code below by C C Heckman
-        # looks for a neighboring fraction that reduces
-        if num > 1:
-            gcd1 = gcd (num - 1, den)
-            gcd2 = gcd (num, den)
-            gcd3 = gcd (num + 1, den)
-            if gcd1 > gcd2 and gcd1 > gcd3:
-                d = -1; g = gcd1
-            elif gcd3 > gcd2 and gcd3 > gcd1:
-                d = +1; g = gcd3
-            else:
-                d = 0; g = gcd2
-                # for now, do nothing
-            num = (num + d) / g
-            den = den / g
-        if den == 1:
-            if num == 1:
-                s = '%d' % self.dur
-            elif num == 3 and self.dur != 1:
-                s = '%d.' % (self.dur / 2)
-            else:
-                s = '%d*%d' % (self.dur, num)
-        else:
-            s = '%d*%d/%d' % (self.dur, num, den)
-        # end modified code
+(1) Note lengths currently are fractions like 61/120. If the numerator
+can be changed by 1 to get a better fraction, it is done. Quarter
+notes are also replaced with eighths/sixteenths if appropriate.
+For example: a duration of 461/120 is changed to 41/2 and then
+to 8. (2) Current version of midi2ly does not support meter changes.
+Patch does.

------
+Resolves: #5119
+
+http://codereview.appspot.com/325800043

assigned_to: pkx166h
Needs: -->
Type: --> Enhancement

Anonymous - 2017-05-27

Patch: new --> needs_work
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2017-05-27

This fails a basic 'make' when applied to current master.

Also Christopher, if and when you submit a new patch, can you clean up the whitespace errors that are also reported ('git apply' squelches some, but I can see that you are using tabs in the diff on Rietveld and this will cause comments by other devs). Also see:

http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#policy-decisions-_0028finished_0029

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Anonymous - 2017-08-12

summary: MIDI2Ly fraction reduction --> [enhancement] midi2ly.py better note lengths, now supports meter changes

Description has changed:

Diff:

--- old +++ new @@ -21,6 +21,4 @@ to 8. (2) Current version of midi2ly does not support meter changes. Patch does. -Resolves: #5119 - http://codereview.appspot.com/325800043

status: Started --> Accepted

assigned_to: pkx166h --> nobody

Patch: needs_work --> abandoned
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

[enhancement] midi2ly.py better note lengths, now supports meter changes

Issue Tracker for LilyPond

Searches

Help

#5119 [enhancement] midi2ly.py better note lengths, now supports meter changes

Discussion