Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERROR: Didn't find any notes #113

Closed
jstma opened this issue May 11, 2023 · 23 comments · Fixed by #115
Closed

ERROR: Didn't find any notes #113

jstma opened this issue May 11, 2023 · 23 comments · Fixed by #115

Comments

@jstma
Copy link

jstma commented May 11, 2023

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.

$ ly2video -i counting-stars.ly --debug --keep
...
Running: lilypond --png -I /path/cleaned/counting-stars -dmidi-extension=midi -dresolution=110 -dpreview '-dprint-pages=#f' /tmp/ly2videobqt4hcex/converted.ly
...
Layout output to `/tmp/lilypond-8LIXAW'...
Converting to PNG...
Deleting `/tmp/lilypond-8LIXAW'...

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:

$ ly2video -i counting-stars.ly
LilyPond was found.
FFmpeg was found.
TiMidity++ was found.
------------------------------------------------------------
Version in counting-stars.ly: 2.18.2
Will convert to: 2.20.0
convert-ly (GNU LilyPond) 2.20.0

convert-ly: Processing `counting-stars.ly'... 
Applying conversion: 2.19.2, 2.19.7, 2.19.11, 2.19.16, 2.19.22, 2.19.24, 2.19.28, 2.19.29, 2.19.32, 2.19.40, 2.19.46, 2.19.49, 2.19.80, 2.20.0


Generating PNG and MIDI files ...
------------------------------------------------------------
GNU LilyPond 2.20.0
Processing `/tmp/ly2videop2zi9odv/converted.ly'
Parsing...
Interpreting music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120]
Preprocessing graphical objects...
Interpreting music...
MIDI output to `converted.midi'...
Finding the ideal number of pages...
Fitting music on 4 or 5 pages...
Drawing systems...
Layout output to `converted.preview.eps'...
Converting to PNG...
Deleting `converted.preview.eps'...
Success: compilation successfully completed
------------------------------------------------------------
Generated PNG and MIDI files
Looking for staff lines in converted.preview.png
First staff line found at (53, 0)
Found 5 staff lines
Margins in mm: left=46 top=83 right=46 bottom=83
Margins in px: left=200 top=360 right=200 bottom=360
Wrote sanitised version of /tmp/ly2videop2zi9odv/converted.ly into /tmp/ly2videop2zi9odv/sanitised.ly
Generating PNG and MIDI files ...
------------------------------------------------------------
GNU LilyPond 2.20.0
Processing `/tmp/ly2videop2zi9odv/sanitised.ly'
Parsing...
Interpreting music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120]
Preprocessing graphical objects...
Interpreting music...
MIDI output to `sanitised.midi'...
Layout output to `/tmp/lilypond-NH9WyR'...
Converting to PNG...
Deleting `/tmp/lilypond-NH9WyR'...
Success: compilation successfully completed
------------------------------------------------------------
Generated PNG and MIDI files
------------------------------------------------------------
ERROR: Didn't find any notes; something must have gone wrong with the use of dump-spacetime-info.

Sorry, ly2video has encountered a fatal bug as described above,
which it could not attribute to any known cause :-(

Please consider searching:
        
    https://github.com/aspiers/ly2video/issues

and if the problem is not listed there, please file a new
entry so we can get it fixed.  Thanks!

Aborted execution.
@aspiers
Copy link
Owner

aspiers commented May 12, 2023

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
Copy link
Author

jstma commented May 15, 2023

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
Copy link
Author

jstma commented May 15, 2023

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
Copy link
Author

jstma commented May 15, 2023

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
Copy link
Owner

aspiers commented May 17, 2023

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
Copy link
Author

jstma commented May 17, 2023

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
Copy link
Owner

aspiers commented May 17, 2023

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
Copy link
Owner

aspiers commented May 17, 2023

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
Copy link
Owner

aspiers commented May 17, 2023

(pitch (ly:event-property cause 'pitch))
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
Copy link
Author

jstma commented May 17, 2023

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
Copy link
Owner

aspiers commented May 17, 2023

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
Copy link
Owner

aspiers commented May 17, 2023

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
Copy link
Author

jstma commented May 18, 2023

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
Copy link
Owner

aspiers commented May 18, 2023

I think the pitch comes from music event properties - this seems to be a big hint:

(pitch (ly:event-property cause 'pitch))

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:

(midi-pitch (if (ly:pitch? pitch) (+ 0.0 (ly:pitch-tones pitch)) "no pitch")))

This could be reused to avoid crashing when pitch is missing.

@jstma
Copy link
Author

jstma commented May 18, 2023

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
Copy link
Author

jstma commented May 18, 2023

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): ridecymbal

@jstma
Copy link
Author

jstma commented May 18, 2023

This may seem like a stupid question, but the beatmap doesn't know about pitches. So why are pitches necessary for synchronization?

Plodding on,

if grobPitchValue not in midiPitches:

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:

https://github.com/lilypond/lilypond/blob/5bf32449f6d212a8050bb61740a706e8154b1268/ly/drumpitch-init.ly#L152

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
Copy link
Author

jstma commented May 18, 2023

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.

test.mp4

These are the same problems I saw running the tests under ly2video/test/regression.

input.mp4

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 jstma mentioned this issue May 19, 2023
@jstma
Copy link
Author

jstma commented May 20, 2023

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.

#106
#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
Copy link
Owner

aspiers commented May 20, 2023

I'll take a look at those.

Did you try messing around with --width and --height? Might help fix the black space issue.

@aspiers
Copy link
Owner

aspiers commented May 20, 2023

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
Copy link
Author

jstma commented May 20, 2023

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
Copy link
Owner

aspiers commented May 20, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants