jazz-soft / test-midi-files

A framework for producing test MIDI files
MIT License
26 stars 2 forks source link

Add more details to warnings #9

Closed truj closed 5 months ago

truj commented 1 year ago

I would like to see a bit more details in the warnings. Here's an example output:

WARNING: offset 330 track 0 tick 68 -- Unnecessary RPN (b3 65 7f -- Registered Parameter Number MSB)
WARNING: offset 334 track 0 tick 68 -- Unnecessary RPN (b3 64 7f -- Registered Parameter Number LSB)
WARNING: offset 350 track 0 tick 69 -- Unnecessary RPN (b5 65 7f -- Registered Parameter Number MSB)
WARNING: offset 354 track 0 tick 69 -- Unnecessary RPN (b5 64 7f -- Registered Parameter Number LSB)
WARNING: offset 390 track 0 tick 71 -- Unnecessary RPN (bb 65 7f -- Registered Parameter Number MSB)
WARNING: offset 394 track 0 tick 71 -- Unnecessary RPN (bb 64 7f -- Registered Parameter Number LSB)
WARNING: offset 430 track 0 tick 73 -- Unnecessary RPN (b5 65 7f -- Registered Parameter Number MSB)
WARNING: offset 434 track 0 tick 73 -- Unnecessary RPN (b5 64 7f -- Registered Parameter Number LSB)
WARNING: offset 454 track 0 tick 74 -- Unnecessary RPN (b5 65 7f -- Registered Parameter Number MSB)
WARNING: offset 458 track 0 tick 74 -- Unnecessary RPN (b5 64 7f -- Registered Parameter Number LSB)
         and 164 more of "Unnecessary RPN" ...
WARNING: offset 1939 track 0 tick 1322 -- Bad MIDI value set to 0 (255)
WARNING: offset 1940 track 0 tick 1322 -- Bad MIDI value set to 0 (255)
WARNING: offset 2691 track 0 tick 1802 -- Bad MIDI value set to 0 (255)
WARNING: offset 2692 track 0 tick 1802 -- Bad MIDI value set to 0 (255)
WARNING: offset 3427 track 0 tick 2282 -- Bad MIDI value set to 0 (255)
WARNING: offset 3428 track 0 tick 2282 -- Bad MIDI value set to 0 (255)
WARNING: offset 3871 track 0 tick 2532 -- Bad MIDI value set to 0 (255)
WARNING: offset 3872 track 0 tick 2532 -- Bad MIDI value set to 0 (255)
WARNING: offset 4179 track 0 tick 2747 -- Bad MIDI value set to 0 (255)
WARNING: offset 4180 track 0 tick 2747 -- Bad MIDI value set to 0 (255)
         and 84 more of "Bad MIDI value set to 0" ...

For the "Unnecessary RPN" warnings I can see the problematic message in hex values. That's really helpful, and it would help a lot for the Bad MIDI value messages, too. The above example has 33 messages in track 0, tick 1322 - so it's a bit hard to find out which message is the problematic one.

1st Suggestion: Add the full message in hex values for every printed warning.

2nd Suggestion Add a comment why the RPN messages are regarded unnecessary.

The Bad MIDI value set to 0 (255) warnings are a bit hard to understand. I guess the problematic message here is an incorrect pitch bend: E4 FF FF. And of cause the FF (255) is illegal because the first bit is reserved for status bytes, and it must not be 1 in a value byte. But Bad MIDI value set to 0??? Why 0?

3rd suggestion Add a bit more context why the value is bad (e.g. because it's higher than 127)

By the way, I didn't find a second problematic message in tick 1322. My impression is that E4 FF FF causes both warnings. Is that possible?


While writing this I just looked at the rest of the output to check the bad pitch wheel message. test-midi-files tells me, the message is E4 00 00. [ 1938] 1322: e4 00 00 -- Pitch Wheel The MIDI analyzer of Midica tells me that the very same message is E4 FF FF. I'm not 100% sure which one is right. But it would surprise me if Midica is wrong here. It mainly uses methods from the midi package of the Java Sound API.

But if test-midi-files would be correct, then the output Bad MIDI value set to 0 (255) would be even more confusing. Why 255?


Just debugged a bit, and now I'm pretty sure that the message is in fact E4 FF FF. But maybe this is a good opportunity to create a new test file ;)

jazz-soft commented 1 year ago

Hi! Thank you for asking. For a more detailed MIDI file analyses it would take a whole new application. Good idea for when I have more time. E4 FF FF is an invalid message because MIDI 1.0 does not allow values larger than 0x7F anywhere except the status bytes. SMF reader tries to fix minor errors automatically, that's why you see E4 00 00 instead of E4 FF FF. "Unnecessary RPN" means that you never set value for that RPN, so you can safely delete those messages from the file.

truj commented 1 year ago

Yes, adding detailed text descriptions to every possible problematic message is probably way too much work, you're right. But maybe just including the message in Hex more consequently for all warnings could be feasible. That would already help a lot.