On Tue, Jul 24, 2012 at 01:17:10PM +0100, Ian Jackson wrote: > Ron writes ("Re: Bug#682010: [mumble] Communication failures due to CELT > codec library removal"): > > My primary concern is with the fact we would be shipping very complicated > > code, that only about 3 people in the world really understand, and which > > has no committed ongoing maintainer from among them or anyone else. > > I don't think this is really a huge issue. As I understand it the > code in the celt codec has been reused in the implementation of opus - > obviously not quite identically, but that means that it's not really > right to say that no-one understands this code and that it's dead > upstream.
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. There's a simplified description of some of that here: http://people.xiph.org/~xiphmont/demo/celt/demo.html (be sure to mouse-over the block diagram too) If there is anybody reading this who thinks they understand all the concepts there enough to analyse code implementing them, then please do put your hand up, we may need your expert attention at some point in the future. (and we have other codecs we'd love you to help out with too :) For those who don't, I probably should note that was described as the "lies for children" simplification by the people who wrote it. Which wasn't meant to be rude to anyone else, it just really does skip over an enormous amount of the actual complexity that is really there, to try and give ordinary people some broad idea of how it works, and things to go research if they want to learn more. I agree that saying "nobody" understands it is also an oversimplification on my part, but I'm not aware of there being anybody at present in the intersection set of "people who care about mumble using old celt" and "people who do understand this". If that wasn't an empty set, I'd also be less concerned. (and I did spend quite some time asking around to try to find someone who might fill that void) I have no reason to doubt that the upstream maintainers who said they have no further interest in this codebase really do mean that. I've been involved with them long enough to see them move from one project to the next before and it really will be "our problem, and our problem alone" if we continue using this. > 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) There's a 'spiritual' connection between these codebases, but so much has been rewritten and reinvented so many times now, that backporting anything from the new code to the old won't be a case of understanding the code. It will likely require reverse engineering the math and then completely re-analysing the problem in a very different domain, just to see if it still applies, let alone to figure out a fix. If that wasn't the case, the problems identified in later code would have already had fixes backported to the code we have. As it is, nobody has yet figured out how those things really map together, to confirm or deny the old code is affected -- all the people who cared enough to try got lost at the very first step. For a more empirical clarification of how much has changed, see the diffstat below. > > If there is a consensus among the members of the TC and the security > > team that the risk of doing that is justified by other factors, then > > I'll consider the peer decision making process to have worked as it > > should, and quite the opposite of being 'irritated', I'll be quite > > relieved that this decision and its possible consequences do not fall > > on my head alone. > > Well I asked this question of the security team, and while they > weren't particularly positive about it they did not object to the > inclusion in wheezy. Yes, I did see nion's earlier response to that: http://lists.debian.org/debian-ctte/2012/07/msg00192.html > > So ... really the only decision I see to be made here, is will we > > ship with celt 0.7.1 enabled or not. If -ctte and -security weighs > > up the risks and tells me they are happy doing that, then I'm happy > > to make that happen with no further delay. > > I don't speak for the whole TC, but my personal view at the moment is > that this tradeoff is worthwhile. I think I've made my concerns are clear as I can, so in my mind then, if Don and Steve agree with your assessment, then I'm happy to consider that a sufficient 'consensus' of the TC (since they have already been following this, and it seems cruel and unnecessary to inflict reading all of it on the rest of the -ctte members if they don't of their own free will wish to do so and chime in with an opinion). If Phil is ok with this from his perspective as SRM, then let's all get beer and move on to the wrap party :) Best, Ron Diffstat of celt 0.7.1 to opus 0.9.14 currently in Wheezy: _kiss_fft_guts.h | 153 -- arch.h | 138 -- bands.c | 1790 +++++++++++++++---------- bands.h | 85 - c64_fft.c | 344 ---- c64_fft.h | 58 celt.c | 3513 ++++++++++++++++++++++++++++++++++----------------- celt.h | 265 --- celt_header.h | 70 - celt_lpc.c | 188 ++ celt_lpc.h | 53 celt_types.h | 140 -- cwrs.c | 515 +------ cwrs.h | 25 dump_modes.c | 223 --- ecintrin.h | 99 - entcode.c | 54 entcode.h | 132 + entdec.c | 258 ++- entdec.h | 87 - entenc.c | 283 +++- entenc.h | 108 - fixed_c5x.h | 26 fixed_c6x.h | 26 fixed_debug.h | 383 ++++- fixed_generic.h | 85 - float_cast.h | 169 +- header.c | 129 - kfft_double.h | 79 - kfft_single.c | 44 kfft_single.h | 84 - kiss_fft.c | 777 +++++------ kiss_fft.h | 158 +- kiss_fftr.c | 165 -- kiss_fftr.h | 82 - laplace.c | 168 +- laplace.h | 26 match-test.sh | 30 mathops.c | 206 ++ mathops.h | 285 ---- mdct.c | 200 +- mdct.h | 41 mfrngcod.h | 18 mfrngdec.c | 248 --- mfrngenc.c | 236 --- modes.c | 551 +++---- modes.h | 91 - opus_custom_demo.c | 210 +++ os_support.h | 99 - pitch.c | 326 +++- pitch.h | 28 plc.c | 136 - psy.c | 211 --- psy.h | 57 quant_bands.c | 523 +++++-- quant_bands.h | 49 rangedec.c | 210 --- rangeenc.c | 209 --- rate.c | 654 +++++++-- rate.h | 135 - stack_alloc.h | 44 static_modes_fixed.h | 595 ++++++++ static_modes_float.h | 599 ++++++++ testcelt.c | 209 --- vq.c | 396 +++-- vq.h | 47 70 files changed, 9563 insertions(+), 9178 deletions(-) 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: $ cat libcelt/*.[ch] | wc -l 13441 I'll leave trying to understand and review the diff itself as an exercise for the truly dedicated reader :) -- To UNSUBSCRIBE, email to debian-ctte-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20120724165132.gd18...@audi.shelbyville.oz