openmusic-project / openmusic

The OpenMusic visual programming / computer-aided composition environment
https://openmusic-project.github.io/openmusic/
GNU General Public License v3.0
319 stars 22 forks source link

portmidi: first note in measures (voice, poly) is silenced #49

Closed andersvi closed 1 year ago

andersvi commented 1 year ago

Seems the first note, at date=0, is silenced in these classes

Seems to be caused by time-signature midi-evt's being initialized to chan=0, then being sent to the port with chan=-1 (in #'portmidi-send-evt).

Not sure whats right? If they are set up with chan=1 (as the other default midi-evts), voices play ok.

But not poly's....

andersvi commented 1 year ago

Any of you experts in the MIDI-specs? Are time-signatures supposed to have a channel? Or are they per track? none?

karimhaddad commented 1 year ago

Hi Anders,

Not sure about this. notes are played here on onset 0 (data? not sure about what you describe?) Can you be more specific please? Is it an imported file, a midi file? can you send an example?

Best K

karimhaddad commented 1 year ago

Yes indeed! You are right. very strange? will check this out ASAP

andersvi commented 1 year ago

"date=0" = at time zero

andersvi commented 1 year ago

The midi-channels for all evt's are moved from 1-based to 0-based inside PortMidi/portmidi-api.lisp, leaving the time-signature with chan=-1.

karimhaddad commented 1 year ago

No i don't think it is that. I just tried on om6.15 (very old version), thru 7.3, same result. I suspect it is a bug in Qsynth. On macos all notes plays well. and if you use the fluidsynth player in OM, all notes plays also. So this must come from qsynth. Maybe they've changed something? Will try on an old version of qsynth if i can. and will let you know.

BEst K

andersvi commented 1 year ago

using fluidsynth from cli I get the same error here.

is mac also using the portmidi-interface?

outputting a timesig object with chan=-1 seems strange anyways, no? Perhaps some synths just disregard it?

the poly trouble can be bypassed if i make sure only one time-signature is output at the beginning of each measure, cfr patch below (just a hack, but may perhaps help debug)

index 5071b8fe..f9855ac0 100644
--- a/OPENMUSIC/code/projects/midi/tools/midi-preparetoplay.lisp
+++ b/OPENMUSIC/code/projects/midi/tools/midi-preparetoplay.lisp
@@ -118,13 +118,15 @@
          (div 8 )   ;;; (max-div self))
          (MeasureEvent (om-midi::make-midi-evt :type :TimeSign
                                       :ref 0 ; (if voice voice 0) ;; send to 0 to read in external sequencers/DAWs (?)
-                                      :chan 0 ;;; WHANT IS THE CHANNEL OF A TIME SIGN EVENT ?
+                     ;; :chan 0 ;;; WHANT IS THE CHANNEL OF A TIME SIGN EVENT ?
+                             :chan 1 ;; channels in OM seem to be 1-based
                                       :date at
                                       :fields (list num den 24 div)
                                       :port (or port *def-midi-out*))))

-    (cons MeasureEvent
     (remove nil
+       (cons (if (and voice (= voice 1))           ;only send measure info for first voice
+             MeasureEvent nil)
          (loop for sub in (inside self) collect
                         (let ((objstart (+ at (offset->ms sub))))
                           (let ((in-interval (or (null interval)
karimhaddad commented 1 year ago

Bizarre, using fluidsynth player from OM doesn't do that. I will test your proposition to see if it works on all platforms. Will let you know.

karimhaddad commented 1 year ago

However, if it is a polymetric object, i don't know how this will end up? Should reflect on it. THanx

andersvi commented 1 year ago

...test your proposition...

just a hack, please don't use in any production code or master branch before better checking :-)

karimhaddad commented 1 year ago

Dear Anders,

Just to make sure if i understood you well, you've tried the fluidsynth player via OM ans this didn't work? Strange, it works for me. Or, what did you mean by "fluidsynth from CLI" exactly?

karimhaddad commented 1 year ago

Dear Anders,

Just tried the latest openmusic rpm on fedora 32 (which is an old release), and just using directly qsynth with the default om midi player, and it played alright. So, my suspicion is right here. It has to be something in the libportmidi, or qsynth that has been changed/modified in the late release. However, that thing with chan=-1, and timesignatures, we should check it out of course.

Best K

andersvi commented 1 year ago

The issue is with the handling of time-signatures. Are they useful at all in live-output? Why not just filter them out?

When using fluidsynth (from command-line) or qsynth (fluidsynth inside), sending the time-signature makes the first keyOn silent.

If i use another synth, e.g. yoshimi, things are fine. Possibly it disregards time-signatures?

OMs fluid-player is also fine - ie. with the same libfluidsynth.so as the command-line version. Probably because fluid-player does not send the time-signatures out?

om# is fine both with qsynth and fluidsynth (cli), can't remember now whether it outputs time-signatures

andersvi commented 1 year ago

Looking at the output in kmidimon when using om-midi-player, it shows a 'reset' message where the time-signature would be, which probably explains the silent tones?

I think there might be two separate issues - 1) the midi time-signature should probably not be sent out a port at all during playback (just for midi-files), and 2) the message is perhaps not formatted correctly, instead ending up as a 'reset'-message?

No idea why other synths (ie. yoshimi) plays nice. Possibly it just disregards the 'reset message?

karimhaddad commented 1 year ago

Hi,

Did tried your diff. Just changing the chan=0 to chan=1. And this worked. Do you think it is necessary to do the rest?, ie:

THanx again

andersvi commented 1 year ago

Please don't use any of that as it is - it was just a quick hack to help isolate and try to understand the bug :-)

Is there a reason to allow time-signatures in the (streamed) output? Why not filter them away? Fluid-player does not output them, neither does om#'s midi-player if i remember right.

Vs. the actual timesign-message : since the 'reset' message is reported in the streamed output where there should be a 'timesign', there's probably something wrong with the format as well? Do time signtures come out correctly in an exported midi-file?

karimhaddad commented 1 year ago

Please don't use any of that as it is - it was just a quick hack to help isolate and try to understand the bug :-)

Just changing the chan to chan=1 works for me. Do you think there are some consequences?

Is there a reason to allow time-signatures in the (streamed) output? Why not filter them away?

I don't really know. This was the code since v6.15 and maybe before then. You should ask Jean. But I presume, this was meant to be exported to a midi file as you suspect it. Should maybe check if it is exported correctly. One thing though, which is important, is the tempo export, which i think works. But should also check this against the new libportmidi. So i'll do some testing. and be back to you my friend.

Best Karim

karimhaddad commented 1 year ago

Some feedback: Tested it using the Jean's MIDI tutorials, patch: '05-midifile analysis'

No export of tempo. all durations are translated following tempo indication.

Checked this also using kmidmon

karimhaddad commented 1 year ago

Another fix should be made for MidiFile method:


(defmethod PrepareToPlay ((player (eql :midi)) (self MidiFile) at &key approx port interval voice)
  (declare (ignore approx voice))
   ;(setf converted-seq (delete-tempo-info (fileseq self) (clicks self)))
   ;(setf converted-seq (fileseq self))
  (setf port (or port *def-midi-out*))
  (let ((newinterval (and interval (interval-intersec interval (list at (+ at (get-obj-dur self)))))))
    (loop for event in (fileseq self) 
          when (and (or (not interval) newinterval)
                    (not (equal (om-midi::midi-evt-type event) :Tempo))
                  + (not (equal (om-midi::midi-evt-type event) :timesign))
                   + (not (equal (om-midi::midi-evt-type event) :pitchbend))
                    (or (null interval) (point-in-interval (+ (om-midi::midi-evt-date event) at) newinterval))
                    (not (equal (om-midi::midi-evt-type event) :EndTrack)))
          collect   
          (let ((newevent (om-midi::copy-midi-evt event)))
            (print (list newevent))
            (setf (om-midi::midi-evt-port newevent) port)
            (setf (om-midi::midi-evt-date newevent) (if newinterval 
                                               (- (+ (om-midi::midi-evt-date event) at) (first interval))
                                             (+ (om-midi::midi-evt-date event) at)))
            newevent))
  ))

Therefore removing :timesign and :pitchbend info. Since if we remove timesign and keep pitchbend the pitchbend will transpose played note one tone lower.

karimhaddad commented 1 year ago

I think there might be two separate issues - 1) the midi time-signature should probably not be sent out a port at all during playback (just for midi-files), and 2) the message is perhaps not formatted correctly, instead ending up as a 'reset'-message?

Yes indeed. time signatures don't have neither channel nor port (the same for tempo). I just exported a Finale file as midi, and it shows indeed on kmidmon no chan info. So maybe the structure of om-midi-evt is not quite right for these types of mdi msg.

Furthermore, MIDI doesn't handle at all polymetrics. So you are right to use the metrics of the first voice only. Tempo, are apparently modukated in duration according to quarter note= 60 always.

andersvi commented 1 year ago

Some feedback: Tested it using the Jean's MIDI tutorials, patch: '05-midifile analysis' No export of tempo. all durations are translated following tempo indication.

With current released code? Or with our hacks here?

karimhaddad commented 1 year ago

Yes with current released code, ie the code as it was since 6.15. Beside that, tempo and timesig don't require chan info that is required in the midi-evt structure. Maybe we should create a non channel midi-something structure particular for these.?

andersvi commented 1 year ago

Do you think this is perhaps getting a bit 'messy'? :-) I am starting to get a bit confused. Perhaps it's good to set up another branch to sort out things?

karimhaddad commented 1 year ago

No, it was like that as it is. Maybe "Messy" from the start. But no big deal, i am working on it. Just have to decide if or not to take into consideration some notation al stuff. Of what i can see, most DAWs don't even care about exported timesig and tempo, the same flow with musicxml. therefore it's up to us to decide what's what... What is your point of view here?

Best Karim

andersvi commented 1 year ago

No, it was like that as it is. Maybe "Messy" from the start.

Some of this might have been left in a state between the old midishare and the current portmidi/CL-midi.

When moving to portmidi/CL-midi, i don't think we ever supported playback of time-signatures or tempo-messages to midi-ports (but supported in midi-file exports).

most DAWs don't even care about exported timesig and tempo, the same flow with musicxml.

musicxml-export seems to output time-signatures no? but not tempo-messages (probably easy to add)

Exported midi-files here include both when checking here. midi_export_voice_w_tempo_master

andersvi commented 1 year ago

What is your point of view here?

If they are not used anyway in playback why not remove them, until someone complains? :-) . After all, neither OMs fluid-player, nor om# sends them out atm.

Or do some more work to make sure the time-signatures are output properly - so that they don't trigger a 'reset'-message in the engines which respects these (like e.g fluidsynth)

Tempo-messages could perhaps be more useful, if OM left the scheduling to external players? But would need some re-thinking of the way OM handles playback-tempo now.

karimhaddad commented 1 year ago

About the tempo:

Yes it takes into account the first tempo only. No tempo changes. Lilypond midi outputs correct tempo changes.

Some of this might have been left in a state between the old midishare and the current portmidi/CL-midi.

I don't think so, but maybe true. One thing is that Jean reworked a lot MIDI (cf. OM MIDI tutorials.

musicxml-export seems to output time-signatures no? but not tempo-messages (probably easy to add)

As I remember, i have implemented tempo export. No? Will test it . But musicxml is soemhow independent from MIDI here.

andersvi commented 1 year ago

I've pushed a 'fix'-branch: fix_MIDI_issues_sep_2023

Seems to get rid of the problems in 2 steps:

Tested with simple voices and polys, better check more throughly before merging. Perhaps there's a test-suite for various MIDI i/o already?

karimhaddad commented 1 year ago

Thank you Anders,

I will clone, and add some other issues regarding midifiles. Let's keep this under testing. Best K

andersvi commented 1 year ago

Last commit reverts all the hacks, and just removes the MeasureEvent with the TimeSign from the output stream. Seems to work fine with playback here using external fluidsynths.

Do you see any reasons to include them in playback? Or other reasons to keep them in the output?

karimhaddad commented 1 year ago

Dear Anders,

Yes of course, if we get rid of both (timesig and tempo) we don't have any problems for playback. However, checking what other soft export midi with these works. Let us meditate on that before taking a decision.

Best K

karimhaddad commented 1 year ago

Anders,

Checked your fork and it works good on playback no problem. However, the problem is in the export midi: If a voice is exported in midi file and re-opened (played back) with the MIDIDILE editor, we have these issues: 1) the pitchbends are working: it transposes the notes one tone lower 2) still tempo and time signature are exported but in a working way concerning dates and duration.

The reason is that prepare-to-play is related to this export. I think i have a clue here but needs some coding. Will let you know about the progress. Right now i have to finish editing a book so it will take some time.

BEst K

andersvi commented 1 year ago

Hi Karim. I'd like to finish this, at least the errors w. playback.

Yes of course, if we get rid of both (timesig and tempo) we don't have any problems for playback. However, checking what other soft export midi with these works. Let us meditate on that before taking a decision.

om# and fluid-player don't seem to output time-signatures or tempo when playing. Since it makes troubles,, why not remove it until someone complains?

Perhaps you can be even more specific about the export/import troubles? Or preferably send me a test-workspace so we know we are talking about the same thing :-)

When importing, OM only includes the initial signature and tempo, nothing further out in the file. If i recall correctly it has been like this always?

Testing with both branches - master and fix_MIDI_issues_sep_2023 - yield the exact same result vs. import/export in both:

karimhaddad commented 1 year ago

Hi Anders,

om# and fluid-player don't seem to output time-signatures or tempo when playing. Since it makes troubles,, why not remove it until someone complains?

The reason thatfluid-player doesn't output ts and tempo is that it is not midi based which is its advantage. Removing it is a serious possibility that i acknowledge. But first let me test this against Mac and win MIDI. It should be compatible. But just in case.

Now about exporting/importing MIDI. sorry i was not very clear. If i export MIDI from lilypond and view it in kmidimon i have this: lili-1

Now with the same VOICE exporting from OM: om-1

Now you can see the difference IF we need to export these info (see time vs ticks difference between the two). Furthermore, the pitch bend info is very problematic if played in a MIDI file. It transposes the notes 1/2 tone lower.

Will prepare these examples and send them to you ASAP.

Best K

andersvi commented 1 year ago

Skjermbilde fra 2023-10-02 12-53-27

I see.

But do you see any difference in that behaviour between master and the current 'fix_MIDI' branch?

(Files above: -dev, is from the 'fix_MIDI, -master is from master...)

andersvi commented 1 year ago

If it has nothing to do with the playback, and is not something which came up because of these fixes - when you've done the necessary checking i suggest I'll close the issue and set up a PR.

Then fix other export-related bugs as a separate issue?

andersvi commented 1 year ago

Hi Karim, do you see any difference between master and 'fix...'-branch vs. the timing of the TimeSig messages?

karimhaddad commented 1 year ago

Dear Andres,

I have the fix for the midifile but still not pushed. Do you think we need these pitchbend msg? I don't think they are needed. Maybe it is up to the user to send them explicitly and not let the midi file send them.

This is one thing.

Other: I have pushed your pull request regarding compiler warning. But now if i see correctly, the midi fork is not synchronized no more. So when we need to merge, some problems are expected no ?

BEst K

andersvi commented 1 year ago

Vs. pitchbend alternatives, there are some choices in the preferences. I'm not sure if they are working as expected though.

Merging is easy, let's just finish things in the branch first.

So if you consider the playback from OM ok now, i'll mark this issue as closed (but still not push anything to master), and open a new issue for the midifile export thing.

karimhaddad commented 1 year ago

Ok let me do some testing here. I will see about the pitchbend (maybe push some code?) Will also test on all other platforms (win mac...)

Best K

andersvi commented 1 year ago

If the pitch-bend is broken on playback now, it must have been broken before as well, it's the same behavior in master.

Why not open a separate issue about pitch-ben, to keep things apart?

karimhaddad commented 1 year ago

On the fork: The midifile playback transposes 1 tone lower. On the master branch it is playing ok. So i am pushing a fix: removing the tempo, timesign and pitchbend messages for the preparetoplay method for midifile.

K

andersvi commented 1 year ago

The midifile playback transposes 1 tone lower. On the master branch it is playing ok.

Strange, here it is the exact same behavior. Perhaps you could send me a workspace?

karimhaddad commented 1 year ago

Here is a patch. I don't think sending the whole workspace (unless you have special preferences). Tell me if you here the diff. I am preparing some highly hack for this issue. Just give me some more time.

Best K Patch 4.omp.zip

karimhaddad commented 1 year ago

Hey wait a minute,

Something is wrong with approximation 1/4. Ie on both master and fork. On chord-seq it is working ok. But not on voices. Will first fix this! Best K

karimhaddad commented 1 year ago

Sorry sorry my bad!!!! Nothing'as wrong here.

karimhaddad commented 1 year ago

...so better use this patch for testing purposes: patch 5.omp.zip

andersvi commented 1 year ago

Here's what i observe:

midifile export/import: when i export a voice with micro-intervals to an external midifile, it includes the correct pitch bend messages. Verified when listing or played back by something external (e.g. kmidimon).

But in both branches there seems to be a bug when importing the same midi-file with pitch-bend messages in to a midifile-object. When playing the midifile-object, OM outputs pitch bend = -8192 to the first 4 channels, resetting any previous pitch-bends sent to the synth. Seems to be a separate issue, since its the same in both branches.

In the master-branch, with the buggy time-signature, playing a voice forces a reset in the synth, and all pitch-bend info is also removed immediately

In the fix_MIDI branch voices play as they should, without resetting the synths

(Probably unrelated to the pitch-bend issue, but it seems the chord-seq in "patch 5" have all notes set to channel 4 (microtuned up 3/4), even though it's connected to the voice above having all chan=1. If i connect a 'fresh' chord-seq to the voice, it gets the same channels as in the voice, and plays the same pitches as the voice above.)

karimhaddad commented 1 year ago

Dear Anders,

Yes sorry i didn't "clean" the patch. My bad. I am making progress with play and export/import midi. Changed some stuff but still didn't commit. Will do ASAP.

Best K

karimhaddad commented 1 year ago

Dear Anders,

Did my commit: 1) in prepare-to-play (measure) we don not need chan =1 for timesignature. so commented. However, in order to make it work added a condition in the player-play-object in order not to have an error! 2) Under Improvments, the setup-retune-messages function (needed when midi files are exported, the one responsible for the pitchbend messages) are now updated to OM's channel shift standard. When in 1/2 tone, no pitchbend is exported.

Here is a CLEAN patch for testing :-) testmidipatch.omp.zip

Should test furthermore in poly's.

Best K