mumble
mumble copied to clipboard
mumble-voip
Force TCP Mode does not affect incoming UDP stream
Describe the bug According to https://github.com/mumble-voip/mumble/blob/d6f9e97ad64828066d5134c9d87e2673847eed79/src/mumble/ServerHandler.cpp#L557
the third true parameter essentially disable the TCP mode check here: https://github.com/mumble-voip/mumble/blob/d6f9e97ad64828066d5134c9d87e2673847eed79/src/mumble/ServerHandler.cpp#L277
Which in this case, client keeps sending UDP ping regardless of TCP mode, trickling server thinking the client is capable of receiving UDP packet and keep sending them, rendering the option useless.
Expected behavior A clear and concise description of what you expected to happen.
Currently when this mode is enabled, only outgoing voice stream goes through TCP tunnel. Expected behavior is the incoming stream also passes through it.
Desktop (please complete the following information):
- OS: Windows 10
- Version: Mumble 1.3.4, with a custom server based on Grumble.
I guess that this code block should not get executed if TCP mode is forced in the first place: https://github.com/mumble-voip/mumble/blob/d6f9e97ad64828066d5134c9d87e2673847eed79/src/mumble/ServerHandler.cpp#L552-L558
That should also fix the issue at hand here.
Using the force parameter for the UDP ping is important to ensure that UDP pings will actually end up being sent over UDP and not via TCP. But sending UDP pings at all while TCP mode is forced, does not make a whole lot of sense :thinking:
@kwonhur Absolutely! I would recommend creating a draft pull request very early during developing so we can assist you with any questions you might have during working on this. Of course you can also ask questions here in this issue, if you prefer :)
Actually now I think there is a point in only using TCP for outgoing stream while keep using UDP for incoming. Some field test shows that clients are fine hearing via UDP but facing problem speaking consistently. Might worth an extra option or a dropdown select.
@wfjsw indeed. That's a separate issue though. Could you create a dedicated feature request for that, please? :)