mumble-voip / mumble

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

RFC: remove CELT 0.11 for 1.4.0 #1999

Closed mkrautz closed 6 years ago

mkrautz commented 8 years ago

I propose we remove CELT 0.11 from 1.3.0.

Thoughts?

Kissaki commented 8 years ago

We still have CELT 0.7 as baseline, and Opus has been in for some time now, sure, lets remove it. Especially considering there is no upstream for CELT any more, that’s a good thing.

funkydude commented 8 years ago

What's the benefit of removing it?

Kissaki commented 8 years ago

Less code to maintain and compile (on builds). The burden of maintenance is especially heavy for CELT because there is no upstream any more (well, there is, but bear with me). CELT has been superseded by Opus, and as pre-1.0 is still labeled experimental and thus not maintained. So should there ever be a problem with CELT (for example a security issue in the library), we have to maintain that ourselves. Just having to maintain 0.7 instead of 0.7 and 0.11 would ease such a case.

Although we do not expect this to occur, if we expect people to use Opus anyway, we could just drop that risk by dropping 0.11 for our next stable feature release 1.3.

Reducing our code and compile footprint is very nice as well.

funkydude commented 8 years ago

Fair enough. I assume you're choosing to drop 0.11 instead of 0.7 to maintain compatibility with really old apps?

hacst commented 8 years ago

Only in favor of dropping it in 1.3 if we actually have working and widespread Opus support out there (client + servers). Otherwise it'll be an instant quality regression if users use the default settings.

@funkydude: 0.7 is our baseline codec for the 1.2.X series (old meaning ;) Now basically 1.X series). As such we cannot drop it without completely breaking backwards compatibility. While the vast majority of clients (all on windows and osx) support 0.11 and it is in every way superior to 0.7 it's just an optional addon we can get rid of.

Kissaki commented 8 years ago

If 0.11 has hearable better quality, we should make sure to have overwhelming Opus usage.

Celt was introduced in 1.2.0 December 10, 2009. The 1.2.1 page states

Updated CELT codec with enhanced packet loss concealment (less robots)

which is the 0.11 addition I guess. Not sure how much packet loss makes this noticeable!? Opus was introduced in 1.2.4 June 1st 2013.

According to our stats, the overwhelming number of users is > 1.2.4. 9th rank is 1.2.3 with 7344, and then rank 12 1.2.2 with 494.

I think that’s ok. It is just 2% of our userbase.

hacst commented 8 years ago

Unless I'm misremembering 0.7 to 0.11 made a noticeable difference w.r.t to low bandwidth quality even outside of PLC scenarios. Also have to consider that one user with an old client will pull down the whole server to 0.7 unless opusThreshold is configured against it (still 2% would be low). For Opus I'm mostly concerned about correctness of implementation in what is out there. Afaik the old stuff was a bit rough (server and client) even outside of the screech bug.

Then again I'm probably a bit to conservative here and we should simply get people to update as quickly as possible now and going forward. Only people who really can't do that easily are 3rd party client users and Linux users using the distro version.

funkydude commented 8 years ago

simply get people to update as quickly as possible

That's not going to happen until we have the auto updater in place :D You need to drag people kicking and screaming just for minor security updates. Funny thing is Linux distros actually have it better in that regard.

crknadle commented 8 years ago

Removing CELT 0.11.0 should be fine. Although most clients support it, the mumble package in Debian does not and hasn't for at least 2.5 years, and we've never gotten a report of a backwards compatibility issue. [Removing CELT 0.11.0 support was part of the Debian Technical Committee decision in #682010.]

funkydude commented 8 years ago

Doesn't look like anyone has any objections.

ghost commented 8 years ago

@funkydude Please don't do this until you fix bug https://github.com/mumble-voip/mumble/issues/957. I keep my server on CELT because of that bug. That means the quality will drop if you remove CELT 0.11.0.

davidebeatrici commented 6 years ago

Superseded by #3484.