Suggestions for sequencer API changes

[samhocevar]

Hello,

right now the MidiFileSequencer object can only play and stop. Also the Render() method gives no indication of how many non-zero samples were written before reaching the end of the file. I would be interested in the following:

• a Length() or Duration() method that indicates the length of the file
• a Tell() or Pos() method that indicates the current position in the file
• a Seek() method to seek into the file (block granularity would be OK)

To avoid accuracy issues, I would also suggest that these methods handle a number of samples rather than a duration, so that the client application can directly use the data with Render().

Also note that seq.LoopIndex is not updated (and is currently unused).

What do you think? All these changes are fairly easy and I will gladly submit a PR, but I’d like to get your opinion first.

[sinshu]

Thank you for your proposal. However, I would like to keep the API as it is.

The reason for having MidiFileSequencer in this library is simply to meet the common basic requirement of playing MIDI files, not to provide fine-grained control over MIDI file playback. If users of this library want such control, I believe they should create their own sequencer. In fact, this library is designed to make that possible.

In particular, it is difficult to implement a function like Seek in a MIDI synthesizer in a way that satisfies all users. When seeking, should the reverb of the notes played so far be preserved? How should skipped events be handled? Should time be dealt with in sample units or using Duration? I think these matters should be adjusted according to user preferences, and if so, it's best for the user to create their own sequencer that meets their needs.

Regarding LoopIndex, thank you for pointing it out. I believe it might be unused code that was left behind when the C# version was ported. I will take a look at the code later.

[samhocevar]

All of the above makes sense, thanks. How about exposing MidiFile.times and MidiFile.messages to client applications, then? I believe that’s the only requirement for a custom sequencer.

(Note that I still believe that the current default sequencer is a bit inconvenient as is, because it does not provide the position or an easy way to detect end of file. All that can be computed by keeping track of the sample rate and how many bytes have been rendered, but it would still be more convenient to have something like the Pos() in #7.)

[sinshu]

I believe adding the following features would be beneficial, as they are present in the original C# version:

• MidiFileSequencer.Pos() returning currentTime
• MidiFileSequencer.Eof() returning msgIndex == len(midiFile.messages)

For Go, perhaps EOF or isEOF would be more suitable names? I'm not certain.

I believe that MidiFile.times and MidiFile.messages are heavily influenced by implementation-specific details, making them unsuitable for exposing to users. For example, users would need to understand the following non-obvious aspects when using them:

• times and messages are separate arrays, but their elements correspond one-to-one
• Events from all channels in the MIDI file are aggregated and mixed within a single array
• The type of times is Duration, and information regarding measures and tempo is entirely lost
• message.channel does not necessarily represent the channel number and could be a magic number indicating a special state

I encountered similar design concerns when creating the C# version. The current API is a result of these considerations, as I wanted users to be able to imagine how to use it just by looking at the types and function names without needing to understand the implementation. However, if users still wish to access MidiFile's internal data, they can either modify the library on their side or copy and modify the following three files to create their own custom sequencer:

• binaryreader_ex.go
• midifile.go
• midifile_sequencer.go

While these solutions may seem roundabout, I hope for your understanding.

[samhocevar]

Instead of EOF(), the Golang way is to have read-like methods return an error and/or a zero number of read bytes. See io.Reader() which returns 0, EOF when EOF is reached. In our case, I believe an easy solution would be if Render() wrote a possibly smaller number of bytes when EOF is reached, and returned that value.

I understand that you do not wish to expose times and messages, but previously you suggested that users of your library should write their own sequencer. How can they do that, then?

[sinshu]

If a user is thinking about creating a custom MIDI file sequencer, they should also consider developing their own MIDI file parser. Generally, it's challenging to create MIDI file parsers and sequencers as separate, reusable, and versatile components. Thus, if precise control is needed during MIDI file playback, the user should implement this functionality, starting with the file parser.

In this library, view MidiFile as a simple pointer-like object used to specify the file for playback in MidiFileSequencer. MidiFile is not intended to provide users with a convenient way to manage a chronologically ordered list of notes.

[samhocevar]

Okay. From my side it means either:

• maintaining a fork of go-meltysynth, despite having only minimal knowledge of MIDI
• maintaining custom MidiFile and MidiFileSequencer logic, despite my project not being MIDI-specific

Since the alternative is 15 lines of code of a Seek() function that only add to the current API, that no one has to use, but that could be helpful to others, whose shortcomings that can be well documented and understood, and that can actually be made exact if the need arises, I am a bit dissatisfied… I guess I’ll go the fork way for now.