Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2265473003:
Removed the global limitation of the native sample rates on ARM devices and replaced it with an APM… (Closed)
|
Created:
4 years, 4 months ago by peah-webrtc Modified:
4 years, 3 months ago 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, aleloi Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionRemoved the global limitation of the native sample rates on ARM devices and replaced it with an APM-internal limitation
when building the code for ARM.
The intention is to follow up this CL with other CLs that
further addresses the internal resampling in APM
BUG=webrtc:6181
Committed: https://crrev.com/cc34fafcf924df5c36edc278062ba8923ca92597
Cr-Commit-Position: refs/heads/master@{#13974}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Rebase and changes in response to reviewer comments #Messages
Total messages: 20 (10 generated)
Description was changed from ========== Removed the global limitation of the native sample rates on ARM devices and replaced it with an APM-internal limitation BUG= ========== to ========== Removed the global limitation of the native sample rates on ARM devices and replaced it with an APM-internal limitation when building the code for ARM. The intention is to follow up this CL with other CLs that further addresses the internal resampling in APM BUG=webrtc:6181 ==========
peah@webrtc.org changed reviewers: + henrik.lundin@webrtc.org, tina.legrand@webrtc.org
This LGTM, but I'd like you to wait with landing until after the next Chrome cut. Also not the changed behavior in the audio_conference_mixer (new and old). For the case of mixing more than one incoming stream at 48 kHz on ARM, the mixer will now in fact send the mixed audio to APM for limiting. This was not the case before. As a consequence, APM will downsample to 32 kHz before applying the limiter. (+aleloi FYI) https://codereview.webrtc.org/2265473003/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/2265473003/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:111: return max_native_rate; return kInternalNativeRates[arraysize(kInternalNativeRates) - 1]; And skip the local variable max_native_rate.
LGTM after removing the local variable Henrik mentions.
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from henrik.lundin@webrtc.org, tina.legrand@webrtc.org Link to the patchset: https://codereview.webrtc.org/2265473003/#ps20001 (title: "Rebase and changes in response to reviewer comments")
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_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/15981)
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/...
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)
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 ========== Removed the global limitation of the native sample rates on ARM devices and replaced it with an APM-internal limitation when building the code for ARM. The intention is to follow up this CL with other CLs that further addresses the internal resampling in APM BUG=webrtc:6181 ========== to ========== Removed the global limitation of the native sample rates on ARM devices and replaced it with an APM-internal limitation when building the code for ARM. The intention is to follow up this CL with other CLs that further addresses the internal resampling in APM BUG=webrtc:6181 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Removed the global limitation of the native sample rates on ARM devices and replaced it with an APM-internal limitation when building the code for ARM. The intention is to follow up this CL with other CLs that further addresses the internal resampling in APM BUG=webrtc:6181 ========== to ========== Removed the global limitation of the native sample rates on ARM devices and replaced it with an APM-internal limitation when building the code for ARM. The intention is to follow up this CL with other CLs that further addresses the internal resampling in APM BUG=webrtc:6181 Committed: https://crrev.com/cc34fafcf924df5c36edc278062ba8923ca92597 Cr-Commit-Position: refs/heads/master@{#13974} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/cc34fafcf924df5c36edc278062ba8923ca92597 Cr-Commit-Position: refs/heads/master@{#13974} |
