pd-externals / midifile

Read and write MIDI files (.mid) with Pd
Other
0 stars 0 forks source link

Errors in help file for [midifile] object #4

Open dfkettle opened 2 years ago

dfkettle commented 2 years ago

Errors in help file: the [route_events] subpatch of [midifile_read] shows CC messages as being in the range of 176 to 192 inclusive, but 192 is a Program Change message. Also, any messages except Note On, Note Off, Control Change and System Exclusive messages are considered System messages. So Aftertouch, Channel Pressure, Pitch Bend, Channel Mode, etc., aren't being routed properly. It's not clear to me whether or not these messages are actually being written to an output file, I haven't looked at the source code very closely. But IMHO, the help file should indicate whether or not they're supported.

162739345-7bd92fa1-0604-4d78-8701-bca6335e9dad

Josef-N commented 2 years ago

I did not extensively test this help file, but I can confirm a few things I noticed. route 176 ... 192 should be route 176 ... 191 The rightmost outlet of "route controller events" outputs everything else, which is going to [print sys] at the end, including Aftertouch, Channel Pressure etc., which are not defined by a rout object. This help file is just not complete, but I can say, that all other events are working also. Maybe this should be mentioned, I agree. I never tried recording, only playing back midi files. For this purpose I found, that Dan’s c_midiplay-help.pd including c_midiplay.pd from the rc library is much more complete, convenient and ready to use. There is only one small issue with Channel Pressure and Pitch Bend, which will be corrected soon, I think.

umlaeute commented 2 years ago

There is only one small issue with Channel Pressure and Pitch Bend, which will be corrected soon, I think.

@Josef-N which "small issue"? i can't see any bug-report here. am i missing something?

danomatika commented 2 years ago

I believe this sentence is missing context and refers to an issue with [c_midiplay] and not [midifile] which has since been fixed. Basically I used the status byte value ranges from midifile-help.pd which, as noted, are wrong for those types.

Josef-N commented 2 years ago

Thanks, Dan for clarifying. I see, this sentence was quite unprecise, I have been speaking about [c_midiplay], which is fixed now. I think, it would need only a few things to improve [midifile-help]: – correcting the range for controller events: 176 to 191 – renaming [print sys] to [print others], or something like this – including the subpatch of Dan's [c_midiplay-help] 'pd raw midi vals'. I can put it right here, I consider it as extremely helpful. This is the content:

notes on what values are used to parse raw midi bytes inside c_midiplay

status byte + channels 128 - 143: noteoff 144 - 159: noteon 160 - 175: poly aftertouch 176 - 191: control change 192 - 207: program change 208 - 223: aftertouch 224 - 239: pitch bend 255 127 len data: Midi File Proprietary Event

(to be honest – I don’t know what the last line means, this exceeds my knowledge. I never used meta events)

danomatika commented 2 years ago

255 127 len data: Midi File Proprietary Event

I don't know where that's from. It seems incorrect as 255 is MIDI system reset and F7 is SysEx end. It can probably be removed.

UPDATE: This reminds me to check if the [c_midiplay] output parser can handle running status correctly...

Josef-N commented 2 years ago

255 127 len data: Midi File Proprietary

I found also this: http://www.somascape.org/midi/tech/mfile.html Meta events (status byte 0xFF) ... within a MIDI file the System Reset status byte (0xFF) is used to signify a Meta event, hence Meta events are only found in MIDI files.

But this is another universe, maybe better remove it?

p.s.: ahh, now I see, this has extensively been discussed in the other thread already. So my comment is a bit out of date and not necessary.

danomatika commented 1 year ago

Looking at this again and in response to the original post: yes, [midifile] "supports" all of those message types, in the form of spitting out the bytes, at least when reading AFAICT. Just because the help file doesn't exhaustively show all message types, doesn't mean they don't come out. As @Josef-N mentioned, it's just a matter of adding more filtering or at least noting which status byte number ranges are for which messages.

There is a bug in the help file where 192 should not be listed with the control message routing. As noted previous, 192-207 are program messages for channels 0 - 15 (or 1 - 16 in Pd's parlance).

It's not clear to me whether or not these messages are actually being written to an output file, I haven't looked at the source code very closely.

@dfkettle This seems like a separate issue. Do you mean writing messages, not only reading. I haven't actually used [midifile] to write a file.

danomatika commented 1 year ago

For [c_midiplay] the bytes output by [midifile] are grouped into pairs or triplets and the status bytes are used for routing. Since the midi spec guarantees message data values fall outside the status byte ranges, it works well enough, however it may fail for system messages as I don't attempt to detect them. If I did, I would need a spigot somewhere to keep the sysex stream away from the status byte detection stream since these bytes may use more of the full byte range.

3-byte channel message detection:

Screen Shot 2023-05-17 at 12 25 49 AM

2-byte system message detection:

Screen Shot 2023-05-17 at 12 25 53 AM
danomatika commented 1 year ago

Oh, now that I remember it, there is a further example of raw midi byte handling included with Pd:

This is a test patch I made when I cleaned / overhauled Pd's MIDI handling some years ago.

danomatika commented 1 year ago
Screen Shot 2023-05-17 at 1 38 20 AM

Here is a version of the help patch with additional routing:

midifile-help.pd.zip

Please test and if it is sufficient, I can make a PR.

So far I'm able to see output from most of the message types with the files I've tried reading. The "print_status_names" toggle only controls printing for channel 1 which might make it seem like not everything is coming out, however if you print the byte output from [midifile] it seems to be fine.

Josef-N commented 1 year ago

Yes, this additional routing is good. I tested the patch now – sorry for the delay. Printing is fine, but some of the events that should go to Midi devices do not work. Here is a modified version of the help patch: midifile-help_modified.pd.zip Some event types need reordering of the message bytes. Note events are working fine, also program change and channel aftertouch.

Here some screenshots of routings that have to be modified: route polytouch route controller pitch bend Pitch Bend is fine, I only added a [bendout] object, so there is also Midi output, on Channel 1 at least.

Only one more tiny little thing: the name of the subpatch [pd route system messages 192 - 223] – is this correct? The event types are program change and channel aftertouch. I renamed it to [pd route program & aftertouch 192 - 223], is that ok for you?

danomatika commented 1 year ago

Only one more tiny little thing: the name of the subpatch [pd route system messages 192 - 223] – is this correct? The event types are program change and channel aftertouch. I renamed it to [pd route program & aftertouch 192 - 223], is that ok for you?

That's fine. I forgot to rename the subpatch after changing some things.

Josef-N commented 1 year ago

good!

Josef-N commented 1 year ago

p.s. my knowledge of pd is sufficient for getting things to work, but I know, that I am not always using the best method. Maybe you have better ideas for rearranging the order of the numbers that are going to the midi output objects. For instance, polytouch could also be [$2 $1 1], [$2 $1 2] ... –> [polytouchout] (without argument for channel]

In [ctlout] the problem is, that the argument for channel is just ignored, for whatever reason. So it needs the whole package (value, ctl#, channel)

danomatika commented 1 year ago

I think the issue with ctllout is a bug. This used to work before, I believe,but with recent Pd versions it appears to have broken.

enohp ym morf tnes

Dan Wilcox danomatika.com robotcowboy.com

On May 25, 2023, at 1:58 PM, Josef Novotny @.***> wrote:

 p.s. my knowledge of pd is sufficient for getting things to work, but I know, that I am not always using the best method. Maybe you have better ideas for rearranging the order of the numbers that are going to the midi output objects. For instance, polytouch could also be [$2 $1 1], [$2 $1 2] ... –> [polytouchout] (without argument for channel]

In [ctlout] the problem is, that the argument for channel is just ignored, for whatever reason. So it needs the whole package (value, ctl#, channel)

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

Josef-N commented 1 year ago

ahh, I see, thanks!

danomatika commented 1 year ago

Yeah, maybe try the same thing with an older version of Pd.

I had a problem with my performance setup where [c_midiplay] isforwarding ctl messages via #ctlin. Using [ctlin f f] has stopped working for some reason and I wonder if this is related…

enohp ym morf tnes

Dan Wilcox danomatika.com robotcowboy.com

On May 25, 2023, at 3:41 PM, Josef Novotny @.***> wrote:

 ahh, I see, thanks!

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.

Josef-N commented 1 year ago

Indeed. I am using a modified version of your [c_midiplay], only ctl messages, sent directly to ctlout (not to #ctlin). I noticed, that with each event also program change is sent, which is not present in the midi file. This happens both in current desktop version of Pd and in PdParty. Which older version of Pd should I try? Will do it later this evening.

Josef-N commented 1 year ago

I tried a lot of things now. There is no difference in Pd 0.53.2, 0.52-2 and 0.51-4, same result. I found out, that [u_listlastx 2] and [pd detect message type from status byte] do not completely filter out other message types (messages with 3 bytes). It works somehow, if I only use [u_listlastx 3] for all types, expanding the routing object to [route noteoff noteon polytouch ctl pgm touch bend], but that results in some duplicate events of channel aftertouch. In my patch I just disconnected the cable to [u_listlastx 2], as I do not use pgmout and touchout. These are the ones that make problems.

danomatika commented 1 year ago

Ok, thanks for checking. It's not likely a problem with Pd then but maybe more the midifile output handling in my abstraction. We could move any further discussion to the rc-patches repo. I think I'll try overhauling c_midiplay to use route instead of gathering the message groups.

Josef-N commented 1 year ago

Yes, I think so, too, that’s not a problem with Pd. Although the whole thing is very confusing, because there is no unique system for the order in which individual message bytes are sent or received (type&channel – NoteNumber/CtlNumber ... – value). And yes, using route seems to be more reliable.

danomatika commented 1 year ago

And yes, using route seems to be more reliable.

Done. I've updated c_midiplay and seems to work in my testing.

Josef-N commented 1 year ago

perfect – I tried the same things as before and everything works, great!