Jump to content

  • Log In with Google      Sign In   
  • Create Account

#ActualÁlvaro

Posted 15 December 2012 - 05:47 AM

There are several things I would do differently.

For starters, things will be much easier if you make your timestamps a more reasonable type. Say, a float expressing seconds from the beginning of the song, or perhaps an integer number of milliseconds. Similarly for the duration.

I would make the note value a float, to allow for microtonality.

I don't know if there is any reason why a note should know what channel it belongs to. Presumably there will be a struct "channel" which will contain the notes in that channel, and you don't need to repeat the information of what the channel is inside each note. It is possible that you have a good reason to do that, but I don't know what it is.

`physChannel' is not very descriptive. It is known as "pan" in MIDI, so perhaps you should consider changing its name to something like that.

I would probably have used a 16-bit integer for the instrument. I know there are synthesizers with more than 256 instruments, and it's probably not worth saving the extra byte, given that people have already bumped into this limit at some point.


Similarly to the channel, I don't think I would make the identifier a part of the note. If some container of notes wants to locate them using an identifier, that's great, but I think that should be part of the container, not the contained type. (In C++ I would perhaps store the notes in an object of type std::map<unsigned, note>, where the unsigned integers are the identifiers.)

struct note {
  float note; // in semitones, with 60 being C4, like in MIDI
  float timestamp;
  float duration;
  short instrument;
  char volume;
  char pan;
};

#2Álvaro

Posted 15 December 2012 - 05:47 AM

There are several things I would do differently.

For starters, things will be much easier if you make your timestamps a more reasonable type. Say, a float expressing seconds from the beginning of the song, or perhaps an integer number of milliseconds. Similarly for the duration.

I would make the note value a float, to allow for microtonality.

I don't know if there is any reason why a note should know what channel it belongs to. Presumably there will be a struct "channel" which will contain the notes in that channel, and you don't need to repeat the information of what the channel is inside each note. It is possible that you have a good reason to do that, but I don't know what it is.

`physChannel' is not very descriptive. It is known as "pan" in MIDI, so perhaps you should consider changing its name to something like that.

I would probably have used a 16-bit integer for the instrument. I know there are synthesizers with more than 256 instruments, and it's probably not worth saving the extra byte, given that people have already bumped into this limit at some point.


Similarly to the channel, I don't think I would make the identifier a part of the note. If some container of notes wants to locate them using an identifier, that's great, but I think that should be part of the container, not the contained type. (In C++ I would perhaps store the notes in an object of type std::map<unsigned, note>, where the unsigned integers are the identifiers).

struct note {
  float note; // in semitones, with 60 being C4, like in MIDI
  float timestamp;
  float duration;
  short instrument;
  char volume;
  char pan;
};

#1Álvaro

Posted 14 December 2012 - 08:49 PM

There are several things I would do differently.

For starters, things will be much easier if you make your timestamps a more reasonable type. Say, a float expressing seconds from the beginning of the song, or perhaps an integer number of milliseconds. Similarly for the duration.

I would make the note value a float, to allow for microtonality.

I don't know if there is any reason why a note should know what channel it belongs to. Presumably there will be a struct "channel" which will contain the notes in that channel, and you don't need to repeat the information of what the channel is inside each note. It is possible that you have a good reason to do that, but I don't know what it is.

`physChannel' is not very descriptive. It is known as "pan" in MIDI, so perhaps you should consider changing its name to something like that.

I would probably have used a 16-bit integer for the instrument. I know there are synthesizers with more than 256 instruments, and it's probably not worth saving the extra byte, given that people have already bumped into this limit at some point.


Similarly to the channel, I don't think I would make the identifier a part of the note. If some container of notes wants to located them using an identifier, that's great, but I think that should be part of the container, not the contained type. (In C++ I would perhaps store the notes in an object of type std::map<unsigned, note>, where the unsigned integers are the identifiers).

struct note {
  float note; // in semitones, with 60 being C4, like in MIDI
  float timestamp;
  float duration;
  short instrument;
  char volume;
  char pan;
};

PARTNERS