mumble-voip / mumble

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

Noise gate generating packet loss #4385

Closed blablablerg closed 3 years ago

blablablerg commented 4 years ago

Hi, I and my friends have discovered that applying a noise gate before the audio input to mumble seems to generate significant packet loss and possibly introduces short static noise bursts into the stream ('kchchk kchchk' sounds). I haven't fully verified the static noise part, so this ticket is about the packet loss, but we suspect that noise bursts are introduced because of the packet loss. This is especially noticeable at higher bandwidth settings in Murmur (bandwidth = 240000 in Murmur.ini). This is the only setting I have changed in murmur, the rest is default.

I have attached two samples, one with a noisegate applied and one without: noisegate_bug.zip

This is when playing back the sample through a virtual audio cable with no noise gate, it is just the room recorded with my microphone: image

This is when the noisegate is applied. I used Reagate (noise gate from Reaper) on the lowest attack and release setting, with the threshold at the noise floor to make the noise gate open and close at a high frequency. The less frequent the noise gate opens and closes, the less packet loss I see. image

Steps to Reproduce Steps to reproduce the behavior:

  1. Set bandwidth to 240000 in murmur.ini
  2. Start murmur on localhost to eliminate the network as a possible confounder
  3. Route the audio output of your PC to mumble with a virtual audio cable, and play back both attached samples.
  4. Check UDP network statistics in the connection information window. You should see a difference in packet loss From Client.

Alternatively, you can check live input with and without a noise gate applied to the stream, with low attack and release settings to make the noise gate 'stutter'.

My mumble audio input configuration:

image

Expected behavior No packet loss

Desktop (please complete the following information):

Additional context A friend of mine can also reproduce it on Catalina.

Krzmbrzl commented 4 years ago

Does the problem also occur with different bandwidth settings?

Is the used server the same version as the used client?

Is a noise-gate in principle just a noise suppression system? Aka it let's audio through that it thinks is not noise but blocks audio that it considers noise?

Have you tried increasing the audio per packet and/or decreasing the quality?

blablablerg commented 4 years ago

Hello Krzmbrzl,

Thanks for your quick reply.

Does the problem also occur with different bandwidth settings?

At lower bandwidth settings the problem is reduced (as stated in my original report).

Is the used server the same version as the used client?

Yes, as stated in my original report.

Is a noise-gate in principle just a noise suppression system? Aka it let's audio through that it thinks is not noise but blocks audio that it considers noise?

A noise gate attenuates the audio signal below a certain dB threshold, see https://en.wikipedia.org/wiki/Noise_gate. That is the audio effect I used. A noise suppression system is in my experience a more marketing kind of term, it can mean multiple things.

Have you tried increasing the audio per packet and/or decreasing the quality?

No, I haven't, because I am not looking for a "quick and dirty" fix. That would be for me to simply not use a noise gate and accept low volume noise passing through. However, I find it very strange that audio information in itself can produce significant packet loss on a connection no matter the settings, and I thought it worthwhile to report!

So I am interested if you guys can reproduce it and if yes, hopefully fix it, because a noise gate is a very handy and commonly used effect in (live) vocal recording and processing. This behaviour might also impact audio quality in other unforeseen ways.

mcdowell84 commented 4 years ago

Hi, I have reproduced the packet loss bug using only the mumble internal noise gate at maximum settings, no external noise gate. All other settings are identical.

Mumble Noise suppression set to maximum results in 8.5% loss, during this short test: image

Mumble noise suppresion turned off, zero packet loss: image

streaps commented 4 years ago

I wonder if this is real packet loss or if packets just don't get send by the client intentionally when there is silence (even in continuous mode).

Krzmbrzl commented 4 years ago

No, I haven't, because I am not looking for a "quick and dirty" fix.

@blablablerg This is not about finding a "quick and dirty fix". It's about figuring out what is going on so that eventually we might be able to fix it. So please go ahead and test it :)

Hi, I have reproduced the packet loss bug using only the mumble internal noise gate at maximum settings, no external noise gate. All other settings are identical.

@mcdowell84 By "noise gate" I assume you are referring to the "noise suppression" feature of Mumble in the Audio Input settings? Which Mumble version are you using?

I wonder if this is real packet loss or if packets just don't get send by the client intentionally when there is silence (even in continuous mode).

I was kinda thinking the same but this would be weird as the fact that it is shown as packet loss indicates that the packet numbering still increases so that the receiving client receives jumps in the packet's number. It could be however that the increasing of the counter is done before it is decided that the packet isn't actually going to be sent... :thinking: This however wouldn't explain why the server's bandwidth setting seems to have influence on the issue :thinking:

mcdowell84 commented 4 years ago

yes, noise suppression feature, under audio processing, set to -60dB and OFF.

version 1.3.2

Krzmbrzl commented 4 years ago

Alright thanks - will try once I find the time :+1:

Rickvlaar commented 4 years ago

I was kinda thinking the same but this would be weird as the fact that it is shown as packet loss indicates that the packet numbering still increases so that the receiving client receives jumps in the packet's number. It could be however that the increasing of the counter is done before it is decided that the packet isn't actually going to be sent... 🤔 This however wouldn't explain why the server's bandwidth setting seems to have influence on the issue 🤔

Also there is degradation of quality, so it seems that some packets aren't sent that do contain some audio.

Interesting to note is that issue occurs both with the internal noise suppression feature and third party noise gate software. Also we noticed decreasing the 'audio per packet' setting increases the number of packets lost.

I'm the friend mentioned in the bug btw ;) Thanks for checking this out!

mcdowell84 commented 4 years ago

Latest opus build contains a bugfix for something which seems related, discontinous transmission (DTX) of digital silence.

https://opus-codec.org/release/stable/2019/04/12/libopus-1_3_1.html

"This Opus 1.3.1 minor release fixes an issue with the analysis on files with digital silence (all zeros), especially on x87 builds (mostly affects 32-bit builds). It also includes two new features:

A new OPUS_GET_IN_DTX query to know if the encoder is in DTX mode (last frame was either a comfort noise frame or not encoded at all)"
Krzmbrzl commented 4 years ago

Hm... In theory though the codec itself shouldn't affect packet loss as this happens in the networking layer. Unless Mumble counts invalidly encoded packets as "lost" :thinking:

mcdowell84 commented 4 years ago

Yeah it's very puzzling. Something seems to be coordinated, and opus has packet loss features and packet loss concealment.

Opus developer says "This is only useful is you're writing a VoIP/videoconferencing application and you've asked Opus to stop transmitting when there's no speech. "

https://hydrogenaud.io/index.php?topic=117526.0

fatexs commented 4 years ago

I can confirm this, I am running Mumble on a local network <1ms ping. I did a UDP transmit test with iperf and 500Mbit bandwidth and had 0,0% packet loss.

However if I have Noise Reduction enable in Mumble Client, my packetloss gets reported at 24% but I get perfectly understood and my quality is good.

That suggests only non vital packages get discarded. Disabling Noise reduction reduces my packetloss to 0% Client 1.3.2 on Windows 10 Server 1.3.2 on Arch linux

iperf Edit: this is with server bandwidth at 128kbit/s so not much higher than default.

Rickvlaar commented 4 years ago

The quality degradation is not so bad that you won't be understandable anymore, but it does degrade the quality in a way that is disruptive.

This is a recording of the static that is generated by the issue: static_noise_example.wav.zip

Recording made in Mumble 1.3.2 as uncompressed .wav on 2020-08-26

mcdowell84 commented 4 years ago

I can also cause the same issue to appear if I turn the gain on my XLR interface (in which the mic is plugged) all the way down to 0 (thus causing silence): It causes massive packet loss, and other people on the server can hear the scratchy audio artefacts ('kcchhk kcchhhk' sounds). This is without any noise gate settings within mumble or on the computer.

So it seems that using an external volume control can also cause packet loss, which is very counter-intuitive :)

Krzmbrzl commented 4 years ago

I'm pretty sure that the issue is indeed not the noise gate per se but rather the way Mumble handles silent audio frames. There must be some logic (probably something that tries to be smart and to save bandwidth or whatever) that kicks in in these cases but doesn't recover properly from it :thinking:

mcdowell84 commented 4 years ago

yeah, or in how it interfaces with opus the opus codec also has bandwidth saving features

RX14 commented 4 years ago

I'm on mumble 1.3.2 and several people using rnnoise in the server i frequent have noticed this bug: hearing a very short period of static very infrequently. Having it be linked to packets with absolute silence makes sense.

TerryGeng commented 3 years ago

I remember the mumble uses a sequence number mechanism to detect if there's any missed packet. A possible cause for the "packet loss" may most possibly not a network fault, but the internal logic breaks somewhere.

I just want to make sure it is not caused by some delay caused by the audio process chain. Did you notice any kind of audio glitch or high CPU usage on the client PC where the noise gate is applied?

reelsense commented 3 years ago

I think I have the same issue. Try mumble 1.3.0 or 1.2.19, Mumble 1.3.1 and greater have the issue.

When I do a continuous broadcast on on any audio interface except my built in mic I get egress only packet loss.

blablablerg commented 3 years ago

@TerryGeng No, no high CPU usage or anything. You can try it out yourself if you want, I have included some audio files and a description on how to reproduce.

reelsense commented 3 years ago

I solved my client side egress only packet loss by using 1.3.0. Out of curiosity is there any transcoding done on the server?

Krzmbrzl commented 3 years ago

Out of curiosity is there any transcoding done on the server?

No. The server only relays the audio packets.

I solved my client side egress only packet loss by using 1.3.0.

Okay that's interesting. Does this mean that the first version that introduced the issue for you was 1.3.1? Or did you not test that one?

reelsense commented 3 years ago

Okay that's interesting. Does this mean that the first version that introduced the issue for you was 1.3.1? Or did you not test that one?

I tested 1.2.19, 1.3.0, 1.3.1 and 1.3.3 clients.

If I use ≥1.3.1 and I enable continuous transmission my outbound packet loss will immediately be 4-25% and from the receivers perspective it looks like I might have voice activation enabled, but on my end it's clearly a continuous transmission.

When I record mumble audio with software like Audio Hijack it scrapes the application audio and records the missing segments of my transmission as the receiver is evidently hearing it... I found 1.3.0 to solved this.


Recent Anecdote: I've hosted mumble servers for 10 years. I have years of experience with partial and full network outages, clients with bad wireless coverage, etc; so I like to think I'm pretty good at diagnosing mumble audio issues. The following experience seemed similar to the issue I just described above:

I recently recorded mumble audio when both clients were running 1.3.0 and the server is running the 1.3.3 PPA for Ubuntu 18.04 or 20.04. The reason I ask if there is any transcoding done on the server is, I actually experienced a similar episodes of missing audio transmission but only from the server to the client this time. But my recording method captured my mumble client audio with my full transmission. I connected with the mumble mobile client as an additional test and it looked like the continuous transmission was voice activation. Could this be an issue with 1.3.3 on the server too?

Screen_Shot_2020-12-17_at_20_07_28 Screen_Shot_2020-12-17_at_20_07_38
TredwellGit commented 3 years ago

On master if you set transmit to continuous and loopback to server then join a server: With noise suppression disabled there is no packet loss reported and no audio issues. Set to either Speex or RNNoise there are very load audio artifacts and packet loss. Set to both there is packet loss but much less audio artifacts.

https://github.com/mumble-voip/mumble/issues/4417 seems to be a duplicate issue.

TredwellGit commented 3 years ago

@Krzmbrzl @Johni0702 Bisect yields 6131499bc819433327ce4e1505f3ed70c499766a is the first bad commit.

TredwellGit commented 3 years ago

Whenever CryptStateOCB2::ocb_encrypt returns false, which it does very often, there are issues. https://github.com/mumble-voip/mumble/blob/4b533dce5252833aad452f2b39bcd1c305a58796/src/crypto/CryptStateOCB2.cpp#L279-L288

TerryGeng commented 3 years ago

I see. This patch is made to prevent a possible attack on OCB2. Interestingly, we prevent such an attackable situation by simply refusing to encrypt that package. The assumption of such a workaround is this attackable situation doesn't happen a lot, which is, as we do know now, apparently not true.

In the comment, it writes

For an attack, the second to last block (i.e. the last iteration of this loop) must be all 0 except for the last byte (which may be 0 - 128).

But on L284, it only detects the situation that all bits except the last bit are 0, but it doesn't care about the last bit itself.

I think in a lot of cases, zeros are formed at the end of the packet. If we modified L287 into

success &= (sum != 0) || (!plain[AES_BLOCK_SIZE - 1]); 

we will be able to encrypt a lot of packets that end with zeros.

Though I'm still wondering why zeros appear at the end of the packets.

Johni0702 commented 3 years ago

@TredwellGit Could you dump a few of the failing packets to check if the condition described in that comment is indeed true (as opposed to the implementation just being incorrect)? I.e. chunk it into 16 byte blocks, look at the second to last block, it'll be all 0 (except maybe for the last byte).

I assumed the chances of that happening by pure luck to be sufficiently small (cause why would there be 15 bytes of 0 when we just tried to compress something) as to be insignificant in practice but it doesn't seem too unreasonable that Opus just uses a bunch of 0s to indicate digital silence. If that is indeed the case, then I'm afraid this will be difficult to solve because we must never encode or decode such packets or we potentially become susceptible to the attack described in that paper. And our current voice packet format doesn't really give us enough flexibility to universally prevent such packets from ever being generated. The only two workarounds I can see (proper solution being to ditch OCB2) are to add dummy positional data (though it'd have to be as much as 17 bytes) or making sure we never actually encode digital silence.

it only detects the situation that all bits except the last bit are 0, but it doesn't care about the last bit itself

(you meant to write byte instead of bit here) That is because the last byte may have any value from 0 to 128 for the attack, so if we allow 1 - 128, we're susceptible. We could allow 129 - 255 (need to be very careful in implementing that though) if it happens that our packets have that value, but we'd have to check if that's indeed the case (and I doubt it).

TerryGeng commented 3 years ago

@Johni0702 Thanks for the explanation. So the only solution is to prevent producing packets that end with zeros?

Johni0702 commented 3 years ago

@Johni0702 Thanks for the explanation. So the only solution is to prevent producing packets that end with zeros?

For encryption the message is chunked into 16 byte blocks, it's the second to last block which is important, the last one (which may be less than 16 bytes) doesn't matter (iirc). Otherwise, yes.

TerryGeng commented 3 years ago

So everything goes back to the original point: why don't we move on to another better encrypt method....

Johni0702 commented 3 years ago

So everything goes back to the original point: why don't we move on to another better encrypt method....

We definitely should cause the workarounds will just keep piling up but unless we want to immediately break backwards compatibility, some workaround is still needed.

our current voice packet format doesn't really give us enough flexibility to universally prevent such packets from ever being generated

Looking at the Opus spec, it seems like Opus itself does have enough flexibility for us to add padding bytes (see section 3.2.5 "Code 3 packets"). It also says The decoder MUST accept any value for the padding bytes, meaning we can put non-0 values in there, even though by default an Opus encoder would put 0s. So for the small price of 3-19 bytes added only to offending packets and a slightly complicated, medium size piece of code which converts code 0, 1 and 2 packets to code 3 ones, we could fix this is a backwards compatible way (at least of Opus).

Actually, thinking more about it, this might already be the source of the 0s (Mumble uses Opus in Constant Bit Rate mode, see here, therefore Opus probably already adds padding to the packets). And if that's indeed the case, then the fix would be as simple as checking that it already is a code 3 packet (if it isn't, we'll have run into what I hope this time really is the improbable case), then reading the length of the padding (so we don't overwrite real data), and finally just flipping a few bits in there until the packet may be encrypted again (e.g. changing the entire padding to be 1 instead of 0).

TerryGeng commented 3 years ago

Personally, I'm against workarounds that will alter the format of the packet on the level of the audio codec. We will (and must) support more cipher in the future. Designing a very specific workaround for OCB2 is against this target.

Now that the old client was designed the way that doesn't have the ability to prevent this kind of attack, what we are doing is purely for further clients. Therefore, why don't we expedite the process of getting new encryption methods working and just leave OCB2 as it is before this problematic patch? We can still support OCB2, but in the next release, we should note that OCB2 is not secure (in theory) and encourage people to use the new encryption method as discussed in #3918. I would also resume the work on #4299.

TredwellGit commented 3 years ago

plain in CryptStateOCB2::ocb_encrypt in hex often looks like:

80 30 78 F0 FF FE 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0  <-- Second to last block all zero.
0 0 0 0 0 0 0 0 0 0 0 
TredwellGit commented 3 years ago

https://github.com/mumble-voip/mumble/discussions/3637

Johni0702 commented 3 years ago

Now that the old client was designed the way that doesn't have the ability to prevent this kind of attack.

Not entirely sure what you mean by "client" here. The client does have a way to prevent this kind of attach (you linked the commit above). It's also possible to fix the issue introduced by that commit in a backwards compatible way such that updating your client to hypothetical 1.3.4 fixes the issue while still preventing this kind of attack.

what we are doing is purely for further clients.

Obviously it is, cause we can't time travel. The workaround I'm proposing would be for 1.3.4 though and fully backwards compatible. A new encryption method would require everyone, including the server, to update to 1.4 (which may be more cumbersome than just a point release) before it fixes the issue. Having the workaround and then allowing everyone to upgrade at their own pace (instead of all at once) seems better to me.

leave OCB2 as it is before this problematic patch

I am fundamentally opposed to releasing insecure-by-default (or even by uninformed choice) software. Therefore I do not consider unpatched OCB2 to be an option. Just because I couldn't immediately think of any way to exploit the issue given Mumble's feature set at the time, doesn't mean that there isn't one or that there won't be one when new feature X will be introduced (and no one will double check after every new feature that OCB2 is still secure with it).

plain in CryptStateOCB2::ocb_encrypt in hex often looks like:

80 30 78 F0 FF FE 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0  <-- Second to last block all zero.
0 0 0 0 0 0 0 0 0 0 0 

Thank you very much for checking! That's 117 0s. If I'm not mistaken, the start can be read as follows:

Given it's not a code 3 packet, that throws out the simple workaround (leaving only the complex (converting to Opus code }) or the ugly (never allow for digital silence) one) and my attempt at finding a plausible explanation for why the 0s are there in the first place (at least on the Opus frame layer, would probably have to go deeper).

TerryGeng commented 3 years ago

I am fundamentally opposed to releasing insecure-by-default (or even by uninformed choice) software.

But as you have said, we cannot time travel back, so the old client is still vulnerable. We may encourage 1.4.0 users to use new encryption methods but it also means saying goodbye to old clients. As long as we are going to continue supporting the old clients with OCB2, we have to deal with this problem.

Personally, I think we can always allow users to choose. We don't release broken (insecure-by-default ) software since we are going to support more secure encryption methods in the next version, but if they are going to use weak encryption to deal with the old clients, they are implying that they accept the risk. They can also decide to ban vulnerable clients using OCB2.

There are examples like TLSv1.1 and the older TLS version, which is vulnerable in principle but is still supported by some browsers. But users can always be warned about the risk, or the browsers can always be set to refuse to access such sites.

Alas, I feel like this thread is going to be another argument graveyard that people start to attack each other's fundamental beliefs and principles. I'm not planning to continue this discussion in this direction.

TredwellGit commented 3 years ago

Right now, the force TCP mode option can be enabled to avoid both the packet loss and using OCB2.

Johni0702 commented 3 years ago

But as you have said, we cannot time travel back, so the old client is still vulnerable.

The current release (1.3.3) is not (and it's even sufficient for either client or server to be using 1.3.3). Releasing a new version (1.3.4) without the patch, would make that newer version (and we can all agree that it's generally good to keep your software up-to-date) vulnerable while the older one was not. So at that point we would have released insecure-by-default software. Even if the new version is 1.4.0 and it supports and uses a new encryption method by default if the server supports it, it'll still be insecure by default if it falls back to unpatched OCB2 when used on an older server (and that is not the case with the current 1.3.3).

We may encourage 1.4.0 users to use new encryption methods but it also means saying goodbye to old clients.

I wouldn't at all mind 1.4.0 not supporting OCB2 at all (falling back to TCP on old servers, I previously forgot that was an option, so not everyone would have to upgrade to 1.4 at the same time). The only problematic course of action imo is releasing a new version with support for unpatched OCB2. If we do not drop OCB2 immediately, then we should keep the patched / buggy variant (whether or not we add a workaround for the bug), and just encourage people to update client and server to 1.4 as the solution to the bug.

Personally, I think we can always allow users to choose.

I agree in almost all cases except for security cause people tend to just not make informed decisions where it matters. There's a reason browsers made it really difficult to bypass the invalid certificate page. Took way too long for me to notice that my server certificate had expired cause everyone just kept silently kept ignoring the warning.

Anyhow, we didn't want to get stuck on the fundamentals, all I'm trying to say is: keep the bug and tell people to update to post-OCB2 1.4 to fix it, rather than converting it into a security vulnerability.

Krzmbrzl commented 3 years ago

Reverting the workaround without providing a new (better) one is not an option. At least not as long as we have some ideas as to how this could be fixed in a different way.

Dropping OCB2 support all of a sudden completely is also not really an option (TCP mode would work but there's a reason why UDP is normally used). Imo the correct correct way would be to make something else the default so that a few releases down the line we can finally drop OCB2 since by then we can assume that most clients and servers support alternatives.

@Johni0702 did I understand this correctly that the issue occurs if we encode a packet that is 100% silence? Do you think we could just flip a single bit to be a 1 instead of a zero? I would assume that this would affect the underlying audio in only a negligible way and if that's what's needed for the bug to be fixed while providing safety from the OCB2 attack, I think that might be an acceptable price :thinking:

Johni0702 commented 3 years ago

@Johni0702 did I understand this correctly that the issue occurs if we encode a packet that is 100% silence? Do you think we could just flip a single bit to be a 1 instead of a zero? I would assume that this would affect the underlying audio in only a negligible way and if that's what's needed for the bug to be fixed while providing safety from the OCB2 attack, I think that might be an acceptable price thinking

This issue occurs if the audio in encoded form has lots of 0s (specifically if the second to last AES block is all 0). This appears to reliably be the case when the input audio is 100% silence. I don't know enough Opus internals to know whether 99% silence will produce a vastly different result, but if it does (and that seems likely), then that would fix the bug. Unless someone happens to know someone familiar with Opus internals, the easiest way to find out would be to just implement some tiny amount of noise and see if that fixes it.

TerryGeng commented 3 years ago

@Johni0702 did I understand this correctly that the issue occurs if we encode a packet that is 100% silence?

I would also wonder why dropping silence packets causes glitchy sound.

I think as long as you secretly flip bits in the ocb_encrypt function, we are good. It wouldn't affect other parts of the codebase right?

What worries me is if there is a checksum mechanism in the compressed frame, but a brief glance through the RFC page shows there is no such mechanism.

TerryGeng commented 3 years ago

If we do not drop OCB2 immediately, then we should keep the patched / buggy variant (whether or not we add a workaround for the bug), and just encourage people to update client and server to 1.4 as the solution to the bug.

Can be a solution (strategy) as well. Looks good to me.

Johni0702 commented 3 years ago

I would also wonder why dropping silence packets causes glitchy sound.

I'm guessing it's because pretty much all the silent packets are being dropped, so as far a the receiver is concerned, we aren't silent and the packet loss concealment algorithm uses non-silent frames to do its concealing.

I think as long as you secretly flip bits in the ocb_encrypt function, we are good. It wouldn't affect other parts of the codebase right?

Flipping bits in the already encoded audio could result in vastly different decoded audio (like if the bit you flipped was in the MSB, or the index into some lookup table; we don't even know if we're looking at Opus by the time we're in ocb_encrypt), so if we want it to be unnoticeable, we gotta do the flipping before audio encoding, where we know that a change from 0 to 1 in a range of -2^15 to 2^15 is practically not noticeable.

TerryGeng commented 3 years ago

if we want it to be unnoticeable, we gotta do the flipping before audio encoding, where we know that a change from 0 to 1 in a range of -2^15 to 2^15 is practically not noticeable.

This approach (tweaking audio before encoding) is not neat IMO. We are going to gradually move away from OCB2. If we make an OCB2-specific workaround that messed up with other parts of the code, it doesn't sound very wise. Anyway, distorting audio even in a very subtle way, just to avoid a vulnerability in the encryption method, hmmmmmm....

If we are going to go with the after-encoding bit flipping strategy, I think maybe we can just test several positions to see which one won't mess things up too apparently.

But I believe the best way is to expedite the cipher negotiation protocol and add more encryption algorithms in the next release and mark OCB2 as obsolete. I'd really love to see progress on this aspect.

blablablerg commented 3 years ago

For what it's worth, I managed to circumvent most of the packet loss by adding a sine wave of 20 hz at around -70dB to my audio stream. The packet loss dropped to about 0.1%. A sine wave around that frequency and amplitude is probably only noticable if at all on PA system with huge subwoovers and is easily removable by using a high pass filter if needed.

Modifying the audio stream itself is probably not something you want to do, but for the time being it can be practical as long as this change is reverted when the encryption scheme is upgraded.

TerryGeng commented 3 years ago

@blablablerg Currently, I think you can just use force TCP mode. OCB2 is only used for UDP transport. TCP is free from this problem.

blablablerg commented 3 years ago

Doesn't TCP induce latency? We use mumble to remotely record a podcast, so we want our bad jokes to be as timed as possible :)

These days I don't use a noise gate anymore, I just let the mic noise pass through so there never is digital silence, so I don't suffer from this problem. But some hardware or software can have a noise gate on by default without the user knowing it, that is why I think some people still experience this bug.

reelsense commented 3 years ago

I have this problem with TCP.

I don't have this problem if I use mumble-server 1.3.0 in the Debian 10 repository and Mumble 1.3.0 client.

Krzmbrzl commented 3 years ago

Doesn't TCP induce latency?

Yes it does. That's why I don't think this is a good solution to force on everybody.

@TerryGeng modifying the audio is not optimal but from the options that were present here so far this is by far the best imo. Moving away from OCB2 all of a sudden is just not possible and forcing everyone that wants to communicate with a slightly older client to fall back to TCP is also not an option. Thus the change has to be done gradually and therefore we need a working workaround to bridge the time until OCB2 can be dropped. And I don't see issues with some OCB2-specific code being introduced as long as it is clearly marked as such so that once OCB2 is removed, we can remove that as well.

I have this problem with TCP.

@reelsense are you 100% sure that you are using TCP for audio transmissions when this happens? :thinking: