Bug#799176: Bug #799176 - Overflow in libspeexdsp resampler

[Ron]

On Wed, Sep 16, 2015 at 07:51:25PM +0200, Erik Montnemery wrote:
> Hi Ron!
> 
> The subject of my bug report was not so good, I should have included that
> the problem affects the fixed point implementation. Can I change the
> subject of the bug report?

You can, but it's fine.  That it's a fixed point overflow when the input
clips was already obvious.

> I replied with a patch which is a minimal merge from upstream (SpeexDSP
> 1.2rc3) which completely solves the problem for me.
> Please let me know if this is not the correct way to suggest a patch!

A better report would have just pointed directly to the commit(s) in the
upstream repo that you thought were relevant to this.

Most of the changes there which are directly relevant to us we've already
been carrying (I pushed a bunch of those into the upstream repo myself),
but I seem to have missed this one happening between the refactoring and
optimisation work.

I'm not going to apply a backported frankenpatch for this though, we
should just fast forward to the current head if there's changes there
now which are actually useful here.

> Regarding severity, after reading the guidelines I guess my suggestion for
> 'grave' was a bit much.
> At the same time, 'normal' is maybe a bit conservative since the fixed
> point libspeex resampler seems to be completely broken, and the libspeex
> resampler is the default resampler for both ALSA and PulseAudio. Maybe
> 'important' is more suitable?

As the only actual bug currently open on this package, any distinction
between severity wishlist and important is irrelevant.  It's a real bug
with a real upstreamed patch and it ought to be fixed.

It's a bit of a stretch to call something "completely broken" when there's
a huge number of existing users who haven't hit this problem over the last
like, 8 years or so (and possibly even longer since the resampler code had
changed in any way relevant to this).  That kind of naturally precludes an
"everybody panic and kick it out of testing" severity ...


Updating this has become slightly more complicated since the speexdsp code
has now been split out of speex as a separate source, and I haven't been
in a hurry to introduce a potential disruption like that until the timing
for that was good and the need for it compelling - but this bug at least
qualifies the latter requirement, so I'll bump making that happen up the
list to suit.

  Cheers,
  Ron