Implement Callback API #10
Conversation
Input functions
Merge with main fork
Merge with main fork
Revert incorrect pull request
Merge with main fork
Note that this is not a direct implementation of the PaStreamCallback type. Instead it is a class to facilitate the cross thread communication between the portaudio thread and the node event thread.
This will prevent anyone from misunderstanding what the class is really doing.
The only thing that Nan::TypedArrayContents uses the template types for are the length attribute and the pointer type. Both are irrelevant in the current use case. The length attribute isn't used, and the PAReadStream and PaWriteStream functions both use void pointers as parameters.
|
For sure, I'm excited to test it out. Just glancing over the code it looks like an elegant solution to the callback problem. I'll dig in tomorrow and do some testing then get back with you. Thanks |
|
It's working great for me and the code looks good. I think it's good to merge. That being said, this pull request is against the callback branch which was divergent from master and has nothing of any use. Perhaps make the pull request against master as there should be no conflicts and we can get rid of the callback branch here. |
|
I just pushed a commit that greatly reduces the code complexity, and makes paFramesPerBufferUnspecified something achievable. It should also make non-interleaved buffers easier to handle. |
This greatly reduces memory operations, and negates the need for a mutex. It will eliminate the need for extra, complicated, code to implement paFramesPerBufferUnspecified.
|
Sorry, am resurrecting this pull request from the dead. Has there been any more thought to merging this pull request? Seems like great progress was being made. Would love to have the callback method integrated as well! Also would be happy to do some tests and have a look over things |
|
It would be an understatement to say I dropped the ball on this. Way back when the PR was opened it worked for me, should have been merged then. I'll need to run through things one more time to make sure that no changes within Node have caused this to not work. @tonetechnician If you wouldn't mind giving this branch a run through that would be helpful as well. |
|
Cool! I'll test it and get back to you within the next couple days with a report. I think it would be worth migrating from NAN to NAPI for the ABI stability. Will see if I can try do something like that. Don't think it would be too difficult! |
|
Cool. I actually managed to find some time quickly to check it out. This pull request is working on my end. I've tested the two added examples pa_api_callback_sine.js and pa_api_callback_wire.js My system: Would be great to confirm with later versions of node! Can try do this a bit later, but unable to at this point (need to get my nvm stuff sorted). I do see within the build that some NAN stuff has been deprecated. Definitely think porting over to NAPI is a good option for a next step in this project. I may be able to handle this upon which I will make a pull request. |
|
Unfortunately, doesn't seem to be working on my Node version (11). That's not just this PR, the module in full. I'll dig into why later (looks like maybe a removed interface at a glance). |
|
Oh man, that's bleak! I need to get my NVM going and then can check it out aswell. Is it to do with using NAN? I know on my node v8 I got a lot of build warnings saying NAN interfaces are deprecated. I really do want to port this over to NAPI, just need a bit of time - most likely in the next two weeks I'll be able to look into it. Just have some other deadlines coming up I need to get to first. I really want to use this in a project I'm working on, so I will definitely need to get to it sooner or later |
|
Yeah, I'm sure it's a NAN issue. Definitely some TLC needed here. I never ramped up the project I intended this for so it kind of just went stale. I'd like to get the PA libs updated as well. I'm going to try and at least get what's in place working some time this week, and maybe start on the PA updates as well as merging this. That way if you are able to look into NAPI in a couple weeks you'll have a better base to work from. |
|
All good man! Yeah, it would be amazing to get it updated to the latest PA lib! I guess the NAN stuff may take a bit more time.
Yes, would definitely be a better timeline to get the it working with the latest library and NAN, and then port. It shouldn't stop me from doing what I need to do though. I'll stay in touch here on the this pull request about when I start looking in to it |
|
Okay, I've got it reworked to accommodate API changes in v8 & Nan. That's in my |
|
You make quick sir! Nice one! I will check on my v8 node to see if it works. Will get my NVM up and running to speed things up aswell. Adding CI sounds great aswell, good call. I will be able to start looking into the NAPI port sometime in the next two weeks |
Creating a PR for review. This code should take care of Issue #2.
@Musocrat if you could take a look, I would be happy to work on any problems you can find. I would like an extra pair of eyes, and someone else to test this before I move forward.