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

Exported MIDI files contain only Hi Bongo note on/note off events. #307

Closed
dawookieuk opened this issue Sep 28, 2015 · 10 comments
Closed

Exported MIDI files contain only Hi Bongo note on/note off events. #307

dawookieuk opened this issue Sep 28, 2015 · 10 comments
Labels
Milestone

Comments

@dawookieuk
Copy link
Contributor

Current Master branch of 0.9.7 generates MIDI files which have all note events as instrument number 60 which corresponds to the GM Midi Kit Hi Bongo instrument.

All the below was performed on Debian Linux 7.9 'Wheezy' and replicated in OS X 10.9 'Mavericks'

For testing, I cloned the 0.9.6 and 0.9.7 branches into separate folders and built them with ."/build.sh m"

I opened the executable for 0.9.6 and generated and saved the test file "Test Scale.h2song".

Still within the 0.9.6 application I exported the song to a MIDI file named "0.9.6-Master.mid"

I closed the 0.9.6 application.

I opened the 0.9.7 application.

I within the application I loaded the file "Test Scale.h2song" and then exported the song to a MIDI file named 0.9.7-Master.mid.

I closed the 0.9.7 application.

I installed the python-midi package from https://github.com/vishnubob/python-midi

I dumped the contents of the MIDI file 0.9.6-Master.mi

d to a text file called 0.9.6-Master.mid.txt with the line: mididump.py 0.9.6-Master.mid > 0.9.6-Master.mid.txt

I dumped the contents of the MIDI file 0.9.7-Master.mid to a text file called 0.9.7-Master.mid.txt with the line: mididump.py 0.9.7-Master.mid > 0.9.7-Master.mid.txt

Viewing the two files it is apparent that 0.9.6 is generating the note events correctly and that 0.9.7 does note, all not events have a value of 60 for the instrument number.

I apologise for the inline texts but the site would allowed me to upload.

0.9.6-Master.mid.txt Contents:

midi.Pattern(format=1, resolution=192, tracks=
[midi.Track(
[midi.TrackNameEvent(tick=0, text='Hydrogen song!!', data=[72, 121, 100, 114, 111, 103, 101, 110, 32, 115, 111, 110, 103, 33, 33]),
midi.NoteOnEvent(tick=0, channel=9, data=[36, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[36, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[37, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[37, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[38, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[38, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[39, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[39, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[40, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[40, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[41, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[41, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[42, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[42, 101]),
midi.NoteOnEvent(tick=144, channel=9, data=[43, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[43, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[44, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[44, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[45, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[45, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[46, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[46, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[47, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[47, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[48, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[48, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[49, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[49, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[50, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[50, 101]),
midi.EndOfTrackEvent(tick=0, data=[])])])

0.9.7-Master.mid.txt Contents.

midi.Pattern(format=1, resolution=192, tracks=
[midi.Track(
[midi.TrackNameEvent(tick=0, text='Hydrogen song!!', data=[72, 121, 100, 114, 111, 103, 101, 110, 32, 115, 111, 110, 103, 33, 33]),
midi.NoteOnEvent(tick=0, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=144, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOnEvent(tick=48, channel=9, data=[60, 101]),
midi.NoteOffEvent(tick=48, channel=9, data=[60, 101]),
midi.EndOfTrackEvent(tick=0, data=[])])])

@elpescado
Copy link
Contributor

I may be responsible for this. Do you have MIDI note property set in drumkit?

@dawookieuk
Copy link
Contributor Author

The drumkit in both cases was the default that came out of the build. I did look for how to examine the MIDI information for each instrument but couldn't find anything, if you could advise how I do that I'd be grateful. :-)

Looking at the diff, I think the issue was introduced with a modification in smf.cpp. Between 0.9.6 and 0.9.7 the lines:

int nInstr = iList->index(pNote->get_instrument());
int nPitch = 36 + nInstr;

were replaced by:

Instrument *pInstr = pNote->get_instrument();
int nPitch = pInstr->get_midi_out_note();

This appears to be the only difference after the section I added to generate the Tempo Map in track0 but I can't figure out why it doesn't work. This section of code in my repo is as 0.9.6 because that's the branch I was working on. My primary machine is a Mac and when I tried to build 0.9.7 I encountered this bug so I reverted to 0.9.6 so that I only had one issue at a time to work on. I then applied the same files from my repo to 0.9.7 and it rectified this bug as well as bug #124. I have also replicated the same behaviour in a Linux VM that I've been using so it's across platforms.

@mauser
Copy link
Member

mauser commented Sep 29, 2015

Hi,
thanks again for bringing this issue up (and of course for your pull request)!

As @elpescado already assumed, the change has to do with his changes earlier in 0.9.7. Before his changes, hydrogen just calculated the midi out note by using the instruments position in the instrument list. After his change, the midi note gets the value of the "note" instrument property. The "note" property is used to define the midi note which gets emitted by hydrogen if a note is played for this instrument. By default, this property is set to C3 for all instruments. This is the reason why you can hear only the Hi-Bongo (which is not always a bad thing 👍 ).

In my opinion, neither solution (the one in 0.9.6 or in 0.9.7) is perfect. Sometimes people want the simple solution like in 0.9.6: the note value gets assigned automatically. Other people want to control the note (maybe because they are using gear which reacts on defined, unconfigurable midi notes) and they want to assign a completely different note to each instrument independently without moving the instruments around.

@mauser
Copy link
Member

mauser commented Sep 29, 2015

So let's talk about possible solutions to this problem:

Option 1) We could introduce a special default value (like 'DEF', 'NINSTR' or 'POS') which is the default value for new instruments and the packaged drumkits. If an instrument with such a value gets exported, were using nPitch = 36 + nInstr;. If there is something else defined, the defined note gets exported (0.9.7 behaviour).

Option 2) The export dialog lets you choose between "use position of instrument" or "use defined notes".

@mauser
Copy link
Member

mauser commented Sep 29, 2015

@elpescado: Indepently from the solution.. wouldn't it make more sense if the GmKit notes are set by default to 36+n?

And in addition, i'm confused by the fact that hydrogen translates C3 to 60. Shouldn't it C3 be 36? According to http://www.electronics.dit.ie/staff/tscarff/Music_technology/midi/midi_note_numbers_for_octaves.htm , 60 should be C5.

@elpescado
Copy link
Contributor

Here's my commit: elpescado@092402e

My motivations were following:

  1. expored files were unusable for me, as generated note numbers were arbitrary anr required remapping after import in target software
  2. it was inconsistent with MIDI out. MIDI playback from Hydrogen emitted different notes than playing exported MIDI file.

I have been thinking about this issue before, so I tried to set "proper" MIDI notes to GMKit to match notes defined by GM spec (in drumkit.xml). In intrument.cpp I changed default note to 36 + number_of_instrument in order to avoid all instruments share the same note number and thus preserve legacy behavior.

Somehow it doesn't work. Maybe some other drumkit.xml file gets loaded, that assigns all instruments to C3?

@elpescado
Copy link
Contributor

Changing MIDI Note

@dawookieuk
Copy link
Contributor Author

I figured that out late last night and just tested it now. I fixed a bug that wasn't a bug - go me!

I am a noob with MIDI software generally, but this way of attaching the MIDI instrument number to a member of a kit makes more sense to me. I was surprised when I first started using 0.9.6 and couldn't find a way to tie a midi instrument value to an instrument. Setting up a column of instruments isn't an ideal solution to me as I'd like to create a kit that models my real life kit and I ended up with instruments in my hydrogen kit that I didn't have in my real kit just to pad the instrument numbers out correctly.

I do feel that the if this is to be a way of configuring the MIDI aspect of the instrument then the UI should be updated to reflect it. The use of a musical note value rather than a MIDI instrument number isn't intuitive to me at all and is the main reason I didn't realise that value had MIDI implications.

There could be validation to restrict the range to the GMKit with an advanced option to enable the user to select any MIDI note value perhaps? If people want to do weird stuff with MIDI then let 'em I say. ;-)

Now I understand the code better I'll cancel my pull request and put the MIDI note extraction back in to my repo and push a new commit. I'll resubmit a new pull request on that commit because it will still resolve bug #124.

As this behaviour is by design and not a bug does anyone have any problem with me closing this issue or shall I leave it open to enable this discussion to continue?

@mauser
Copy link
Member

mauser commented Oct 1, 2015

Lets leave it open for the discussion. There is still a problem: You should not get values of 60 for your complete drumkit.

I had a deeper look into the issue and i think that i can now explain your behaviour. The problem is that a song has also midi out notes (not only instruments of drumkits). This means that the drumkit is loaded correctly with midi values starting from 36, as @elpescado described. But if you're using a song which has no midi out notes (like a song saved with hydrogen 0.9.6), hydrogen uses a default value for the midi out note (song.cpp, line 502):

QString sMidiOutNote = LocalFileMng::readXmlString( instrumentNode, "midiOutNote", "60", false, false );

So this is why where the Hi Bongo comes from :)

Why is the midi out note saved in the song file? Well, if it is done this way, you can use the same drumkit with different midi out configurations.

As a quick workaround, you can just re-load the drumkit in hydrogen 0.9.7. After re-loading, the instruments in the song are beeing overwritten with the instrument from the drumkit (which have the proper midi out values set). Sounds confusing, i know..

@mauser
Copy link
Member

mauser commented Nov 27, 2016

Hi,

it turned out that a lot of the described behaviour stems from the fact that we in 0.9.7 the default song had note 60 set for all instruments. This is just irritating and confusing.. I've fixed that in 1.0 and maybe i will backport that fix to 0.9.7 as well.

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

No branches or pull requests

4 participants