grimmdude / MidiPlayerJS

♬ MIDI parser & player engine for browser or Node. As a parser converts MIDI events into JSON. Works well with single or multitrack MIDI files.
https://grimmdude.com/MidiPlayerJS/
MIT License
357 stars 52 forks source link

More duration bugs, one minor, another major #64

Open Andrew-J-Larson opened 4 years ago

Andrew-J-Larson commented 4 years ago

In VNC, the duration on this song is around 16:46 (about 1006 seconds), in the MIDI player, it shows up a bit longer at about 1050 seconds.

Song Midi: BohemianRhapsody-_Queen_mid

Is there a specific reason that it goes longer? Or is this truly another bug?

grimmdude commented 4 years ago

Looks like the problem here is that track 1 contains only meta events that span 262,143 ticks. The function that calculates duration is based on the track with the most ticks. If I open this file in Garage Band it seems to omit this track and calculates duration at 5:30.

I wonder if tracks without voice events should be omitted. Here are all the events on track 1, note the final tick count in the last event.

{ track: 1,
  delta: 0,
  tick: 0,
  byteIndex: 0,
  name: 'Set Tempo',
  data: 78 }
{ track: 1,
  delta: 0,
  tick: 0,
  byteIndex: 7,
  name: 'Time Signature',
  data: <Buffer 02 02 18 08>,
  timeSignature: '2/4' }
{ track: 1,
  delta: 56832,
  tick: 56832,
  byteIndex: 15,
  name: 'Set Tempo',
  data: 154 }
{ track: 1,
  delta: 21888,
  tick: 78720,
  byteIndex: 24,
  name: 'Set Tempo',
  data: 78 }
{ track: 1,
  delta: 183423,
  tick: 262143,
  byteIndex: 33,
  name: 'End of Track' }
Andrew-J-Larson commented 4 years ago

What about this file? LudovicoEinaudi-_Nuvole_Bianche_mid

In VLC, it shows up as 5:47 ish (about 347 seconds), when with the library it seems to have a duration of about 1788 seconds

Andrew-J-Larson commented 4 years ago

Oh I figured out what was wrong now... while the song is playing, the duration keeps changing... That's why it's not matching up... So I think you need to do something about how duration is calculated... (maybe a dryrun that accounts for each event and the tempo changes to create a finalized duration?)

Andrew-J-Larson commented 4 years ago

And it seems to affect almost any song now that I have been checking a bunch of songs.

Andrew-J-Larson commented 4 years ago

https://github.com/Tonejs/Midi seems to do a good job at implementing a note duration time, which I'm sure could help you to tame the duration / remaining time functions

Andrew-J-Larson commented 1 year ago

Coming back to this, I know it's been a while figured this would be useful (C++, but handy to know an actual list of MIDI types), from https://code.videolan.org/videolan/vlc/-/blob/master/modules/codec/omxil/OMX_Audio.h#L767

/** MIDI Format 
 * @ingroup midi
 */
typedef enum OMX_AUDIO_MIDIFORMATTYPE
{
    OMX_AUDIO_MIDIFormatUnknown = 0, /**< MIDI Format unknown or don't care */
    OMX_AUDIO_MIDIFormatSMF0,        /**< Standard MIDI File Type 0 */
    OMX_AUDIO_MIDIFormatSMF1,        /**< Standard MIDI File Type 1 */
    OMX_AUDIO_MIDIFormatSMF2,        /**< Standard MIDI File Type 2 */
    OMX_AUDIO_MIDIFormatSPMIDI,      /**< SP-MIDI */
    OMX_AUDIO_MIDIFormatXMF0,        /**< eXtensible Music Format type 0 */
    OMX_AUDIO_MIDIFormatXMF1,        /**< eXtensible Music Format type 1 */
    OMX_AUDIO_MIDIFormatMobileXMF,   /**< Mobile XMF (eXtensible Music Format type 2) */
    OMX_AUDIO_MIDIFormatKhronosExtensions = 0x6F000000, /**< Reserved region for introducing Khronos Standard Extensions */ 
    OMX_AUDIO_MIDIFormatVendorStartUnused = 0x7F000000, /**< Reserved region for introducing Vendor Extensions */
    OMX_AUDIO_MIDIFormatMax = 0x7FFFFFFF
} OMX_AUDIO_MIDIFORMATTYPE;

As for dealing with the different file types, might be able to look over the VLC code likely in https://code.videolan.org/videolan/vlc/-/blob/master/modules/demux/smf.c

And looks like they have some sort of code for getting the correct duration around https://code.videolan.org/videolan/vlc/-/blob/master/modules/demux/smf.c#L690

    if (vlc_stream_Control (stream, STREAM_CAN_FASTSEEK, &b) == 0 && b)
    {
        if (SeekSet0 (demux))
            goto error;

        for (uint64_t pulse = 0; pulse != UINT64_MAX;)
             if (ReadEvents (demux, &pulse, NULL))
                 break;

        sys->duration = date_Get (&sys->pts);
    }

There's other bits in there with how they calculate tick timing based on tempos read in all tracks. So it looks like reading all tracks for tempo and subsequently duration time is unavoidable, @grimmdude

Andrew-J-Larson commented 1 year ago

Update: I checked out https://github.com/mscuthbert/midicube , and their library seems to be calculating times and tempos correctly. Maybe you should check there.