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

losing instruments information when converting midi to note sequence #1464

Closed
DavidPrimor opened this issue Mar 11, 2019 · 14 comments
Closed

Comments

@DavidPrimor
Copy link
Contributor

Hello,
I'm doing manipulations on midi files using Magenta. I convert the midi file to NoteSequence using

import magenta.music as mm
mm.midi_file_to_sequence_proto(midi)

The problem is that if I have unused midi channels (lets say 5 and 6) and used midi channels (lets say 7 and 8), the conversion put the used channels in place of empty ones, so I loose the instrument true setup, and when converting it back to midi and playing it with midi player, the instruments are mixed up. Any suggestion how to avoid this wrong conversion (only happens when there are some unused midi channels in the midi file). Thanks!

@cghawthorne
Copy link
Contributor

Our MIDI conversion code preserves the concept of "instrument" (all events in a track with the same program and channel) and what MIDI program each instrument should have. Unfortunately, we do not specifically preserve MIDI channel, so when we convert from NoteSequence to MIDI, the channel assignment is arbitrary (but the programs should be correct).

@DavidPrimor
Copy link
Contributor Author

Thanks. So any advise how to use Magenta for this midi manipulation, keeping the consistancy of the midi channels ?

@cghawthorne
Copy link
Contributor

@craffel am I correct in understanding that pretty_midi doesn't preserve this information?

@craffel
Copy link
Collaborator

craffel commented Mar 12, 2019

Yes, pretty_midi does not store or preserve any information about which notes happened on which channel. I don't have any plans to add this functionality, since it does not really map to anything intuitive about music. However, I'd be happy to be convinced that this is important.

A simple workaround which I think should work is to add track name events to each of the channels. This name will propagate into the Instrument, and will be written out onto the instrument's channel when using pretty_midi.PrettyMIDI.write. You could do some simple post-processing to re-assign different channels based on their track names.

@DavidPrimor
Copy link
Contributor Author

Thanks for your answers. Actually, this functionality is required for my music program. I'm using DAW with third party sound (VST) libraries to create basic music tracks. Then, I export the midi files and use it within my python program. I'm doing midi manipulations as well as some AI stuff. All manipulations are done using Magenta and noteSequence. Then, I convert it back to midi and produce wave file with high quality, using this VST. I was trying to do direct conversion to wave using SF2 libraries, but the quality is not good enough so I have to use a better sampler.
Concerning the track name events, if the channel is empty (no notes) pretty_midi fill it with the next channel. I guess in this case the track names wouldn't help, right?
Thanks!

@craffel
Copy link
Collaborator

craffel commented Mar 13, 2019

Concerning the track name events, if the channel is empty (no notes) pretty_midi fill it with the next channel. I guess in this case the track names wouldn't help, right?

I'm sorry, I don't know what you mean.

@DavidPrimor
Copy link
Contributor Author

Looking at pretty_midi, in def _load_instruments(self, midi_data):
I can see the track name events, however I don't find its propagation to noteSequence in
def midi_to_note_sequence(midi_data): . Can you please note the exact place?
I tried to make another workaround by creating an "almost" empty channel by using one note with velocity = 0. Unfortunately it is ignored by prety_midi. So currently I create one note with low velocity and non-existing pitch, so this empty channel will exist and keep the midi channels order.
If you can add the midi channel information it could really help!

@craffel
Copy link
Collaborator

craffel commented Mar 13, 2019

I don't find its propagation to noteSequence in def midi_to_note_sequence(midi_data):

@cghawthorne will have to answer this, I'm not sure if note sequences use track names or not (and if not, why not).

If you can add the midi channel information it could really help!

I don't have any plans to add channel information to pretty_midi. It's not within the scope of the project since it's not a musically meaningful concept.

@DavidPrimor
Copy link
Contributor Author

@cghawthorne I understand that changes to pretty_midi are not trivial; however I’ve spent tens hours working with Magenta for our project, and I really want to continue to use it, even more intensively. I couldn't come up with a workaround that can fit a long term solution. I would appreciate if you can help me find a workaround to maintain the midi channels information. I don’t mind to add any code at my side or contribute to Magenta. Thanks!

@cghawthorne
Copy link
Contributor

I think the most straightforward thing to do would be to add an InstrumentInfo message to NoteSequence, similar to PartInfo. It could contain the track name from pretty_midi, indexed on instrument id. During conversion back to pretty_midi, that field could be used to populate the track name, which pretty_midi would then include in the midi file.

Then, as long as your source has a track name that you can map to channel, all the information would be present. You could either modify whatever your output system is to use track name instead of channel, or add a post-processing step using Mido or something similar to remap the channels of the MIDI file based on their track names.

I'd be happy to review a PR for adding the InstrumentInfo message and carrying the track name across.

@DavidPrimor
Copy link
Contributor Author

@cghawthorne thank you. I think it is a good solution. I'll be happy to write a PR (first contribution to Magenta :) ) for your review.

@DavidPrimor
Copy link
Contributor Author

@cghawthorne In order to test your approach I used PartInfo message and added the right functionality to midi_io.py in two places: note_sequence_to_pretty_midi and midi_to_note_sequence(midi_data). I also added the functionality in my code to keep and rearrange the midi channels. I'm happy to say that it works on my framework using the cloned magenta code with Partinfo message. However, when trying to add InstrumentInfo message instead of PartInfo, I found it quite complex and not streight forward. I tried to change music.proto and music_pb2.py by copy creating almost similar messege as PartInfo:

_NOTESEQUENCE_INSTRUMENTINFO = _descriptor.Descriptor(
  name='InstrumentInfo',
  full_name='tensorflow.magenta.NoteSequence.InstrumentInfo',
  filename=None,
  file=DESCRIPTOR,
  containing_type=None,
  fields=[
    _descriptor.FieldDescriptor(
      name='instrument', full_name='tensorflow.magenta.NoteSequence.PartInfo.part', index=0,
      number=1, type=5, cpp_type=1, label=1,
      has_default_value=False, default_value=0,
      message_type=None, enum_type=None, containing_type=None,
      is_extension=False, extension_scope=None,
      serialized_options=None, file=DESCRIPTOR),
    _descriptor.FieldDescriptor(
      name='name', full_name='tensorflow.magenta.NoteSequence.PartInfo.name', index=1,
      number=2, type=9, cpp_type=9, label=1,
      has_default_value=False, default_value=_b("").decode('utf-8'),
      message_type=None, enum_type=None, containing_type=None,
      is_extension=False, extension_scope=None,
      serialized_options=None, file=DESCRIPTOR),
  ],
  extensions=[
  ],
  nested_types=[],
  enum_types=[
  ],
  serialized_options=None,
  is_extendable=False,
  syntax='proto3',
  extension_ranges=[],
  oneofs=[
  ],
  serialized_start=4325,
  serialized_end=4363,
)

and:

// Stores information about an instrument name
  // See usage within Note for more details.
  message InstrumentInfo {
    // The instrument index.
    int32 part = 1;
    // The name of the instrument. Examples: "Piano" or "bass".
    string name = 2;
  }

I'm sure there is something I'm missing since I don't know which values to put in serialized_start and serialized_end and when trying to run it I got an error:

File "/Users/didi/PycharmProjects/MusicMindServer/nvenv/lib/python3.7/site-packages/google/protobuf/descriptor.py", line 288, in __new__
    return _message.default_pool.FindMessageTypeByName(full_name)
KeyError: "Couldn't find message tensorflow.magenta.NoteSequence.InstrumentInfo"

Can you please advise what is the proper way to add message to NoteSequence?
Thanks!

@cghawthorne
Copy link
Contributor

You should only need to modify the .proto file. The *pb2.py files are autogenerated by protobuf compiler. There are instructions in the README in the protobuf directory on how to install and run it.

You can see some more information about protocol buffers here: https://developers.google.com/protocol-buffers/

@DavidPrimor
Copy link
Contributor Author

Thank you for your help! I've just add a PR with InstrumentInfo message for your review. I changed the .proto file and add some functionality in midi_io.py. As I mentioned before, I checked it with my external code with additional logic for fixing the midi channel order, and it works. Thanks!

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

No branches or pull requests

3 participants