Gielert / NoodleJS

A NodeJS Mumble client
36 stars 14 forks source link

Implement receiving and decoding (opus) voice data packets #30

Closed kripton closed 4 years ago

kripton commented 4 years ago

More or less a request for comments since it's not tested pretty much. But it's working perfectly fine for me with Opus-only clients. I didn't test whispering at all but audio from a channel where client is not joined but listening to is working :)

Gielert commented 4 years ago

It would be nice to eventually change the implementation to have a ReadableStream Receiver like how Dispatcher is a WritableStream. But for now it will suffice.

kripton commented 4 years ago

About the Streams: I do agree that this would be desirable. I've also looked at the Dispatcher code and it looks like it's above my current skill level. Of course I could learn but I'm not even sure on how to test the implementation. For the moment I will leave it like this and if the code that I'm needing it for would work better with Streams, I might re-think that. Another point: Since several users could be talking at once, I assume we would be having one Stream per user? We need to check when to create/open those streams and then to close them.

About the warning on packets encoded with non-Opus: I will add a warning but not on every voice packet. I'll think about something, maybe taking in account the sender and sequence numbers.

kripton commented 4 years ago

Implementation done. Since I cannot test this (I don't have an old Mumble that can still send Celt), @Gielert could you please check if what I did makes sense to you? Thanks :)

Gielert commented 4 years ago

lgtm, I'm gonna merge this, and eventually contemplate further on this. Thanks for the awesome work!