Closed Bug 916807 Opened 7 years ago Closed 7 years ago
Update libopus from upstream (1
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.
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.
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.
Assignee: nobody → giles
(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 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.
Attachment #805600 - Flags: review?(tterribe)
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.
Attachment #805600 - Flags: review?(tterribe) → review+
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.
Thanks. Currently neither our webrtc code, nor OpusTrackEncoder set the complexity. We should double-check on the next update.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(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/
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.
Target Milestone: mozilla28 → mozilla27
5 years ago
Depends on: 790405
5 years ago
You need to log in before you can comment on or make changes to this bug.