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

added option to clip input files for questionable files #176

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sleep-yearning
Copy link

@sleep-yearning sleep-yearning commented Feb 23, 2020

default behavior doesn't change, but if the clip parameter is passed as True it's passed on to the mido backend which already implements the same behavior.

This should resolve #164
Edit: might increase the version number for mido requirement

default behavior doesn't change, but if the clip parameter is passed as True it's passed on to the mido backend which already implements the same behavior.
@sleep-yearning
Copy link
Author

sleep-yearning commented Feb 23, 2020

After looking through the mido versions, the required version should be 1.2.8, that's when clipping got implemented there.

I also added a commit to please the formatting restraints.

@craffel
Copy link
Owner

craffel commented Feb 24, 2020

Hey, thanks for this. To be honest I don't think that corrupt MIDI files should be read in this manner - it seems dangerous to arbitrarily clip any non-127 data byte to 127. If a pretty_midi user wanted this behavior, I'd suggest they load in the MIDI file with mido with clip=True, write it out to a tempfile, and then read it in with pretty_midi. I could be convinced otherwise. Either way, you'd need to add the clip argument to the docstring.

@sleep-yearning
Copy link
Author

Alright, I get where you're coming from. An explanation why it was useful for me:
I wanted to use pretty-midi to analyze and convert large sets of midi files.
Having files in there which stop the whole process is pretty cumbersome, especially if you can be tolerant for some values being off.

I switched from mido to pretty-midi in my code and had to change it anyway for me to work.
And since another user requested this too, I added the PR.
Let me know if you want to pull or not, I will add the docstring then.

@craffel
Copy link
Owner

craffel commented Mar 2, 2020

In that case I would suggest wrapping your MIDI file loading in a try/except and skipping the problematic files. If a data byte is > 127 I would treat the file as corrupt, i.e. not useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clip values outside 0-127
2 participants