mumble-voip / mumble

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

Use Opus FEC (Forward Error Correction)? #4519

Open nh2 opened 3 years ago

nh2 commented 3 years ago

Opus has an in-band FEC (Forward Error Correction) feature that helps with packet loss (at cost of of a bit more bandwidth).

It works by "including a coarse encoding of the previous packet in the next packet" (see page 13 of this presentation).

It may be beneficial for Mumble to use that to improve audio quality under packet loss. Mozilla did an experiment on the effectiveness of Opus FEC including audio samples.

How to enable FEC

FEC is quite easy to enable as described in https://ddanilov.me/how-to-enable-in-band-fec-for-opus-codec/:

Sending side

Simply enable it with a flag here:

        // Forward error correctoin
        oCodec->opus_encoder_ctl(opusState, OPUS_SET_INBAND_FEC(1));
        const int expected_packet_loss_percent = 50; // example
        oCodec->opus_encoder_ctl(opusState, OPUS_SET_PACKET_LOSS_PERC(expected_packet_loss_percent));

Receiving side

To decode a missing packet by using the FEC data in the next packet, you call opus_decode_float(.., decode_fec=1) on the data of the next packet.

Mumble's existing call to opus_decode_float() is here.

Open questions

I've played around a bit with implementing this on top of Mumble 1.3.2, but have not succeded yet. Questions:

  1. How would I get at the "next packet"? Mumble uses the Speex jitter_buffer on Opus packets, and that one's API only allows you to pop out the next packet, not to peek at the subsequent packet.
  2. (How) does the Mumble server side have to be changed? Are there limitations on packet size or kbps that need to be lifted for the FEC-increased packet sizes to pass through? The server only forwards packets and does no decoding/mixing, is that still accurate?

Thanks!

Krzmbrzl commented 3 years ago
  1. I'm not sure if this is possible with the way packets are currently handled. Maybe @davidebeatrici has an idea about this :eyes:
  2. The server only relays the packets and does not performing any decoding (or encoding). Atm there is a limit on the packet size though (max 1024 bytes per UDP packet - this is part of the Mumble protocol). See https://github.com/mumble-voip/mumble/issues/4351 for our plans in that regard.

In general anything that can improve the way Mumble handles packet-loss is would be very welcome. So if you get this to work, that'd be great :+1:

davidebeatrici commented 3 years ago

Should be pretty easy to implement and only client-side changes are required.

What happens when a client sends a non-FEC packet though (backward compatibility)? Is decoding still possible?

nh2 commented 3 years ago

What happens when a client sends a non-FEC packet though (backward compatibility)? Is decoding still possible?

Yes, according to http://lists.xiph.org/pipermail/opus/2013-January/001904.html

If no FEC is available in the packet, normal PLC is used instead.

where PLC is Opus's normal Packet Loss Concealment.

The link has another useful statement:

If you encode with the same target rate with FEC on and off, then while FEC is on, the encoder will reduce the rate of the non-FEC portion in order to achieve the same total rate.


@davidebeatrici Do you have a tip how one should get at the "next packet" from the jitter_buffer?

trudnorx commented 3 years ago

FEC should be optional because it adds delay (see http://lists.xiph.org/pipermail/opus/2013-January/001904.html) and a major Mumble goal is to be low latency. Today, many Internet connections have extremely low packet loss. I do not experience audio glitches while using Mumble so for me there would be no benefit from the option and only drawbacks. However, it could be useful for people in some environments.

I suppose making it optional means that OPUS_SET_INBAND_FEC and OPUS_SET_PACKET_LOSS_PERC are only set from the sender side in case they turned FEC on, and from the receiver side opus_decode_float should only be called with decode_fec=1 on packets from users that have FEC on.

ancarda commented 3 years ago

FEC should be optional because it adds delay. Today, many Internet connections have extremely low packet loss.

This is true for wired Internet, but less true for wireless and satellite Internet.

I agree most people would benefit from not using FEC when packet loss is low -- so latency can be lower -- but wouldn't it make sense to also have FEC available as an option? In particular, when using Mumble on cellphone? I have friends who hang out on Mumble, and when we're not playing games together, I'd like to go on a walk and chat to them, but 4G packet loss is actually too high and people complain they can't hear me

I believe FEC would help a lot with that

J0s3f commented 5 months ago

Opus now even has Machine Learning based deep-plc in the decoder and Deep REDundancy (DRED) in the encoder: https://opus-codec.org/demo/opus-1.5/

Mumble should compile them in the Opus library and provide options to enable them.