lucidstack / ex-portmidi

Elixir bindings to the portmidi library
MIT License
35 stars 12 forks source link

Fix Hanging On Input Read #15

Open Kovak opened 5 years ago

Kovak commented 5 years ago

Just started working with this lib for a project I was on and was having segfaults, buffer overflows, and general hanging when trying to read my keyboard. Turns out there were a couple things wrong:

  1. When reading from midi, bufferSize was incorrectly being looked up as the third arg in a two arg list. In addition it was being decoded using the c -> erl encoding (enifmake*) instead of the erl -> c (enifget*). The type was also being used incorrectly, should be a long instead of an int I believe.

  2. Pm_Read was being used incorrectly. It can return negative values if it errors out, these were not being handled and were just being used as if they were positive integers, resulting in massive allocation requests and segfaults when the array was then created with a near max int number (from the negative number wrapping around as an unsigned int). These errors are now properly caught and passed out to be logged.

  3. Even when the above 2 issues were fixed, the lack of a sleep in between polling loop was locking up my device. I saw another PR for this but I didn't like how it did the sleep in C-space as A. the usleep function is not guaranteed to be in every platform, and B. It makes the nif more complicated and instead of just exposing the portmidi API introduced client-side logic to it. I have instead used elixir's :timer.sleep and exposed the amount to sleep as configurable. In my testing even a 1 ms sleep seems adequate.

thbar commented 5 years ago

Just a casual reader here. If I read correctly, point 3 is a different strategy to fix what's already described by @zenwerk in #13, but more configurable?

Also adding a comment right onto the PR.

olafklingt commented 4 years ago

I use Kovak's version. not that I really tested it but it seems to be more stable. what holds you back from merging the pull request?