-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AudioFork does not work on Asterisk 19 #14
Comments
Thanks for filing this issue. I have yet to test the module with Asterisk 19. But a quick review could suggest that its config related and maybe something with the WebSocket server. In a nutshell, some older Websocket libraries are incompatible with the module and many changes made to address this issue. I am not sure if this is exactly the issue, but it is highly likely that something such as the WS library is causing this issue. Just so I can understand this better: |
Hi, It seemed to be a server-side issue. My first attempt was connecting to a php-websocket server which was not working. I switched to Net::WebSocket::Server which works great. I had to fix a few issues with the code:
I'll get a pull request going for you. |
Hi, Also. I am a part-time developer for the Asterisk Project. I primarily maintain the AEL module. I am interested to get this audiofork module of yours officially added to the Asterisk Project. Would you be willing to sign a (free) developer agreement with Digium/Sangoma so we can include this module in Asterisk? Thanks! |
I am very open to this. Please send the details and I will have a look |
Hi Nadir, Here you go: Then submit the developer agreement: You'll want to review this: As well as the coding standards (the main thing being Asterisk uses tabs instead of spaces, so the code will have to be re-intended and change every 2 spaces to one tab) |
A few additional things should be taken care of as well.
|
@kobaz I will have a look at this over the next few days. Thanks! |
@kobaz Reviewed it. And agreement is done. I'll be looking over the coding changes shortly. They shouldn't be too complex. |
Hello, Just looking into this now, and I had a few quick observations. Firstly, based on the code right now, a routine for reconnections would be enhancement but not critical to the overall functionality. I will consider this if you really think you need this, but it seems to be a small upgrade and not crucial to how it works. Right now, the script creates a connection to the WS server each time a new channel is created (or when the app is called), and there is one connection per channel. If you want to add reconnection support, can you please let me know where you want to add it. For example, are you suggesting that we try to reconnect before we create the first connection, or should the app try to reconnect if connections are lost ? I would need to know more aboout this. Secondly, referring to the test suite, it doesnt seem like this will be so complex so I can look into it. I will have to study the Asterisk core and see how tests were done there, but integration wise, it can be added. If you can get back to me about the reconnection part it would be much appreciated. Thanks |
HI Nadirhamid, For production-readyness and robust behavior there should be options to avoid data-loss, such as more than one reconnect, and possibly storing data on disk. In terms of reconnection support... if the remote websocket server becomes unavailable, the audiofork module should keep trying to connect, with exponential timeouts. Up until an upper limit. Example 1:-
Bonus features (not required, but would be nice to have)
|
@kobaz sorry been delayed with this. However I did create a branch and I am currently testing some of these changes. I will keep you notified of progress. And as soon as there is a working version ready, I will update this thread. In the meantime, if you have any other queries or need more info please let me know. Thanks |
Just to add on this: Older & possibly Certified AST is not sticking RFC6455; You need a recent version of Asterisk. |
Thanks for the updates. I'm going to work on getting this posted up on the reviewboard for official Commit. Do you have any pending work to commit? |
AudioFork leaves dangling channel references after channel hangup
Also...
Server-Side tcpdump (all data for port 8888):
There is no more data... despite the call completing and going to MusicOnHold... the audio stream is never sent.
The text was updated successfully, but these errors were encountered: