Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(175)

Issue 150103006: Enabling ARM optimizations for Opus on Android and ChromeOS (Closed)

Created:
6 years, 10 months ago by tlegrand1
Modified:
6 years, 9 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Enabling ARM optimizations for Opus on Android and ChromeOS This CL turns on the ARM optimizations for the Opus codec on Android and ChromeOS. It gives roughly a 40% CPU reduction on Android, and probably the same on Chrome OS (don't have any numbers there to compare with). BUG=296844 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=258909

Patch Set 1 #

Total comments: 11

Patch Set 2 : Addressing comments #

Total comments: 3

Patch Set 3 : Enable for ChromeOS #

Patch Set 4 : Rename file #

Patch Set 5 : Changed conditions #

Total comments: 2

Patch Set 6 : Use float unless on ARM #

Total comments: 2

Patch Set 7 : Adding back two comments #

Patch Set 8 : Renaming #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -1 line) Patch
M third_party/opus/opus.gyp View 1 2 3 4 5 6 7 2 chunks +19 lines, -1 line 2 comments Download
A third_party/opus/opus_srcs_arm.gypi View 1 2 3 1 chunk +39 lines, -0 lines 0 comments Download

Messages

Total messages: 34 (0 generated)
tlegrand1
Hi Sergey, Thi CL enables the ARM optimizations in the new Opus 1.1. Please help ...
6 years, 10 months ago (2014-02-06 10:55:30 UTC) #1
Sergey Ulanov
There is nothing android-specific in this CL. So maybe remove 'Android' from the description and ...
6 years, 10 months ago (2014-02-06 18:23:13 UTC) #2
tlegrand1
Thanks for your great comments! I will upload a new patch set, but please wait ...
6 years, 10 months ago (2014-02-07 14:38:42 UTC) #3
tlegrand1
Hi Sergey, I realized I needed to land another CL before this one (in your ...
6 years, 10 months ago (2014-02-11 15:45:07 UTC) #4
Sergey Ulanov
On 2014/02/11 15:45:07, tlegrand1 wrote: > Hi Sergey, > > I realized I needed to ...
6 years, 10 months ago (2014-02-13 20:34:28 UTC) #5
tlegrand1
Thanks for you input Sergey! I realize my knowledge is too limited, and I need ...
6 years, 10 months ago (2014-02-14 13:40:16 UTC) #6
minyue
https://codereview.chromium.org/150103006/diff/80001/third_party/opus/opus.gyp File third_party/opus/opus.gyp (right): https://codereview.chromium.org/150103006/diff/80001/third_party/opus/opus.gyp#newcode87 third_party/opus/opus.gyp:87: ['(target_arch=="arm" and arm_version==7) or target_arch=="armv7"', { We may try ...
6 years, 10 months ago (2014-02-18 13:52:56 UTC) #7
tlegrand1
Hi Sergey, The CL is ready for a new round of review. The optimizations will ...
6 years, 9 months ago (2014-03-14 09:35:57 UTC) #8
tlegrand1
Hi Sergey, Do you have time to review this CL? I'd really like to get ...
6 years, 9 months ago (2014-03-18 12:39:02 UTC) #9
Sergey Ulanov
https://codereview.chromium.org/150103006/diff/190001/third_party/opus/opus.gyp File third_party/opus/opus.gyp (right): https://codereview.chromium.org/150103006/diff/190001/third_party/opus/opus.gyp#newcode9 third_party/opus/opus.gyp:9: 'use_opus_fixed_point%': 1, This will enable fixed-point math on Intel ...
6 years, 9 months ago (2014-03-19 19:21:51 UTC) #10
tlegrand1
https://codereview.chromium.org/150103006/diff/190001/third_party/opus/opus.gyp File third_party/opus/opus.gyp (right): https://codereview.chromium.org/150103006/diff/190001/third_party/opus/opus.gyp#newcode9 third_party/opus/opus.gyp:9: 'use_opus_fixed_point%': 1, I wanted to preserve the previous setting ...
6 years, 9 months ago (2014-03-20 10:10:56 UTC) #11
tlegrand1
New patch set is available, where I only enable fixed point if we are on ...
6 years, 9 months ago (2014-03-20 10:18:20 UTC) #12
Sergey Ulanov
lgtm https://codereview.chromium.org/150103006/diff/210001/third_party/opus/opus.gyp File third_party/opus/opus.gyp (left): https://codereview.chromium.org/150103006/diff/210001/third_party/opus/opus.gyp#oldcode70 third_party/opus/opus.gyp:70: }, # target opus Keep this comment? https://codereview.chromium.org/150103006/diff/210001/third_party/opus/opus.gyp#oldcode98 ...
6 years, 9 months ago (2014-03-20 18:30:15 UTC) #13
tlegrand1
The CQ bit was checked by tlegrand@chromium.org
6 years, 9 months ago (2014-03-21 08:26:20 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tlegrand@chromium.org/150103006/230001
6 years, 9 months ago (2014-03-21 08:26:32 UTC) #15
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 08:28:04 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on linux_chromium_clang_dbg
6 years, 9 months ago (2014-03-21 08:28:05 UTC) #17
tlegrand1
Sergey, I added back the comments you asked about, and fixed the name of opus_srcs_arm.gypi ...
6 years, 9 months ago (2014-03-21 08:46:32 UTC) #18
tlegrand1
The CQ bit was checked by tlegrand@chromium.org
6 years, 9 months ago (2014-03-21 08:53:54 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tlegrand@chromium.org/150103006/250001
6 years, 9 months ago (2014-03-21 08:54:20 UTC) #20
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-21 09:49:09 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on mac_chromium_rel
6 years, 9 months ago (2014-03-21 09:49:09 UTC) #22
tlegrand1
The CQ bit was checked by tlegrand@chromium.org
6 years, 9 months ago (2014-03-24 08:40:14 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tlegrand@chromium.org/150103006/250001
6 years, 9 months ago (2014-03-24 08:40:16 UTC) #24
commit-bot: I haz the power
Change committed as 258909
6 years, 9 months ago (2014-03-24 11:47:11 UTC) #25
Ami GONE FROM CHROMIUM
Post-commit drive-by https://codereview.chromium.org/150103006/diff/250001/third_party/opus/opus.gyp File third_party/opus/opus.gyp (right): https://codereview.chromium.org/150103006/diff/250001/third_party/opus/opus.gyp#newcode8 third_party/opus/opus.gyp:8: ['(OS=="android" or chromeos==1) and target_arch=="arm"', { Why ...
6 years, 9 months ago (2014-03-24 16:16:33 UTC) #26
minyue
Hi Ami, Thank you for the comments! Currently, the optimization on iOS has some issue ...
6 years, 9 months ago (2014-03-24 16:30:16 UTC) #27
Ami GONE FROM CHROMIUM
> > Currently, the optimization on iOS has some issue with Clang. This will be ...
6 years, 9 months ago (2014-03-24 16:53:54 UTC) #28
minyue
Hi, 1. I was suggested to report the issue with Clang upstream, it is now ...
6 years, 9 months ago (2014-03-25 08:41:03 UTC) #29
Ami GONE FROM CHROMIUM
On Tue, Mar 25, 2014 at 1:41 AM, <minyue@chromium.org> wrote: > 2. We compared the ...
6 years, 9 months ago (2014-03-25 08:55:10 UTC) #30
minyue1
Yes, It was measured outside chrome. Thank you for bringing up this. We will look ...
6 years, 9 months ago (2014-03-25 09:05:41 UTC) #31
Ami GONE FROM CHROMIUM
Thanks for doing this! On Tue, Mar 25, 2014 at 2:05 AM, Minyue Li <minyue@google.com> ...
6 years, 9 months ago (2014-03-25 09:06:19 UTC) #32
henrika (OOO until Aug 14)
As Ami states. The code seems to rely on cpuinfo = fopen("/proc/cpuinfo", "r") and I ...
6 years, 9 months ago (2014-03-25 10:54:44 UTC) #33
minyue1
6 years, 9 months ago (2014-03-25 11:10:09 UTC) #34
good news is that Opus's optimization does not only rely on real time cpu
detection (RTCD)

1. RTCD brings a relatively small contribution compared to inline assemblers

2. Opus provides micros to hard code Neon.


On Tue, Mar 25, 2014 at 11:54 AM, <henrika@chromium.org> wrote:

> As Ami states. The code seems to rely on cpuinfo = fopen("/proc/cpuinfo",
> "r")
> and I don't think that will work inside the sandbox. I think that some
> more work
> is needed here to verify that the performance gains really exists in
> Chrome.
>
> https://codereview.chromium.org/150103006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698