916807 - Update libopus from upstream (1.1?)

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

A primary reason is to reduce CPU use, especially on mobile, doubly so on B2G.  Feel free to link to any other bugs, or See Also to an external tracker for this issue.

Targeting 28 due to B2G linkage.

Comment 1

[Ralph Giles (:rillian)]

I doubt 1.1 will be in stable release during 27 development, but aside from some surround encoder work, which doesn't affect WebRTC, it should be fine to update our in-tree copy to current git master and then again as we get closer to opus 1.1 and ff 28.

If there are no objections, I'll go ahead and do that.

Comment 2

[Ralph Giles (:rillian)]

Attachment: Update to opus git master

Test patch.

- Update to today's git master.
- Remove upstream patches we were carrying
- Update update.sh for source list changes
- Make update.sh update the compiled-in OPUS_VERSION define

I didn't check to see if the new asm worked.

Comment 3

[Ralph Giles (:rillian)]

https://tbpl.mozilla.org/?tree=Try&rev=086e66637c91

Comment 4

[Timothy B. Terriberry (:derf)]

(In reply to Ralph Giles (:rillian) from comment #2)
> I didn't check to see if the new asm worked.

Unless you include #defines to enable it, you aren't using it. There will be more build-system work to do on that front once the NEON stuff has landed upstream (that can obviously be a separate bug, but I think that's what we'd like to get in before 28).

Comment 5

[Ralph Giles (:rillian)]

Comment on attachment 805600 [details] [diff] [review]
Update to opus git master

As you say. Sounds like this is a useful start for now, and no slower than what we had before. Patch is green on try and worked in local testing.

Comment 6

[Timothy B. Terriberry (:derf)]

Comment on attachment 805600 [details] [diff] [review]
Update to opus git master

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

This patch LGTM.

Two caveats for the future:
1) As mentioned in comment 4, this doesn't enable any of the inline ARM asm.
2) Once the exp_surround1 branch is merged upstream (which should happen before the final 1.1 release), there will be floating point tonality analysis, etc., enabled even on fixed-point ARM builds at encoder complexity 10. We'll need land something to drop the complexity we use on ARM to avoid a performance regression on devices with a slow/no FPU.

Comment 7

[Jean-Marc Valin]

Re 2), the Opus code was changed to default to complexity 9, so unless we explicitly set it to 10 in the WebRTC code, things should work fine in fixed-point.

Comment 8

[Ralph Giles (:rillian)]

Thanks. Currently neither our webrtc code, nor OpusTrackEncoder set the complexity. We should double-check on the next update.

Comment 9

[Ralph Giles (:rillian)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f74b1fe6bcc8

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f74b1fe6bcc8

Comment 11

[u279076]

(In reply to Wes Kocher (:KWierso) from comment #10)
> https://hg.mozilla.org/mozilla-central/rev/f74b1fe6bcc8

Can you please confirm the target milestone is correct? This would be Firefox 27 according to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013/09/2013-09-24-03-02-02-mozilla-central/

Comment 12

[Ralph Giles (:rillian)]

Git shows this is in 27 beta as well. I think the target was set to 28 as a goal when the bug was opened. Thanks for catching.