fix: RS5K mgr handle invalid media source lengths and prevent crashes #63
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi and thanks for your wonderful work, I was playing around with some scripts from you and was digging through some of the code to find some solutions to minor issues I had.
I hope it's okay for me to create this PR where I tried to split the fixes into separate commits with messages what the intention was. You're welcome to reject the PR if it's not what the project needs, I'm new to this repository and lua/Reaper scripting in general.
First one was
MIDI/mpl_Export selected items as MIDI files into project path.luawhere I noticed it loses the selection after executing the script, which doesn't play well with adding more actions to the selection (or to add to or remove from the selection).I also noticed the Media Item length wasn't respected - this is debatable, since if we export only the existing MIDI events, the loop only spans those events, but I was creating drum loops and the .mid files didn't span the length of the media items (and the Action indicates the "media item" itself is being exported, which spans a longer time than the events).
The workaround is to always manually add some event (silent note or CC) to the end of the selection to ensure that the whole media item length is respected, but my commit does this out of the box - maybe a boolean makes more sense to toggle if auto-adding to the end of the media item is the wanted behavior?
I also tried to auto-name the files based on the media item's name and noticed special characters could cause issues (at least on Windows) so I added basic filename sanitation. Maybe there's a better way to do this?
Regarding RS5K Manager, I had a runtime error crashing the plugin whenever it loaded an "empty" file (0 Byte .wav files) from a Database. The error helped me find some folders in the database where I had indeed some empty files, but a runtime error didn't fully explain what was going on so I thought it would be better to just ignore empty files - I'm not 100% sure the commit fully fixes the issue but at least it highlights the problem.
Hope this is helpful to you or others using the plugin.
Please ask if something isn't clear and I'll try my best to give more insight.
Happy new year,
Bjarki