Fix for euclidean sequencer #89
Conversation
|
oops, forgot that I also committed some code to fix a compilation issue with GLFW, I am running Ubuntu 16.04 and I think my version is too old, even if it is found by cmake there are unknown references afterward. Hence I reverted to the older behavior: just compile it from scratch. |
|
Yep, the Euclid is very much a work in progress, bordering on a proof of concept. It was thrown together in a few hours. Thanks for the filter fix though, I missed that one. I think (think) the recording part mostly works as we intended it. Yes there needs to be a way to disable the sequencer, and to start/stop it Also a new UI is coming, so that's great! |
| @@ -204,7 +203,8 @@ namespace otto::engines { | |||
| ctx.beginPath(); | |||
| ctx.fillText( | |||
| util::join_strings(util::view::transform( | |||
| util::view::filter(current.notes, [](char note) { return note >= 0; }), | |||
| // extra safe on the note value to avoid overflow | |||
| util::view::filter(current.notes, [](char note) { return note >= 0 && note < 128; }), | |||
| [](char note) { return midi::note_name(note); }), | |||
There was a problem hiding this comment.
how can the note ever be greater than 127? its a signed byte...
There was a problem hiding this comment.
Hum, this is probably due to my poor comprehension of the "newest" C++ syntax, I was not sure how the declared type were applied. Now I think I get it better, indeed < 128 is over the top. However in term of "extra safety" I considered adding a if (note >= 0) return midi::note_name(note); else return "MISS"; } since it would not crash in case the filter iterator fails (e.g. nice for unit testing).
There was a problem hiding this comment.
note == -1 denotes a empty field in the array. I just use an array instead of a vector because its fine, and we avoid allocation.
There was a problem hiding this comment.
Yes, I got that :) Note that I am currently trying to resolve the remaining issues I mentioned in my branch, I will create a new pull request (since this one is now locked) when I finish.
| # target_link_libraries(otto ${GLFW_LIBRARIES}) | ||
| # | ||
| # #target_link_libraries(glfw INTERFACE dl) | ||
| #else() |
There was a problem hiding this comment.
we should fix this instead of removing it
There was a problem hiding this comment.
Definitely, it was a hack not meant to appear here ^^
Hello,
I could not try the brand new sequencer, the program crashed as soon as I tried to record anything. After some investigations it seems that the filter iterator behaves badly, i.e. even when no element fulfills the condition one is still being returned upon iteration, which triggers a buffer overflow down the line when the name of the midi note not is being read. My fix might not be the most elegant -- a looong time since I coded seriously in c++ -- but now the first element will point to a valid one upon init (or to the last element).
There are other oddities with the sequencer (e.g. a 0 length provokes a floating point exception + I am not sure I understand how are supposed to work new recordings -- I would reset all the notes each time), I don't see how not to use the sequencer and directly play the synth, but I guess you are aware of that and I get that this is a heavy work in progress. At the very least now I can start toying with OTTO!
Hope it helps, keep the good work :)