Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1496243002:
Remove scaling for CNG 48 kHz (Closed)
|
Created:
5 years ago by tlegrand-webrtc Modified:
5 years ago Reviewers:
hlundin-webrtc CC:
webrtc-reviews_webrtc.org, kwiberg-webrtc, tterriberry_mozilla.com, audio-team_agora.io, peah-webrtc, minyue-webrtc Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionThere was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling.
The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz.
BUG=webrtc:5303
R=henrik.lundin@webrtc.org
Committed: https://crrev.com/325b34542d9bdc5a38d01d1c69e61a63eaa36ab1
Cr-Commit-Position: refs/heads/master@{#10929}
Patch Set 1 #
Messages
Total messages: 19 (10 generated)
Description was changed from ========== Fixing the scaling BUG= ========== to ========== Fixing the scaling BUG= ==========
Description was changed from ========== Fixing the scaling BUG= ========== to ========== There was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling. The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz. BUG=webrtc:5303 ==========
Description was changed from ========== There was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling. The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz. BUG=webrtc:5303 ========== to ========== There was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling. The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz. BUG=webrtc:5303 ==========
tina.legrand@webrtc.org changed reviewers: + henrik.lundin@webrtc.org
Hi Henrik, Please review this small fix. There was no unit-test for this case, and I don't think it makes sense to add one. Also realized that we don't support PCM16b @ 48 kHz, so this can't really be tested, right?
On 2015/12/04 08:12:13, tlegrand-webrtc wrote: > Hi Henrik, > > Please review this small fix. > > There was no unit-test for this case, and I don't think it makes sense to add > one. Also realized that we don't support PCM16b @ 48 kHz, so this can't really > be tested, right? There is one test for this functionality, but it tests G.722 in combination with CNG-wb. I don't think we have to add another test for this. My plan is to refactor this code, so that we can get rid of the NetEqDecoder enumerator eventually. We do support PCM16b, but it is currently not listed in ACM's codec database, so there is no way to select it from libjingle. That may change.
lgtm Thanks!
The CQ bit was checked by tina.legrand@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1496243002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1496243002/1
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) linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by tina.legrand@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1496243002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1496243002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_msan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/6472)
Description was changed from ========== There was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling. The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz. BUG=webrtc:5303 ========== to ========== There was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling. The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz. BUG=webrtc:5303 R=henrik.lundin@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/325b34542d9bdc5a38d01d1c6... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as 325b34542d9bdc5a38d01d1c69e61a63eaa36ab1 (presubmit successful).
Message was sent while issue was closed.
Description was changed from ========== There was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling. The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz. BUG=webrtc:5303 R=henrik.lundin@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/325b34542d9bdc5a38d01d1c6... ========== to ========== There was an old scaling for CNG 48 kHz in the code, from the time where Audio Coding Module didn't have full 48 kHz support. This CL removes the scaling. The bug hasn't caused us any problems, since we don't run CNG together with Opus (our only real 48 kHz codec), but would cause problems if used with PCB16b @ 48 kHz. BUG=webrtc:5303 R=henrik.lundin@webrtc.org Committed: https://crrev.com/325b34542d9bdc5a38d01d1c69e61a63eaa36ab1 Cr-Commit-Position: refs/heads/master@{#10929} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/325b34542d9bdc5a38d01d1c69e61a63eaa36ab1 Cr-Commit-Position: refs/heads/master@{#10929} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
