Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1211703002:
Created SphericalPoint in array_util.h (Closed)
|
Created:
5 years, 6 months ago by bloch Modified:
5 years, 6 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, hlundin-webrtc, kwiberg-webrtc, aluebs-webrtc, bjornv1 Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionCreated SphericalPoint in array_util.h
Committed: https://crrev.com/ebe7422372c561a017990ac59e15e58a2f02d8eb
Cr-Commit-Position: refs/heads/master@{#9507}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Changed distance to radius #Messages
Total messages: 18 (5 generated)
bloch@google.com changed reviewers: + aluebs@webrtc.org, andrew@webrtc.org
lgtm
https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/beamformer/array_util.h (right): https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/beamformer/array_util.h:26: T x() const { Put these methods all on one line to be consistent. https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/beamformer/array_util.h:49: SphericalPoint(T azimuth, T elevation, T distance) { Michael used radius rather than distance elsewhere. https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/beamformer/array_util.h:60: using SphericalPointf = SphericalPoint<float>; Just "using SphericalPoint" to be consistent with Point?
On 2015/06/25 01:32:31, andrew wrote: > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > File webrtc/modules/audio_processing/beamformer/array_util.h (right): > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > webrtc/modules/audio_processing/beamformer/array_util.h:26: T x() const { > Put these methods all on one line to be consistent. > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > webrtc/modules/audio_processing/beamformer/array_util.h:49: SphericalPoint(T > azimuth, T elevation, T distance) { > Michael used radius rather than distance elsewhere. > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > webrtc/modules/audio_processing/beamformer/array_util.h:60: using > SphericalPointf = SphericalPoint<float>; > Just "using SphericalPoint" to be consistent with Point? "using SphericalPoint" throws an error because the name changes.
On 2015/06/25 02:28:39, bloch wrote: > On 2015/06/25 01:32:31, andrew wrote: > > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > > File webrtc/modules/audio_processing/beamformer/array_util.h (right): > > > > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > > webrtc/modules/audio_processing/beamformer/array_util.h:26: T x() const { > > Put these methods all on one line to be consistent. > > > > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > > webrtc/modules/audio_processing/beamformer/array_util.h:49: SphericalPoint(T > > azimuth, T elevation, T distance) { > > Michael used radius rather than distance elsewhere. > > > > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > > webrtc/modules/audio_processing/beamformer/array_util.h:60: using > > SphericalPointf = SphericalPoint<float>; > > Just "using SphericalPoint" to be consistent with Point? > > "using SphericalPoint" throws an error because the name changes. *doesn't change
Changed distance to radius. Kept as "SphericalPointf" because without the "f" I get a redefinition error.
On 2015/06/25 02:29:18, bloch wrote: > On 2015/06/25 02:28:39, bloch wrote: > > "using SphericalPoint" throws an error because the name changes. > > *doesn't change Ah OK.
lgtm
On 2015/06/25 01:32:31, andrew wrote: > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > File webrtc/modules/audio_processing/beamformer/array_util.h (right): > > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > webrtc/modules/audio_processing/beamformer/array_util.h:26: T x() const { > Put these methods all on one line to be consistent. > Done > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > webrtc/modules/audio_processing/beamformer/array_util.h:49: SphericalPoint(T > azimuth, T elevation, T distance) { > Michael used radius rather than distance elsewhere. > Done > https://codereview.webrtc.org/1211703002/diff/1/webrtc/modules/audio_processi... > webrtc/modules/audio_processing/beamformer/array_util.h:60: using > SphericalPointf = SphericalPoint<float>; > Just "using SphericalPoint" to be consistent with Point? Acknowledged
The CQ bit was checked by bloch@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from aluebs@webrtc.org Link to the patchset: https://codereview.webrtc.org/1211703002/#ps20001 (title: "Changed distance to radius")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1211703002/20001
The CQ bit was unchecked by bloch@google.com
The CQ bit was checked by bloch@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1211703002/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ebe7422372c561a017990ac59e15e58a2f02d8eb Cr-Commit-Position: refs/heads/master@{#9507} |
