search

mumble-voip / mumble

Mumble is an open-source, low-latency, high quality voice chat software.
https://www.mumble.info
Other
6.44k stars 1.12k forks source link

Force TCP Mode does not affect incoming UDP stream #4881

Open wfjsw opened 3 years ago

wfjsw commented 3 years ago

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):

Krzmbrzl commented 3 years ago

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 commented 3 years ago

Hi! Can I work on this issue?

Krzmbrzl commented 3 years ago

@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 :)

wfjsw commented 2 years ago

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.

Krzmbrzl commented 2 years ago

@wfjsw indeed. That's a separate issue though. Could you create a dedicated feature request for that, please? :)