Open sapieneptus opened 2 years ago
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
I've signed the CLA, can that build be rerun?
Can you provide a little more information on how you're using the decoder? I'm guessing it's for some kind of custom setup.
The way we're using it (as illustrated in metrics_utils.event_predictions_to_ns
), the ground truth for the current time offset comes from the start_time
passed into decode_events
. That's then used to set state.current_time
. We do this because we're decoding independently-inferred chunks of the full sequence.
We do this because we're decoding independently-inferred chunks of the full sequence.
Right, I had suspected as such. I was just trying to understand how the MT3 + note-seq libraries work and wrote some code to split a midi file into subsequences just to see if I could then reconstruct the original midi file. So my input would be a sequence of events corresponding to the entire midi file (several minutes worth of events).
I can see how this function would work as-is for a small slice containing only a single note event + some shift events, but it would fail if it encounters multiple note events (unless there are increasingly more shifts between subsequent note events).
So perhaps it's by design - but I believe this change is still an improvement, as it should not change functionality for the 'small slice' use-case and will prevent errors in a longer slice use-case.
I have updated the description with my relevant code.
I think it still doesn't work for our case because state.current_time
at the end of one chunk isn't necessarily the right start time of the subsequence chunk. For example, what if there are several chunks in a row with no note events? The start time of the chunk needs to come from some external source.
Background
While running this decoder on a sequence of tokens (midifile ->
NoteSequence
-> event_tokens), I realised that after every note is processed and we return to processing shifts, thecur_time
is reset (sincestart_time
is 0). This then causes an exception to be thrown fromnote_sequences#decode_note_event
:I believe the
cur_time
should be an offset ofstate.current_time
, not ofstart_time
- this allows the decoding sequence to pick up where it left off along the timeline.Code example: