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

Changing the 'note_off' property doesn't affect the 'duration_sec' property #293

Closed
Lelant opened this issue Jul 15, 2023 · 3 comments · Fixed by #295
Closed

Changing the 'note_off' property doesn't affect the 'duration_sec' property #293

Lelant opened this issue Jul 15, 2023 · 3 comments · Fixed by #295
Assignees
Labels
good first issue Good for newcomers question Further information is requested

Comments

@Lelant
Copy link

Lelant commented Jul 15, 2023

I noticed something. Not sure if it is a bug. I have a PerformedPart. When I change the note_off property of a note, then the duration_sec property of the corresponding note_array() stays the same.

Here is an example of how I encountered this phenomenon:

print(performedPart.notes[0])
print(performedPart.note_array()[0])

performedPart.notes[0]['note_off'] += 1.3

print(performedPart.notes[0])
print(performedPart.note_array()[0])

I print the values of the first note before and after changing the note_off value. I can see that this value has changed accordingly. The note_array() shows two types of duration, but not a note_off value. It shows the duration_sec and the duration_tick. Now in my output I see that the value of duration_sec has not changed, but the value of duration_tick has changed as expected.

Am I missing something or is this a bug or is this intentional?

I noticed this because of using the Piano Roll. After changing the note_off value, the new created piano roll shows the same as before. So it seems the piano roll uses this duration_sec value, that hasn't changed.

For now I found a workaround. For changing the length of a note I now use the note_array()'s property duration_sec instead of note_off. And after the change I overwrite my notes of the PerformedPart with the changed note_array().
Like so:

performanceNoteArray = performedPart.note_array()
performanceNoteArray[0]['duration_sec'] += 1.3
performedPart = pt.performance.PerformedPart.from_note_array(performanceNoteArray)

But I'm still curious if it should be possible to get a correct result in the piano roll just with changing note_off

Thanks a lot,
Tim

@manoskary manoskary added good first issue Good for newcomers question Further information is requested labels Jul 15, 2023
@manoskary manoskary self-assigned this Jul 15, 2023
@Lelant
Copy link
Author

Lelant commented Jul 16, 2023

I also noticed another related thing.

When I set the note_on and note_off values of notes of a performedPart by hand, and when I then try to compute the piano roll, I get the following error message: (coming from file partitura/utils/music.py, line 1316)

raise ValueError("Note durations should be >= 0!")

But as I set the note_on and note_off values by myself, I know that they should result in positive durations for the notes. Here is how I did set the values:

note_on_val = 0.0
note_off_val = 1.0
for note in performedPart.notes:
        note['note_on'] = note_on_val
        note['note_off'] = note_off_val
        note_on_val += 1.0
        note_off_val += 1.0

So it looks like changing properties of performedPart.notes doesn't effect the note_array() that is generated to compute the piano roll (at least I guess that the piano roll uses the note_array() and not the .notes property)

@manoskary manoskary linked a pull request Jul 24, 2023 that will close this issue
@sildater
Copy link
Member

sildater commented Sep 22, 2023

Hi!

sorry for the late answer! This behavior is bit confusing. Here's a script that explains the main cases:

import partitura as pt
import numpy as np
performedPart = pt.load_performance_midi(pt.EXAMPLE_MIDI)[0]

# when creating a note array, 
# and the SOUND_OFF property is available, 
# it's used for duration_sec -> no change due to NOTE_OFF
print("changes in 'note_off' don't show up in note array, if there is a 'sound_off'")
print("original 'note_off': ",performedPart.notes[0]['note_off'])
print("same note in note array ",performedPart.note_array()[0])
performedPart.notes[0]['note_off'] += 1.0
print("changed 'note_off': ",performedPart.notes[0]['note_off'])
print("note in note array ",performedPart.note_array()[0])
print("dtypes of note array 'onset_sec', 'duration_sec','onset_tick','duration_tick'")

print("-----------------------------")
# when creating a note array, 
# and the NOTE_OFF_TICK property is UNavailable, 
# NOTE_OFF is used for duration_sec -> change due to NOTE_OFF
for n in performedPart.notes:
    n.pop('note_on_tick', None)
    n.pop('note_off_tick', None)
print("delete the NOTE_OFF_TICK and the the tick values are inferred from 'note_off' and not 'sound_off'.")
print("the 4th value in note array ('duration_ticks') is higher than before, and inconsistent with the other duration")
print("same note in note array ",performedPart.note_array()[0])
print("dtypes of note array 'onset_sec', 'duration_sec','onset_tick','duration_tick'")

# for manipulations of the performedpart: change both 'sound_off' and 'note_off', for consistency delete all ticks information

print("-----------------------------")
# when creating a performedpart from note_array, 
# only the times in seconds are used...
print("when creating the a performedpart from note_array, times in seconds are used")
na_to_change = np.copy(performedPart.note_array())
print("original note in note array ",na_to_change[0])
print("original 'note_off' in performedpart ", pt.performance.PerformedPart.from_note_array(na_to_change).notes[0]["note_off"])
print("change 'duration_tick'")
na_to_change[0]["duration_tick"] = 17
print("changed note in note array ", na_to_change[0])
print("UNCHANGED 'note_off' in performedpart ", pt.performance.PerformedPart.from_note_array(na_to_change).notes[0]["note_off"])
print("change 'duration_sec'")
na_to_change[0]["duration_sec"] = 100.
print("changed note in note array ", na_to_change[0])
print("CHANGED 'note_off' in performedpart ", pt.performance.PerformedPart.from_note_array(na_to_change).notes[0]["note_off"])

# for manipulations of the note_array: change both values in seconds, 'onset_sec' / 'duration_sec', tick times are discarded

The main points are the comments underneath the tests:

  • for manipulations of the performedpart: change both 'sound_off' and 'note_off', for consistency delete all ticks information (also for onset times use 'note_on', not the value in ticks)
  • for manipulations of the note_array: change both values in seconds, 'onset_sec' / 'duration_sec', tick times are discarded

The next release will address the negative duration issue, the rest remains as is for now. I hope you can find a workaround!

@Lelant
Copy link
Author

Lelant commented Sep 24, 2023

Alright, thank you! This is very helpful :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants