Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1835073004:
Reland of Added a bitexactness test for the gain controller in the audio processing module. (Closed)
|
Created:
4 years, 9 months ago by peah-webrtc Modified:
4 years, 8 months ago Reviewers:
hlundin-webrtc CC:
webrtc-reviews_webrtc.org, peah-webrtc, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, minyue-webrtc, the sun, aluebs-webrtc, bjornv1 Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionReland of Added a bitexactness test for the gain controller in the audio processing module.
This is a reland of the CL https://codereview.webrtc.org/1812433002/ which
was reverted due to incorrect bitexactness on Android bots.
The changes done in the relanding CL is to Deactivate the test for Android and reduce the number of interations.
TBR=henrik.lundin@webrtc.org
BUG=webrtc:5339
Committed: https://crrev.com/57d5a2e4dff1069f36eca986049be99b758b4979
Cr-Commit-Position: refs/heads/master@{#12143}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase #
Messages
Total messages: 23 (11 generated)
The CQ bit was checked by peah@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1835073004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1835073004/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
peah@webrtc.org changed reviewers: + henrik.lundin@webrtc.org
Hi, Since I've needed to do some reverts of the bitexactness code I've deactivated the gain controller and aec bitexactness tests for the Android and ARM platforms. I've also reduced the number of iterations to reduce the impact of of numerical floating point issues. The problem is that the test fails on the Chromium bots and not on the WebRTC bots, so the CL needs to be reverted if that happens.
Description was changed from ========== Reland of Added a bitexactness test for the gain controller in the audio processing module. This is a reland of the CL https://codereview.webrtc.org/1812433002/ which was reverted due to incorrect bitexactness on Android bots. The changes done in the relanding CL is to Deactivate the test for Android and reduce the number of interations. BUG=webrtc:5339 ========== to ========== Reland of Added a bitexactness test for the gain controller in the audio processing module. This is a reland of the CL https://codereview.webrtc.org/1812433002/ which was reverted due to incorrect bitexactness on Android bots. The changes done in the relanding CL is to Deactivate the test for Android and reduce the number of interations. TBR=henrik.lundin@webrtc.org BUG=webrtc:5339 ==========
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1835073004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1835073004/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_clang_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...) android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...) ios32_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/6271) ios64_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/6259) ios_arm64_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/8597) ios_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/8517) ios_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/13800) ios_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/12430) linux_compile_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_compile_dbg/build...) linux_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/13829) mac_asan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/13451) mac_compile_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...) mac_gn_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/1912) mac_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/8140) mac_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/13845)
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1835073004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1835073004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on 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/patch-status/1835073004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1835073004/20001
Message was sent while issue was closed.
Description was changed from ========== Reland of Added a bitexactness test for the gain controller in the audio processing module. This is a reland of the CL https://codereview.webrtc.org/1812433002/ which was reverted due to incorrect bitexactness on Android bots. The changes done in the relanding CL is to Deactivate the test for Android and reduce the number of interations. TBR=henrik.lundin@webrtc.org BUG=webrtc:5339 ========== to ========== Reland of Added a bitexactness test for the gain controller in the audio processing module. This is a reland of the CL https://codereview.webrtc.org/1812433002/ which was reverted due to incorrect bitexactness on Android bots. The changes done in the relanding CL is to Deactivate the test for Android and reduce the number of interations. TBR=henrik.lundin@webrtc.org BUG=webrtc:5339 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Reland of Added a bitexactness test for the gain controller in the audio processing module. This is a reland of the CL https://codereview.webrtc.org/1812433002/ which was reverted due to incorrect bitexactness on Android bots. The changes done in the relanding CL is to Deactivate the test for Android and reduce the number of interations. TBR=henrik.lundin@webrtc.org BUG=webrtc:5339 ========== to ========== Reland of Added a bitexactness test for the gain controller in the audio processing module. This is a reland of the CL https://codereview.webrtc.org/1812433002/ which was reverted due to incorrect bitexactness on Android bots. The changes done in the relanding CL is to Deactivate the test for Android and reduce the number of interations. TBR=henrik.lundin@webrtc.org BUG=webrtc:5339 Committed: https://crrev.com/57d5a2e4dff1069f36eca986049be99b758b4979 Cr-Commit-Position: refs/heads/master@{#12143} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/57d5a2e4dff1069f36eca986049be99b758b4979 Cr-Commit-Position: refs/heads/master@{#12143}
Message was sent while issue was closed.
One comment after the fact. LGTM. https://codereview.webrtc.org/1835073004/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/gain_control_unittest.cc (right): https://codereview.webrtc.org/1835073004/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/gain_control_unittest.cc:19: #if !(defined(WEBRTC_ARCH_ARM64) || defined(WEBRTC_ARCH_ARM) || \ I think it is better to disable the test itself, rather than excluding all of the code. This will prevent the code from rotting while being disabled.
Message was sent while issue was closed.
https://codereview.webrtc.org/1835073004/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/gain_control_unittest.cc (right): https://codereview.webrtc.org/1835073004/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/gain_control_unittest.cc:19: #if !(defined(WEBRTC_ARCH_ARM64) || defined(WEBRTC_ARCH_ARM) || \ On 2016/03/29 21:21:42, hlundin-webrtc wrote: > I think it is better to disable the test itself, rather than excluding all of > the code. This will prevent the code from rotting while being disabled. Good point! I will create a CL to change that. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
