Closed
Bug 1051087
Opened 10 years ago
Closed 10 years ago
arm_version is not populated to gecko/media/webrtc/trunk/webrtc/common_audio properly
Categories
(Firefox Build System :: General, defect)
Tracking
(blocking-b2g:2.0+, firefox32 wontfix, firefox33 fixed, firefox34 fixed, b2g-v1.4 ?, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: jaywang, Assigned: jesup)
Details
(Whiteboard: [webrtc-uplift])
Attachments
(1 file, 1 obsolete file)
2.30 KB,
patch
|
ted
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
[Blocking Requested - why for this release]: gecko/media/webrtc/trunk/webrtc/common_audio/common_audio.gyp uses arm_version to decide when to include the neon optimized libraries. Here is the part of the common_audio.gyp: ['(target_arch=="arm" and arm_version==7) or target_arch=="armv7"', { 'targets': [ { 'target_name': 'common_audio_neon', 'type': 'static_library', 'includes': ['../build/arm_neon.gypi',], 'sources': [ 'resampler/sinc_resampler_neon.cc', 'signal_processing/cross_correlation_neon.S', 'signal_processing/downsample_fast_neon.S', 'signal_processing/min_max_operations_neon.S', 'signal_processing/vector_scaling_operations_neon.S', ], }, ], # targets However, arm_version variable was stored as string instead of integer in build/gyp.mozbuild. This causes common_audio.gyp incorrectly picks up non-neon libraries. Here is the proposed change: diff --git a/build/gyp.mozbuild b/build/gyp.mozbuild index fa52b2c..c9fd5d9 100644 --- a/build/gyp.mozbuild +++ b/build/gyp.mozbuild @@ -106,7 +106,7 @@ if CONFIG['ARM_ARCH']: # detection, so we have to set armv7=0 for non-Android target gyp_vars['armv7'] = 0 # For libyuv - gyp_vars['arm_version'] = CONFIG['ARM_ARCH'] + gyp_vars['arm_version'] = int(CONFIG['ARM_ARCH'])
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → rjesup
Whiteboard: [webrtc-uplift]
Assignee | ||
Updated•10 years ago
|
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Updated•10 years ago
|
Flags: in-moztrap?(bzumwalt)
Comment 1•10 years ago
|
||
No STR is present to create test case to address bug.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage?][2.0-signoff-need+]
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?][2.0-signoff-need+] → [QAnalyst-Triage+][2.0-signoff-need+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(bzumwalt)
Flags: in-moztrap-
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage+][2.0-signoff-need+] → [QAnalyst-Triage+][lead-review+][2.0-signoff-need+]
Comment 2•10 years ago
|
||
Ni, :jesup for next steps or help with patch here. Jesup we are a week away to get all 2.0+ blockers closed, can you please help here or give us an eta?
Flags: needinfo?(rjesup)
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8478223 -
Flags: review?(ted)
Flags: needinfo?(rjesup)
Updated•10 years ago
|
Attachment #8478223 -
Flags: review?(ted) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2ee81c83a01 Once on central will also nominate for 33
status-b2g-v2.0:
--- → affected
status-firefox32:
--- → wontfix
status-firefox33:
--- → affected
status-firefox34:
--- → affected
Target Milestone: --- → mozilla34
Comment 5•10 years ago
|
||
Backed out for Android bustage. https://hg.mozilla.org/integration/mozilla-inbound/rev/f1254b0f2fd8 https://tbpl.mozilla.org/php/getParsedLog.php?id=46690301&tree=Mozilla-Inbound
Assignee | ||
Comment 6•10 years ago
|
||
resolve android bustage; confirmed locally
Assignee | ||
Updated•10 years ago
|
Attachment #8478223 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8478372 -
Flags: review?(ted)
Updated•10 years ago
|
Attachment #8478372 -
Flags: review?(ted) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=9a7402d892f6 waiting for tree opening
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/32269088de49 Try is green; one known orange
https://hg.mozilla.org/mozilla-central/rev/32269088de49
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 10•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/8f631c20c821 This seems like something we might on v1.4 as well?
status-b2g-v1.4:
--- → ?
status-b2g-v2.1:
--- → fixed
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8478372 [details] [diff] [review] Ensure NEON optimizations are used for libyuv and webrtc audio code Approval Request Comment [Feature/regressing bug #]: gyp.mozbuild landing [User impact if declined]: much worse performance in webrtc code (esp audio) on ARM [Describe test coverage new/current, TBPL]: on b2g32 and nightly for a while. [Risks and why]: low risk given taking on b2g32 already, enabled codepaths used in upstream [String/UUID change made/needed]: none
Attachment #8478372 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8478372 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 13•10 years ago
|
||
Should have been: https://hg.mozilla.org/releases/mozilla-aurora/rev/84bdca67fffe
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•