search

Rantanen / node-mumble

Mumble client in Node.js
MIT License
155 stars 48 forks source link

Events in the client #102

Closed ghost closed 6 years ago

ghost commented 6 years ago

I've written a mumble bot using this great package, and it is for searching YouTube for songs and streaming the audio. It works great with one exception: other events on the mumble server cause disruption in the audio streaming. So if a lot of users connect/ disconnect or change rooms then it causes the audio to skip.

I'm wondering if you have any ideas for avoiding this issue. Is there a way to remove listeners or to deal with them differently? I looked through the source and have a few ideas, but if you have some pointers I think it would be helpful. Thanks!

Rantanen commented 6 years ago

Unfortunately this might be a limitation of Node's single threaded design. I'd expect most of the time to go into receiving and deserializing the protocol messages instead of triggering any events.

ghost commented 6 years ago

That was my intuition too. Do you think there would be anything to be gained from trying a multiprocessing solution (e.g., with cluster)? I have no idea how I'd implement it yet, but the general idea would be one process dedicated to streaming the audio from YouTube to the Mumble client's audio stream, and the master process would handle everything else. Feasible?

Thanks for the quick reply, by the way.

ghost commented 6 years ago

I suppose another alternative would be to make the functions that receive/deserialize the messages asynchronous? I hesitate to mention that, since I'm sure if it would be beneficial then you would have done it already.

Rantanen commented 6 years ago

Nah, the reason it's not async is because there was no async support in the protobuf library that was used previously. I don't know whether the new one has it.

Also blindly going into the codebase and turning things like the protobuf decode into async would most likely be a perfect example of misguided optimization. The real solution would be to profile this - unfortunately I'm not really familiar with the Node.js profiler.

Rantanen commented 6 years ago

Also multiprocessing might end up increasing the overhead instead of reducing it. The problem with the audio streaming is not high CPU usage, but close-to-real-time requirements.

Even with multi processing, there is still only one socket (well two, including the UDP socket) so any access to this would need to be synchronized across the processes.

ghost commented 6 years ago

Okay, I appreciate your thoughts on this. Probably I'll leave it there, since the skipping audio is a relatively minor problem. I may look into profiling the code, but I also don't have experience with profiling node. Thanks for your help!

ghost commented 6 years ago

I was curious and had a look at the protobufjs module. Apparently there have been some improvements in speed since version 4.13, although the interface of the module has changed. I'll attempt to figure out how to translate between the old and new versions to see if these improvements are noticeable. Although, I'm very unfamiliar with even the conceptual background of the module :) I'll do my best.