1042504 - update speex resampler to speexdsp 305e54ea

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Karl Tomlinson (:karlt)]

Development of the speex resampler is happening more in speexdsp than opus-tools now.

Some upstream work in speexdsp includes neon support for the direct resampler, and I'm expecting some other changes soon that we want.  I'll file another bug about enabling the neon support.  This one is just about switching to speexdsp and updating existing files.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: correct strip prefix for applying outside-speex patch in update.sh

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: part 2: switch speex resampler to speexdsp bbe7e099

This is the speexdsp revision most similar to what is currently in Gecko.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: part 3: update speex resampler to speexdsp 305e54ea

Upstream SATURATE32PSHR changes conflict with Gecko's SSE runtime
detection patch.  That patch is updated to reduce deviation from
upstream.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: part 4: use direct resampler for 48->44.1k

Transplanted from patch by Gregory Maxwell <[e-mail]>
http://git.xiph.org/?p=opus-tools.git;a=commitdiff;h=f426ab52

Comment 5

[Paul Adenot (:padenot)]

Comment on attachment 8460745 [details] [diff] [review]
part 2: switch speex resampler to speexdsp bbe7e099

Review of attachment 8460745 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/libspeex_resampler/README_MOZILLA
@@ +1,1 @@
>  This source is from the Speex library (http://git.xiph.org/speex.git/), from

Maybe we want to update this URL to point to http://git.xiph.org/?p=speexdsp.git ?

Comment 6

[Karl Tomlinson (:karlt)]

(In reply to Paul Adenot (:padenot) from comment #5)
> Maybe we want to update this URL to point to
> http://git.xiph.org/?p=speexdsp.git ?

Yes, I've made that change now, thanks.

Comment 7

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8beaf971f950
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8cd15fa9824
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ca7ce33b2b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebb543336e85

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8beaf971f950
https://hg.mozilla.org/mozilla-central/rev/f8cd15fa9824
https://hg.mozilla.org/mozilla-central/rev/0ca7ce33b2b2
https://hg.mozilla.org/mozilla-central/rev/ebb543336e85

Comment 9

[Karl Tomlinson (:karlt)]

(In reply to bhavana bajaj [:bajaj] from comment #12)
> Karl to give his input on risk profile here.

I don't know neon assembler myself, so I don't know the potential for edge
case issues such as incorrect saturation.  Things will sound bad anyway if
content causes the corner cases are being reached.  The common case has
automated tests, which would pick up major bugs but maybe not smaller changes
in quality.  IIRC VLC has been using similar code for a long time.

Porting this requires the SATURATE32PSHR changes from bug 1042504 and some of
the refactoring of conditionals there.  That's probably more costly in terms
of developer work than risk.

The greatest risk is probably generating and running the wrong code on
platforms that won't run it.  Such effects would be fatal, but therefore
easier to detect.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

[Blocking Requested - why for this release]: Blocks bug 1042508. Per bug 1042508 comment 19, uplifting this is the least-risky path to getting it uplifted. Bug 1042508 commment 29 contains a Try push of the entire patch stack on top of b2g32.

Comment 11

[bhavana bajaj [:bajaj]]

blocking for a clean/less risky uplift of 1042508

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2fd6b0a10de2
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/a502857de24a
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f8f522f8c7e3
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/96f5fed12856