Add a FIFO class and use it to replace the code in bytestreamToUMP. #13
Conversation
This file contains 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
Replace the readIndex, writeIndex, bufferLength, and umpMessage fields with a single FIFO instance. The separates the responsibility of maintaining the FIFO and simplifies the more interesting code! This commit only affects the FIFO in this one header file. There are others that There are other ad-hoc FIFOs in the code base that can also perhaps be switched over. I want to limit the scope of this change for the time being. That only leaves the quesiton of how to unit-test the code...
* fix bytestreamToUMP Program Change translate when using MT4 * fix bytestreamToUMP RPN/NRPN index translate when using MT4 * added braces in UMPStreamParse to avoid confusion * added more test for MT4 Translation and PC and RPN
…fixed PE chunking issues -Fixed a bug in midiCIProcessor::processMIDICI where checkMUID was called in a way that would always return FALSE -Added PE Status compiler defs for convinience -Fixed typo in documentation -Small formatting fix
Remove incorrect Check MUID Check from PR
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Whilst reading through the bytestreamToUMP code I realized that the readIndex, writeIndex, bufferLength, and umpMessage fields are used together to form a FIFO/circular buffer. I've added a FIFO class to util.h and replaced those fields with an instance. The separates the responsibility of maintaining the FIFO and simplifies the more interesting code!
This new class also reduces overhead for a 4 element container to 1 byte vs. 3 integers previously and has the further benefit that is guards against under/overflow abuses.
This commit only affects the FIFO in this one header file. There is another in umpToMIDI1Protocol.cpp which should probably be switched over. I haven't done that yet because I want to limit the scope of this change.
That only leaves the quesiton of how to unit-test the code... For this kind of thing I usually use google test. Do you have a preference for how this is done? What would you think about adding it as an optional dependency?