Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1515073004:
Correcting the check for the return code produced by AudioProcessing::ProcessReverseStream (Closed)
|
Created:
5 years ago by peah-webrtc Modified:
5 years ago CC:
webrtc-reviews_webrtc.org, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, minyue-webrtc, the sun Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionCorrecting the check for the return code produced by
AudioProcessing::ProcessReverseStream().
Before the change, only -1 was considered to be an error.
Allthough the error code scheme for AudioProcessing definitely
could be discussed, the current scheme have many error codes that
differ from -1 and thus were not caught by the old code.
BUG=webrtc:5237
Committed: https://crrev.com/e5ae6f82374cb064d39cffffdb650840f734880f
Cr-Commit-Position: refs/heads/master@{#11003}
Patch Set 1 #
Messages
Total messages: 28 (9 generated)
Description was changed from ========== Corrected the check for the return code produced by AudioProcessing::ProcessReverseStream(). Before the change, only -1 was considered to be an error. Allthough the error code scheme for AudioProcessing definitely could be discussed, the current scheme have many error codes that differ from -1 and thus were not caught by the old code. BUG=webrtc:5237 ========== to ========== Correcting the check for the return code produced by AudioProcessing::ProcessReverseStream(). Before the change, only -1 was considered to be an error. Allthough the error code scheme for AudioProcessing definitely could be discussed, the current scheme have many error codes that differ from -1 and thus were not caught by the old code. BUG=webrtc:5237 ==========
peah@webrtc.org changed reviewers: + henrika@webrtc.org, solenberg@webrtc.org
lgtm
LGTM
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/1515073004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1515073004/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 peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1515073004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1515073004/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_baremetal on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/7906)
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/1515073004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1515073004/1
Message was sent while issue was closed.
Description was changed from ========== Correcting the check for the return code produced by AudioProcessing::ProcessReverseStream(). Before the change, only -1 was considered to be an error. Allthough the error code scheme for AudioProcessing definitely could be discussed, the current scheme have many error codes that differ from -1 and thus were not caught by the old code. BUG=webrtc:5237 ========== to ========== Correcting the check for the return code produced by AudioProcessing::ProcessReverseStream(). Before the change, only -1 was considered to be an error. Allthough the error code scheme for AudioProcessing definitely could be discussed, the current scheme have many error codes that differ from -1 and thus were not caught by the old code. BUG=webrtc:5237 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Correcting the check for the return code produced by AudioProcessing::ProcessReverseStream(). Before the change, only -1 was considered to be an error. Allthough the error code scheme for AudioProcessing definitely could be discussed, the current scheme have many error codes that differ from -1 and thus were not caught by the old code. BUG=webrtc:5237 ========== to ========== Correcting the check for the return code produced by AudioProcessing::ProcessReverseStream(). Before the change, only -1 was considered to be an error. Allthough the error code scheme for AudioProcessing definitely could be discussed, the current scheme have many error codes that differ from -1 and thus were not caught by the old code. BUG=webrtc:5237 Committed: https://crrev.com/e5ae6f82374cb064d39cffffdb650840f734880f Cr-Commit-Position: refs/heads/master@{#11003} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/e5ae6f82374cb064d39cffffdb650840f734880f Cr-Commit-Position: refs/heads/master@{#11003}
Message was sent while issue was closed.
This CL spams log outputs using AppRTCDemo on iOS at a 16kHz capture rate. Can it be fixed, please? [007:655] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error [007:656] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error [007:656] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error [007:688] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error [007:689] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error [007:689] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error [007:690] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error [007:720] [43791] (webrtcvoiceengine.cc:966): webrtc: AudioProcessingModule::ProcessReverseStream() => error
Message was sent while issue was closed.
On 2015/12/15 15:36:55, henrika_webrtc wrote: > This CL spams log outputs using AppRTCDemo on iOS at a 16kHz capture rate. > Can it be fixed, please? > > [007:655] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error > > [007:656] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error > > [007:656] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error > > [007:688] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error > > [007:689] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error > > [007:689] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error > > [007:690] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error > > [007:720] [43791] (webrtcvoiceengine.cc:966): webrtc: > AudioProcessingModule::ProcessReverseStream() => error
Message was sent while issue was closed.
On 2015/12/15 21:32:13, peah-webrtc wrote: > On 2015/12/15 15:36:55, henrika_webrtc wrote: > > This CL spams log outputs using AppRTCDemo on iOS at a 16kHz capture rate. > > Can it be fixed, please? > > > > [007:655] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error > > > > [007:656] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error > > > > [007:656] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error > > > > [007:688] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error > > > > [007:689] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error > > > > [007:689] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error > > > > [007:690] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error > > > > [007:720] [43791] (webrtcvoiceengine.cc:966): webrtc: > > AudioProcessingModule::ProcessReverseStream() => error These error messages are the result of that the APM has been called using the wrong sample rate and the issue that this CL fixed caused that not to be reported. These error messages are correct and the errors should definitely be reported. The error is, however, fixed in the CL https://codereview.webrtc.org/1525173002/ so as soon as that has landed these errors will no longer occur.
Message was sent while issue was closed.
As Tina asked, hope this does not mean that AECM is enabled on iOS. Cause it should not be.
Message was sent while issue was closed.
On 2015/12/16 09:36:49, henrika_webrtc wrote: > As Tina asked, hope this does not mean that AECM is enabled on iOS. Cause it > should not be. This has nothing really to do with AECM apart from that fact that it hid a bug which caused AECM not to work. However, if AECM has all the time been active on IOS, the bug generating this message could have caused it to fail such that it was not possible to detect that it was on. In that case, fixing the bug that prevented AECM to at all work could potentially suddenly cause AECM to work on IOS if it has been all along activated on IOS. That we should check, but I don't think that is the case.
Message was sent while issue was closed.
Per, can you sort out any final details? Ping me if you need help.
Message was sent while issue was closed.
On 2015/12/16 10:02:48, henrika_webrtc wrote: > Per, can you sort out any final details? Ping me if you need help. I'd be happy to! The fix for the mismatched sample rates has now landed so I will start looking at it.
Message was sent while issue was closed.
On 2015/12/16 10:02:48, henrika_webrtc wrote: > Per, can you sort out any final details? Ping me if you need help. I'd be happy to! The fix for the mismatched sample rates has now landed so I will start looking at it.
Message was sent while issue was closed.
On 2015/12/16 10:11:57, peah-webrtc wrote: > On 2015/12/16 10:02:48, henrika_webrtc wrote: > > Per, can you sort out any final details? Ping me if you need help. > > I'd be happy to! The fix for the mismatched sample rates has now landed so I > will start looking at it. The AECM was verified by henrika_webrtc not to be active on iOS and the error messages are now gone. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
