Re: Fixes to midi2ly.py: better note lengths, now supports meter changes (issue 325800043 by pkx1...@gmail.com)
On Sat, May 27, 2017 at 4:00 AM, wrote:
> Reviewers: ,
>
> Message:
> This fails a basic 'make' when applied to current master.
It works for me.
> 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
Okay, I'll take a look.
--- Chris
> regards
>
> Description:
> 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 4*61/120 is changed to 4*1/2 and then
> to 8. (2) Current version of midi2ly does not support meter changes.
> Patch does.
>
> Resolves: #5119
>
> Please review this at https://codereview.appspot.com/325800043/
>
> Affected files (+87, -50 lines):
> M scripts/midi2ly.py
>
>
--
==
Christopher Carl Heckman
Honors Faculty
Lecturer
33° 25' 7'' N, 111° 55' 55'' W
ECA 212
School of Mathematical & Statistical Sciences
Arizona State University (T1N R4E Sect22 NW1/4)
Tempe, AZ 85287 - 1804
United States of America
North America
Planet Earth
Solar System
Gould Belt
Local Bubble
Local Fluff
Milky Way Galaxy, Orion Arm
Local Group
Virgo Supercluster
Pisces-Cetus Supercluster Complex
Universe
Multiverse
==
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)
On 2017/05/27 20:03:43, horndude77 wrote: > On 2017/05/21 17:12:26, thomasmorley651 wrote: > > I'd like to mention another point: > > What to do with pitched rests and rests with user-set staff-position, merge > them > > automatically to the zero-position? > > If a user has explicitly set the position of a rest this should be honoured > by default, I think ... Done. If the staff-position property is set it won't attempt to merge rests at that position. Seems to work well. > > I'd say using suspendRestMerging-property is sufficient to cover this case, > but > > this is only me. Other opinions? > > ... unless some property (mergePitchedRests?) is set true. I didn't create another property for overriding this behavior. Merging pitched rests could behave unexpectedly for some users - should one of the merged rests pitches be respected or should they be at the single voice position? Also, I'd hate to add a property like this which I'd expect to be rarely used. It can be worked around with tags (i.e. pitched rest for the single part, normal rest for combined part). If in the future there's a need it could be added when the use cases are better understood. We have a similar situation regarding the directions of slurs. In this case, explicit different directions (in the SlurEvent expression, not in the generated Slur grob) lead to both slurs being retained. I think it reasonable to retain all unique pitched rests, and when there are none, a single unpitched rest. Is this useful? I don't really know. https://codereview.appspot.com/321930043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)
On 2017/05/21 17:12:26, thomasmorley651 wrote: > I'd like to mention another point: > What to do with pitched rests and rests with user-set staff-position, merge them > automatically to the zero-position? If a user has explicitly set the position of a rest this should be honoured by default, I think ... Done. If the staff-position property is set it won't attempt to merge rests at that position. Seems to work well. > I'd say using suspendRestMerging-property is sufficient to cover this case, but > this is only me. Other opinions? ... unless some property (mergePitchedRests?) is set true. I didn't create another property for overriding this behavior. Merging pitched rests could behave unexpectedly for some users - should one of the merged rests pitches be respected or should they be at the single voice position? Also, I'd hate to add a property like this which I'd expect to be rarely used. It can be worked around with tags (i.e. pitched rest for the single part, normal rest for combined part). If in the future there's a need it could be added when the use cases are better understood. https://codereview.appspot.com/321930043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fixes to midi2ly.py: better note lengths, now supports meter changes (issue 325800043 by pkx1...@gmail.com)
Reviewers: ,
Message:
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
regards
Description:
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 4*61/120 is changed to 4*1/2 and then
to 8. (2) Current version of midi2ly does not support meter changes.
Patch does.
Resolves: #5119
Please review this at https://codereview.appspot.com/325800043/
Affected files (+87, -50 lines):
M scripts/midi2ly.py
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
PATCHES - Countdown for May 26th
Hello, Here is the current patch countdown list - slightly late, I apologise for that. The next countdown though will be on May 29th. A quick synopsis of all patches currently in the review process can be found here: http://philholmes.net/lilypond/allura/ Push: 1388 Support OpenType font features - Jay Anderson https://sourceforge.net/p/testlilyissues/issues/1388 http://codereview.appspot.com/323850043 Countdown: 5139 abc2ly: Support R (rhythm / meter) and Z (transcription) fields - Paul Morris https://sourceforge.net/p/testlilyissues/issues/5139 http://codereview.appspot.com/324890043 Review: No patches in Review at this time. New: 5119 MIDI2Ly fraction reduction - Christopher Heckman (via James Lowe) https://sourceforge.net/p/testlilyissues/issues/5119 http://codereview.appspot.com/325800043 Regards James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Lilypond: Two Fixes for midi2ly
Hello Christopher, On Fri, 26 May 2017 19:54:31 -0500 Christopher Heckman wrote: > A few weeks ago, I brought up some issues concerning midi2ly. I have > made changes and created a patch containing both of them. > > (1) Because of calculation errors, "clean" midi files sometimes > produce durations like 4*61/120. The first fix looks for a better > fraction close to 61/120, namely 60/120 or 62/120 that reduces as much > as possible. If there is a factor of 2 in the denominator, the first > part of the duration is modified. For example, 4*61/120 becomes 8 > (being 4*1/2 en route). > > (2) There is a comment in midi2ly about the code "barfing" if there is > a meter change. The patch also features a fix for the barfing bug: It > keeps track of the current meter and calculates the measure number > based on that. > > I know that I probably should have split these up, but in the > debugging process, I did (1) and (2) before knowing about git. > > --- Christopher Heckman Thank you for the patch, it is now in the Patch testing queue. has now been assigned to both the tracker and a Rietveld issue (where I have added you as a CC for any comments/reviews you may need to address). I'll shepherd this through the testing and review process for you, although any changes that may need to be made to the patc, as it is reviewed by the other Devs, will have to be addressed directly by yourself (i.e. submit a new patch and attach it to the tracker). Thank you for contribution so far. James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
