Closed jstma closed 1 year ago
aspiers
commented
1 year ago The /tmp/ly2videoXXXXXXX directory is the one with the important debug; I don't think you need to look at the temporary Lilypond files. Did you see https://github.com/aspiers/ly2video/blob/master/TROUBLE-SHOOTING.md yet? Hopefully that helps.
jstma
commented
1 year ago Yeah I've seen the troubleshooting, but it's not helpful in this case. I cut down my file to what I think is a minimum not-working example:
\version "2.20.0"
preVerse = \drummode {
<<
{ cymr8 cymr8 \tuplet3/2{hh16 hh hh} hh8 \tuplet3/2{tomh16 tomh tomh} \tuplet3/2{tomh16 tomh toml} \tuplet3/2{tomh16 tomfh tomh} tomfh8 }
\\
{ bd8 bd8 s4 s2 }
>>
}
theMusic = \drummode {
\time 4/4
\tempo 4 = 84
\preVerse
}
music = {
\new DrumStaff {
\set Staff.instrumentName = #"drums"
<<
\new DrumVoice { \theMusic }
>>
}
}
\score {
\music
}
\score {
\unfoldRepeats \music
\midi { }
}Okay I could have reduced the number of notes to make it more minimal, but I don't think that's the problem.
Does anything stand out as obviously not supported?
jstma
commented
1 year ago I copied the dump-spacetime-info.ly file from /tmp to my test code directory. I modified test.ly to contain:
\include "dump-spacetime-info.ly"
and when I ran lilypond, no errors were generated. The pdf looked normal. There was no output of "spacetime info" that I could see. Looking again at that troubleshooting page, it says there should be output to stdout. I didn't see any when I ran the lilypond command manually.
jstma
commented
1 year ago I noticed that dump-spacetime-info.ly did not contain entries for DrumStaff or DrumVoice so I added them by copying Voice and Staff:
\layout {
\context {
\DrumVoice
\override NoteHead #'after-line-breaking = #dump-spacetime-info
}
\context {
\DrumStaff
\override BarLine #'after-line-breaking = #dump-spacetime-info-barline
}
\context {
\Voice
\override NoteHead #'after-line-breaking = #dump-spacetime-info
}
\context {
\Staff
\override BarLine #'after-line-breaking = #dump-spacetime-info-barline
}
\context {
\ChordNames
\override ChordName #'after-line-breaking = #dump-spacetime-info
}
}However, when I run lilypond, it gives this error:
Drawing systems...dump-spacetime-info.ly:35:17: In procedure ly:pitch-octave in expression (ly:pitch-octave pitch):
dump-spacetime-info.ly:35:17: Wrong type argument in position 1 (expecting Pitch): ()I guess DrumVoice behaves differently than Voice.
aspiers
commented
1 year ago It's years since I worked on this, but looking at dump-spacetime-info.ly, I see it only generates output for NoteHead, BarLine, and ChordName. Would that explain the problem, if you're only processing unpitched percussion notes?
jstma
commented
1 year ago Unfortunately I don't know anything about lilypond internals. If this isn't something you can fix by some glaring omission, I doubt I'm going to be able to fix it without some hand holding.
Is there a reason NoteHead would be different for pitched and unpitched notes? Is there a more base class that both might derive from?
aspiers
commented
1 year ago That's the right question but I can't remember - however I do seem to remember it's not hard to find out from the LilyPond reference manual.
aspiers
commented
1 year ago I meant the internals docs not reference.
https://lilypond.org/doc/v2.23/Documentation/internals/drum_005fnotes_005fengraver suggests that DrumVoice creates NoteHead grobs.
aspiers
commented
1 year ago https://github.com/aspiers/ly2video/blob/baddaf4cac1e3eb24ac117fa67f4a0cde48f6560/ly2video/cli.py#L1335 assumes that the NoteHead has a pitch, however that might not be true with DrumVoice? I would suggest asking the LilyPond community, and also maybe adding some debug to the dump-spacetime-info function in the dumper to check.
jstma
commented
1 year ago I also hate scheme. I thought it was a waste of time at school and I'm shocked to see something actually written in it!
(format #t "\nly2video: (~23,16f, ~23,16f) pitch ~d:~a:~a @ ~23,16f from ~a:~3d:~d"
left right
(ly:pitch-octave pitch)
(ly:pitch-notename pitch)
(ly:pitch-alteration pitch)
(+ 0.0 (ly:moment-main time) (* (ly:moment-grace time) (/ 9 40)))
file line char))))Can you break down this function call? The error complains about
Wrong type argument in position 1 (expecting Pitch): ()It doesn't seem to like (ly:pitch-octave pitch) in position 1 according to the line number.
What function is complaining?
aspiers
commented
1 year ago Well that seems to confirm my suspicion: the code is assuming the grob has a valid pitch value, but it doesn't, so invoking the ly:pitch-octave function on it fails. The error says it was expecting a Pitch type (obviously, otherwise it wouldn't be possible to get the octave), but instead gets (). The next question is whether ly2video really needs to know the pitch, and if so, why.
aspiers
commented
1 year ago The next question is whether ly2video really needs to know the pitch, and if so, why.
If you search for "pitch" in https://github.com/aspiers/ly2video/blob/master/TROUBLE-SHOOTING.md you'll see that ly2video relies on pitch for ensuring the video frames are synchronised with the MIDI track. So this needs to work with drums too. However drum notes will still generate MIDI NoteOn events with pitch. So if those pitches are not available as part of the LilyPond grob's pitch property, they would need to be obtained from elsewhere. You should probably try using a normal non-drum voice just to confirm this is really the issue.
jstma
commented
1 year ago Based on the documentation, NoteHead doesn't have a pitch property or interface. I'd say the same thing for grobs. Where can I see that pitch is available via the grob?
How do the midi events align with multiple notes played at the same time that are not a chord?
aspiers
commented
1 year ago I think the pitch comes from music event properties - this seems to be a big hint:
https://lilypond.org/doc/v2.23/Documentation/internals-big-page#index-ly_003aevent_002dproperty https://lilypond.org/doc/v2.23/Documentation/internals-big-page#music-properties
Can't remember how multiple notes are handled.
Note the check for pitch here:
This could be reused to avoid crashing when pitch is missing.
jstma
commented
1 year ago Still trying to decipher this code:
(format #t "\nly2video: (~23,16f, ~23,16f) pitch ~d:~a:~a @ ~23,16f from ~a:~3d:~d"
left right
(ly:pitch-octave pitch)
(ly:pitch-notename pitch)
(ly:pitch-alteration pitch)
(+ 0.0 (ly:moment-main time) (* (ly:moment-grace time) (/ 9 40)))
file line char))))The first command is format. Does that make a formatted string? The arguments then are printed and reference another variable called pitch. That comes from
(pitch (ly:event-property cause 'pitch))and it's described as:
Function: ly:event-property sev sym val
Get the property sym of stream event sev. If sym is undefined, return val or '() if val is not specified.So this is a stream event. The object is cause:
(cause (ly:grob-property grob 'cause))The cause for a grob is:
cause (any type)
Any kind of causation objects (i.e., music, or perhaps translator) that was the cause for this grob.It seems to me that maybe there needs to be a check on what the cause is. Somehow it needs to be a note event?
jstma
commented
1 year ago I also came across this tidbit:
Function: expand-repeat-notes! music
Walk through music and give pitchless notes (not having a pitch in pitch or a drum type in drum-type) the pitch(es) from the predecessor note/chord if available. Which may indicate that drum-type is a substitute for pitch. Unfortunately I don't know what to do with this information. If I change:
(pitch (ly:event-property cause 'pitch))to:
(pitch (ly:event-property cause 'drum-type))I get an encouraging error:
Drawing systems...dump-spacetime-info.ly:61:17: In procedure ly:pitch-octave in expression (ly:pitch-octave pitch):
dump-spacetime-info.ly:61:17: Wrong type argument in position 1 (expecting Pitch): ridecymbalThis may seem like a stupid question, but the beatmap doesn't know about pitches. So why are pitches necessary for synchronization?
Plodding on,
If I patch cli.py to print out midiPitches
{51: midi.NoteOnEvent(tick=0, channel=9, data=[51, 90]), 36: midi.NoteOnEvent(tick=0, channel=9, data=[36, 90])}
Starting by hovering over the first grob
{51: midi.NoteOnEvent(tick=192, channel=9, data=[51, 90]), 36: midi.NoteOnEvent(tick=192, channel=9, data=[36, 90])}
Skipping grob and tick
{42: midi.NoteOnEvent(tick=384, channel=9, data=[42, 90])}
Skipping grob and tick
{42: midi.NoteOnEvent(tick=448, channel=9, data=[42, 90])}
Skipping grob and tick
{42: midi.NoteOnEvent(tick=512, channel=9, data=[42, 90])}
Skipping grob and tick
{42: midi.NoteOnEvent(tick=576, channel=9, data=[42, 90])}
Skipping grob and tick
{50: midi.NoteOnEvent(tick=768, channel=9, data=[50, 90])}
Skipping grob and tick
{50: midi.NoteOnEvent(tick=832, channel=9, data=[50, 90])}
Skipping grob and tick
{50: midi.NoteOnEvent(tick=896, channel=9, data=[50, 90])}
Skipping grob and tick
{50: midi.NoteOnEvent(tick=960, channel=9, data=[50, 90])}
Skipping grob and tick
{50: midi.NoteOnEvent(tick=1024, channel=9, data=[50, 90])}
Skipping grob and tick
{45: midi.NoteOnEvent(tick=1088, channel=9, data=[45, 90])}
Skipping grob and tick
{50: midi.NoteOnEvent(tick=1152, channel=9, data=[50, 90])}
Skipping grob and tick
{43: midi.NoteOnEvent(tick=1216, channel=9, data=[43, 90])}
Skipping grob and tick
{50: midi.NoteOnEvent(tick=1280, channel=9, data=[50, 90])}
Skipping grob and tick
{43: midi.NoteOnEvent(tick=1344, channel=9, data=[43, 90])}
Skipping grob and tick
sync points found: 1
from: 16 original indices
and: 17 original ticks
last tick used: 1
ticks skipped: 15
------------------------------------------------------------So it would seem that If I can stuff those midi note numbers into grobPitchValue somehow, then it would get farther along.
The midi map is available in lilypond:
But I don't know how to access midiDrumPitches. If this could be queried from dump-spacetime, the pitch could be grabbed right there. My scheme is not so good. The best I could figure out was:
(drum-type (ly:event-property cause 'drum-type))
(pitch (assoc drum-type midiDrumPitches))but the output is:
(hightom . #<Pitch d >)which isn't an integer. Do you know a way to convert the output of (assoc drum-type midiDrumPitches) to an integer that's needed?
jstma
commented
1 year ago Well I think I made it work and it was surprisingly easy:
(drum-type (ly:event-property cause 'drum-type))
(pitch (ly:assoc-get drum-type midiDrumPitches))combined with my previous inclusion of the DrumVoice and DrumStaff contexts. This could be improved by detecting the drum-type and then conditionally fetching the note directly from the cause or from midiDrumPitches. Though a second mostly duplicate dump-spacetime-info function for the DrumVoice context also works (and is what I did).
Now cli.py is happy and a video is generated. It mostly works, but as you can see there are some problems.
https://github.com/aspiers/ly2video/assets/87273793/7ef522e5-4de9-4402-b9fb-0a26774d786e
These are the same problems I saw running the tests under ly2video/test/regression.
https://github.com/aspiers/ly2video/assets/87273793/cb385359-bb98-4af5-a7bf-17d93411dc68
Namely there's black space to the left of the score, the page is the wrong size, and the cursor doesn't go to the end.
jstma
commented
1 year ago I've noticed that some other pull requests haven't been integrated yet. Both are related to some initial problems I had getting ly2video installed.
https://github.com/aspiers/ly2video/pull/106 https://github.com/aspiers/ly2video/pull/109
Both have merit. One makes the least amount of change, the other does away with having to install unnecessary packages. I'd say bring them both in. That way the github change to https gets captured, but then ly2video moves to less install dependencies which I'm in favour of.
I tested the change to mido + drum-support patches with all the regression tests. They all produced videos that seemed okay upon viewing (ignoring the problems I've already mentioned), but I'm not sure what the tests are supposed to look like. The text printed out to the screen is very noisy when running ly2video.
I also ran test.py which reported "Ok" after printing a bunch of text. I diffed the output of master vs mido + drum-support and the only difference was that the mido branch ran 0.008s faster.
I tested mido + drum-support on a full drum score and it worked as well as could be expected. There are some issues with the video as I mentioned previously, but it created a video where the red bar tracked all the notes (except for the last note).
Would you be able to review/merge those other pull requests?
And if you can spare some time to review my pull request, I'd like to see it integrated too.
aspiers
commented
1 year ago I'll take a look at those.
Did you try messing around with --width and --height? Might help fix the black space issue.
aspiers
commented
1 year ago I figured out some of the layout issues. One is that tagline needs to be set to ##f in the \header of the LilyPond source to avoid that version footer from appearing. ly2video does this automatically, unless \header is missing (see #30). However I added 5d78897 which I thought fixed this a different way, except now I can't get it to appear even when I revert that change.
Another is that the height defaults to 720px. It used to be that ly2video configured the paper top/bottom margins to 200px, so if your music was only (say) 150px high, the output image would be 550px which was too small to crop into an image with the desired 720px height. In this case ly2video would abort but offer a helpful error about how to use --dpi or --height to fix the problem. However #81 changed the top/bottom margins to be half the height which seems to guarantee this doesn't happen any more. I guess that's somewhat more user-friendly ... maybe.
The black space on the left is due to a similar reason, since left/right margins are still hard-coded to 200px. Setting --width to a smaller amount fixes it, but it's a bug that it creates black space instead of white.
jstma
commented
1 year ago Not all of the regression tests contain a \header. The one I added for DrumVoice does not because the regression test I used as an example also did not.
lilypond does not complain when there are duplicate '\header' blocks. The last one to appear will override any of those preceding. So it should be simple for ly2video to append a bit of text to the input file?
I'll play with --width and --height. The end application for me is to put scores on my phone with a metronome track (instead of midi) and it's an oddball size 1644 x 3840.
aspiers
commented
1 year ago Yes it could add a \header but I'm not currently seeing any evidence that we need to. Anyway please let's discuss that in #30 not here.
First time using ly2video. Installation was not straight forward (github doesn't allow git url access any more), but I found a way to get it done. ly2video works on the regression tests, but the output looks wrong. I don't know if that means I have a working installation or not.
I tried an old drum transcription and it failed to generate a video. The sanitised midi plays fine and the sanitised png has what looks like the whole score all in one line.
In the lilypond file I am using a DrumVoice to control line breaks in the pdf, but even if I comment it out the error persists. Not sure how to debug futher.
The --debug flag does not create a local directory (it still goes to /tmp) and despite supplying --keep, some of the temporary files are deleted.
The directory /tmp/ly2videobqt4hcex remains, but /tmp/lilypond-8LIXAW does not. I don't know if that's expected behaviour. There definitely is a bug there and I can report it in a different issue if there is any response to this one.
Back to this issue, here is the output of ly2video: