Bug#682010: [mumble] Communication failures due to CELT codec library removal
On Tue, Jul 24, 2012 at 06:25:04PM +0100, Ian Jackson wrote:
> Ron writes ("Bug#682010: [mumble] Communication failures due to CELT codec library removal"):
> > I understand your line of thinking there, and for 99% of the code in the
> > world, I'd be in complete agreement. I'm not someone who is afraid of
> > code, or of getting my hands dirty in it, but we're not talking about
> > simple programming errors here - if and where there are problems, they
> > are in the boundary conditions of some very deep math that often still
> > confuses the people who created it until they stop and think very hard.
>
> In order to support this in wheezy we do not need to be able to fix
> general bugs in the codec or analyse and understand its signal
> processing behaviour.
I wasn't trying to imply that, and agree.
> We have only to be able to fix security problems, and it is normally
> possible to find and fix such security problems without needing to
> understand the purpose of the signal processing algorithms; typically
> they would occur (as with decompressors) in the handling of invalid
> packets.
This however is only 'trivial' if someone hands us a trigger stream
on a platter - and I don't believe that anyone is in the habit of
recording raw mumble protocol streams that their client receives.
Since nobody else is using this, our odds of a friendly agent stumbling
upon the problem first are greatly reduced, and in the event of such
a problem it would still require a sufficiently deep understanding to
avoid introducing new problems with any fix.
None of this is impossible, but nobody has volunteered to take on the
role of stewarding this.
> Looking at some sample diffs there seem to be a lot of variable and
> type name changes and so on, as well as the algorithmic differences,
> so a diffstat doesn't give an accurate picture.
Nobody has really been just renaming variables for fun and profit, so
I suspect you'll find most or all of those cases are also tied to some
semantic difference in meaning and/or use as the codec evolved.
> > > It's been incorporated as a key part of opus, renamed and
> > > developed. So celt 0.7.11 is really best seen as an old, pre-release,
> > > version of opus.
> >
> > There is a sense in which you are 100% correct there. But it is also
> > the same sense which gave us the aphorism:
> >
> > "This is Ned Kelly's original axe."
> > (only the handle has been replaced 5 times and the head twice)
>
> Having eyeballed some diffs I don't think this is a fair
> characterisation at all.
Aside from the basic MDCT and PVQ, almost everything has changed at least
once, some things several times. Things have been tried and discarded,
and new things have been added. Though to be fair, surely not all those
things will be reflected in this single diff between two versions.
They are near the outer ends of all these changes though. And the encoder
is _still_ evolving (since only the decoder and bitstream are normalised,
improvements in encoding are ongoing). Again by way of mitigation though,
we will hopefully be mostly only concerned with the decoder, since that's
the obvious open vector for a remote exploit.
> There is no need to backport anything other than security fixes. As I
> say, sorting out security fixes does not involve anyone having to
> understand FFTs.
Actually the FFTs need to go away entirely, just nobody has got around
to actually optimising that code yet (but that's a total aside :)
> > And while that may not look like the most horrifyingly large diff that
> > has ever been sent to -release as a minimal 'harmless' proposed fix,
> > let's put it into perspective as a proportion of the total codebase:
>
> Currently celt 0.7.1 is in wheezy. Its removal is blocked by the fact
> that stripping it out introduces the RC bug in mumble.
>
> The proposed fix involves moving the source code for celt 0.7.1 from
> one source package to another, not introducing it newly into wheezy.
Yes, I wasn't implying this was the diff -release would need to review,
just that for people used to looking at half-million line diffs sent
requesting a freeze exception, it may not look very daunting on number
of lines changed alone. This was solely directed to making it clear
that the maintained opus codebase was not just some minor incremental
change to celt 0.7.1. It carried on the name for the MDCT coding mode,
but in most respects aside from a couple of fundamentals is actually
an entirely different codec altogether.
My current understanding is that the code given as celt 0.7.0 in the
current mumble source *is* in fact 0.7.1, though I need to diff that
against the upstream 0.7.1 to be absolutely certain still. So the
diff for 'adding' this to the mumble package itself should be very
minimal. Sorry if I wasn't totally clear about that bit here.
Ron
Reply to: