Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2294093004:
Added build flag around the Intelligibility enhancer performance test code (Closed)
|
Created:
4 years, 3 months ago by peah-webrtc Modified:
4 years, 3 months ago CC:
webrtc-reviews_webrtc.org, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, minyue-webrtc, the sun, aluebs-webrtc, bjornv1 Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdded build flag around the Intelligibility enhancer performance test code
BUG=chromium:641931
Committed: https://crrev.com/d29e3ea4b2f4d7901ddcda5f229c30f4daf731f6
Cr-Commit-Position: refs/heads/master@{#14202}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Added check for that the INTELLIGIBILITY_ENHANCER build flag is explicitly set #Patch Set 3 : Added build flag #Patch Set 4 : Corrected build file #
Total comments: 2
Patch Set 5 : Removed unnecessary parenthesis #
Messages
Total messages: 27 (12 generated)
peah@webrtc.org changed reviewers: + ivoc@webrtc.org
LGTM, but see my question below. https://codereview.webrtc.org/2294093004/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc (right): https://codereview.webrtc.org/2294093004/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:99: #if (WEBRTC_INTELLIGIBILITY_ENHANCER == 1) Does this work if the flag is not set at all? Or is that not supposed to happen?
https://codereview.webrtc.org/2294093004/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc (right): https://codereview.webrtc.org/2294093004/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:99: #if (WEBRTC_INTELLIGIBILITY_ENHANCER == 1) On 2016/08/31 08:45:45, ivoc wrote: > Does this work if the flag is not set at all? Or is that not supposed to happen? That is a very good point! I put it like this in order not to clutter the test, but I'll add a proper check as well.
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org Link to the patchset: https://codereview.webrtc.org/2294093004/#ps20001 (title: "Added check for that the INTELLIGIBILITY_ENHANCER build flag is explicitly set")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios64_gn_dbg on master.tryserver.webrtc (JOB_FAILED, no build URL) ios64_gn_rel on master.tryserver.webrtc (JOB_FAILED, no build URL)
On 2016/08/31 08:45:46, ivoc wrote: > LGTM, but see my question below. > > https://codereview.webrtc.org/2294093004/diff/1/webrtc/modules/audio_processi... > File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc > (right): > > https://codereview.webrtc.org/2294093004/diff/1/webrtc/modules/audio_processi... > webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:99: #if > (WEBRTC_INTELLIGIBILITY_ENHANCER == 1) > Does this work if the flag is not set at all? Or is that not supposed to happen? Your question was definitely motivated! It failed as the flag is not set for this file. I will need to change the CL to add a build flag for this file.
Description was changed from ========== Added build flag around the Intelligibility enhancer performance test code BUG=chromium:641931 ========== to ========== Added build flag around the Intelligibility enhancer performance test code BUG=chromium:641931 ==========
peah@webrtc.org changed reviewers: + kwiberg@webrtc.org
On 2016/09/12 14:39:22, peah-webrtc wrote: > mailto:peah@webrtc.org changed reviewers: > + mailto:kwiberg@webrtc.org +kwiberg@ as an OWNER of the build files.
On 2016/09/12 14:39:22, peah-webrtc wrote: > mailto:peah@webrtc.org changed reviewers: > + mailto:kwiberg@webrtc.org +kwiberg@ as an OWNER of the build files.
lgtm https://codereview.webrtc.org/2294093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc (right): https://codereview.webrtc.org/2294093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:107: #if (WEBRTC_INTELLIGIBILITY_ENHANCER == 1) No parentheses needed.
https://codereview.webrtc.org/2294093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/audio_processing_performance_unittest.cc (right): https://codereview.webrtc.org/2294093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/audio_processing_performance_unittest.cc:107: #if (WEBRTC_INTELLIGIBILITY_ENHANCER == 1) On 2016/09/13 10:09:43, kwiberg-webrtc wrote: > No parentheses needed. Done.
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org, kwiberg@webrtc.org Link to the patchset: https://codereview.webrtc.org/2294093004/#ps80001 (title: "Removed unnecessary parenthesis")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Added build flag around the Intelligibility enhancer performance test code BUG=chromium:641931 ========== to ========== Added build flag around the Intelligibility enhancer performance test code BUG=chromium:641931 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Added build flag around the Intelligibility enhancer performance test code BUG=chromium:641931 ========== to ========== Added build flag around the Intelligibility enhancer performance test code BUG=chromium:641931 Committed: https://crrev.com/d29e3ea4b2f4d7901ddcda5f229c30f4daf731f6 Cr-Commit-Position: refs/heads/master@{#14202} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/d29e3ea4b2f4d7901ddcda5f229c30f4daf731f6 Cr-Commit-Position: refs/heads/master@{#14202} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
