Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1773173002:
Dont always downsample to 16kHz in the reverse stream in APM (Closed)
|
Created:
4 years, 9 months ago by aluebs-webrtc Modified:
4 years, 9 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 Base URL:
https://chromium.googlesource.com/external/webrtc.git@aecm Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionDont always downsample to 16kHz in the reverse stream in APM
TBR=tina.legrand@webrtc.org
Committed: https://crrev.com/df6416aa502d5a9694875a22fcdf286c10f836ea
Cr-Commit-Position: refs/heads/master@{#12024}
Patch Set 1 #
Total comments: 30
Patch Set 2 : Simplify get_proc_rate #Patch Set 3 : Naming #
Total comments: 4
Patch Set 4 : Rename back #Patch Set 5 : Rebase #Patch Set 6 : update output_data_mac.pb #Patch Set 7 : Fix android #
Messages
Total messages: 35 (16 generated)
aluebs@webrtc.org changed reviewers: + henrik.lundin@webrtc.org, peah@webrtc.org, turaj@webrtc.org
https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:87: for (size_t i = 0; i < AudioProcessing::kNumNativeSampleRates; ++i) { I suggest a re-write of the for loop: for (int rate : AudioProcessing::kNativeSampleRatesHz) { if (rate >= min_proc_rate) { return rate; } } // Handle the case of ending the loop without finding a candidate // either as an error or by returning // AudioProcessing::kMaxNativeSampleRateHz
https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:87: for (size_t i = 0; i < AudioProcessing::kNumNativeSampleRates; ++i) { On 2016/03/09 12:10:57, hlundin-webrtc wrote: > I suggest a re-write of the for loop: > for (int rate : AudioProcessing::kNativeSampleRatesHz) { > if (rate >= min_proc_rate) { > return rate; > } > } > // Handle the case of ending the loop without finding a candidate > // either as an error or by returning > // AudioProcessing::kMaxNativeSampleRateHz I see that I am more sensitive than most when writing methods with more than one return statement. But there is no excuse for not using the range for-loop (except maybe that the definition of kNativeSampleRatesHz needed to be moved above this). Anyway, I accepted your suggestion. And the correct thing to do on end of loop is returning kMaxNativeSampleRateHz, which also is the same behavior as originally, since it will downsample any higher sampling rates to the highest supported one and still do the right thing.
https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:78: bool is_multi_band(int sample_rate_hz) { I think the style guide warrants a different name https://google.github.io/styleguide/cppguide.html#Function_Names. What about MultiBandSignal()? https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:83: int get_proc_rate(int input_rate, int output_rate) { I think the style guide warrants a different name (right?) https://google.github.io/styleguide/cppguide.html#Function_Names. Furthermore, I think the purpose of the method would be clearer if the min operation was moved to the actual function call. That would also simplify the naming. E.g. (with a method name change to ClosestNativeRate()) capture_nonlocked_.fwd_proc_format = StreamConfig(ClosestNativeRate(std::min(formats_.api_format.input_stream().sample_rate_hz(), formats_.api_format.output_stream().sample_rate_hz()); A problem I have with the name proc_rate is that it is very unambiguous to me. I have no intuitive feeling for what it does and always need to check it up. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:83: int get_proc_rate(int input_rate, int output_rate) { The term proc is currently quite ambiguous in APM. Inside APM there are two proc formats (fwd_proc_format and rev_proc_format). But then "proc" in the num_proc_channels() refers only to the number of output channels. I think it is good to at this stage as far as possible avoid using "proc" in variable and function names as it is currently an ambiguous term in APM which could lead to confusion. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:87: for (size_t i = 0; i < AudioProcessing::kNumNativeSampleRates; ++i) { I think this should be an example where an ArrayView could be used to simplify the loop even further for (auto rate : rtc::ArrayView<int>(AudioProcessing::kNativeSampleRatesHz)) { if (rate > min_proc_rate) { return rate; } https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1121: bool AudioProcessingImpl::is_data_processed() const { This is the counterpart to output_copy_needed(). If changing the name of that, it would be good to change the name of this method as well at the same time. Currently it is not possible to tell from the name that it only refers to whether the CaptureStream has been modified. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1142: bool AudioProcessingImpl::output_copy_needed() const { The naming rule https://google.github.io/styleguide/cppguide.html#Function_Names should apply here I think. Furthermore, the output in this method is only used on the render side to determine if the render audio stream has been modified, right? I think it would be good if the name reflects where it is used and what it is asked. What about ReverseStreamModified() or IsReverseStreamModified()? https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1149: bool AudioProcessingImpl::synthesis_needed() const { The naming rule https://google.github.io/styleguide/cppguide.html#Function_Names should apply here I think. Furthermore, this is a bit misleading since rev_synthesis_needed refers to the reverse stream. It would be good to specify that this method only refers to the forward stream to avoid confusion. What about ForwardStreamBandSynthesisNeeded() or ForwardBandSynthesisNeeded() https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1154: bool AudioProcessingImpl::analysis_needed() const { As above, what about What about ForwardStreamBandAnalysisNeeded() or ForwardBandAnalysisNeeded() https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1175: is_multi_band(formats_.rev_proc_format.sample_rate_hz())); As above, what about What about ReverseStreamBandSynthesisNeeded() or ReverseBandSynthesisNeeded() https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1178: bool AudioProcessingImpl::rev_analysis_needed() const { As above, what about What about ReverseStreamBandAnalysisNeeded() or ReverseBandAnalysisNeeded()?
https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:87: for (size_t i = 0; i < AudioProcessing::kNumNativeSampleRates; ++i) { On 2016/03/09 13:25:44, peah-webrtc wrote: > I think this should be an example where an ArrayView could be used to simplify > the loop even further > > for (auto rate : rtc::ArrayView<int>(AudioProcessing::kNativeSampleRatesHz)) { > if (rate > min_proc_rate) { > return rate; > } My error, Hlundins example is actually much better than the ArrayView example I proposed as ArrayView is not needed at all. So I take back my comment.
https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:78: bool is_multi_band(int sample_rate_hz) { On 2016/03/09 13:25:43, peah-webrtc wrote: > I think the style guide warrants a different name > https://google.github.io/styleguide/cppguide.html#Function_Names. > > What about MultiBandSignal()? I thought 2 "==" and 1 "&&" could be considered "cheap". But I accept your suggestion. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:83: int get_proc_rate(int input_rate, int output_rate) { On 2016/03/09 13:25:43, peah-webrtc wrote: > The term proc is currently quite ambiguous in APM. Inside APM there are two proc > formats (fwd_proc_format and rev_proc_format). But then "proc" in the > num_proc_channels() refers only to the number of output channels. > > I think it is good to at this stage as far as possible avoid using "proc" in > variable and function names as it is currently an ambiguous term in APM which > could lead to confusion. "proc" always means "for processing" and both "fwd_proc_format" and "num_proc_channels" mean that. I don't find it ambiguous, but yes slightly confusing, since it depends on a few factors. But I am accepting your suggestion below, so this should not be a problem. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:83: int get_proc_rate(int input_rate, int output_rate) { On 2016/03/09 13:25:44, peah-webrtc wrote: > I think the style guide warrants a different name (right?) > https://google.github.io/styleguide/cppguide.html#Function_Names. > > Furthermore, I think the purpose of the method would be clearer if the min > operation was moved to the actual function call. That would also simplify the > naming. > > E.g. (with a method name change to ClosestNativeRate()) > capture_nonlocked_.fwd_proc_format = > StreamConfig(ClosestNativeRate(std::min(formats_.api_format.input_stream().sample_rate_hz(), > formats_.api_format.output_stream().sample_rate_hz()); > > > A problem I have with the name proc_rate is that it is very unambiguous to me. I > have no intuitive feeling for what it does and always need to check it up. In this case I agree that the definition of "cheap" was slightly stretched. And I agree that moving the min out of the function might make things more intuitive. Accepting your suggestion. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1121: bool AudioProcessingImpl::is_data_processed() const { On 2016/03/09 13:25:43, peah-webrtc wrote: > This is the counterpart to output_copy_needed(). If changing the name of that, > it would be good to change the name of this method as well at the same time. > Currently it is not possible to tell from the name that it only refers to > whether the CaptureStream has been modified. Might make sense, but in another CL. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1142: bool AudioProcessingImpl::output_copy_needed() const { On 2016/03/09 13:25:43, peah-webrtc wrote: > The naming rule https://google.github.io/styleguide/cppguide.html#Function_Names > should apply here I think. > > Furthermore, the output in this method is only used on the render side to > determine if the render audio stream has been modified, right? > > I think it would be good if the name reflects where it is used and what it is > asked. What about ReverseStreamModified() or IsReverseStreamModified()? I am pretty sure this naming was chosen assuming it is a "cheap" method. Changing it might make sense, but in another CL. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1149: bool AudioProcessingImpl::synthesis_needed() const { On 2016/03/09 13:25:43, peah-webrtc wrote: > The naming rule https://google.github.io/styleguide/cppguide.html#Function_Names > should apply here I think. > > Furthermore, this is a bit misleading since rev_synthesis_needed refers to the > reverse stream. It would be good to specify that this method only refers to the > forward stream to avoid confusion. > > What about ForwardStreamBandSynthesisNeeded() or ForwardBandSynthesisNeeded() Again, I am pretty sure all these names were chosen because of the "cheap" nature of the functions. And if we think they should change, we should change all of them together in another CL. On the other hand, I will accept that adding a "fwd_" to the name makes sense, since this CL introduces the "rev_" version of it. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1154: bool AudioProcessingImpl::analysis_needed() const { On 2016/03/09 13:25:43, peah-webrtc wrote: > As above, what about > What about ForwardStreamBandAnalysisNeeded() or ForwardBandAnalysisNeeded() As above, if we change the naming of these functions I think we should do it all in one CL, but I agree that it makes sense to add a "fwd_" to it. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1175: is_multi_band(formats_.rev_proc_format.sample_rate_hz())); On 2016/03/09 13:25:43, peah-webrtc wrote: > As above, what about > What about ReverseStreamBandSynthesisNeeded() or ReverseBandSynthesisNeeded() I would want these to have names that are consistent with the ones already in place. If we think we should rename, we can do this for all of them together in another CL. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1178: bool AudioProcessingImpl::rev_analysis_needed() const { On 2016/03/09 13:25:43, peah-webrtc wrote: > As above, what about > What about ReverseStreamBandAnalysisNeeded() or ReverseBandAnalysisNeeded()? I would want these to have names that are consistent with the ones already in place. If we think we should rename, we can do this for all of them together in another CL.
https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:78: bool is_multi_band(int sample_rate_hz) { On 2016/03/09 14:27:48, aluebs-webrtc wrote: > On 2016/03/09 13:25:43, peah-webrtc wrote: > > I think the style guide warrants a different name > > https://google.github.io/styleguide/cppguide.html#Function_Names. > > > > What about MultiBandSignal()? > > I thought 2 "==" and 1 "&&" could be considered "cheap". But I accept your > suggestion. You are of course right about the cheapness. And having seen the new name in action I actually preferred the former name. Please change back if you want. Sorry for the mess! https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:83: int get_proc_rate(int input_rate, int output_rate) { On 2016/03/09 14:27:48, aluebs-webrtc wrote: > On 2016/03/09 13:25:44, peah-webrtc wrote: > > I think the style guide warrants a different name (right?) > > https://google.github.io/styleguide/cppguide.html#Function_Names. > > > > Furthermore, I think the purpose of the method would be clearer if the min > > operation was moved to the actual function call. That would also simplify the > > naming. > > > > E.g. (with a method name change to ClosestNativeRate()) > > capture_nonlocked_.fwd_proc_format = > > > StreamConfig(ClosestNativeRate(std::min(formats_.api_format.input_stream().sample_rate_hz(), > > formats_.api_format.output_stream().sample_rate_hz()); > > > > > > A problem I have with the name proc_rate is that it is very unambiguous to me. > I > > have no intuitive feeling for what it does and always need to check it up. > > In this case I agree that the definition of "cheap" was slightly stretched. And > I agree that moving the min out of the function might make things more > intuitive. Accepting your suggestion. Nice! Acknowledged. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1142: bool AudioProcessingImpl::output_copy_needed() const { On 2016/03/09 14:27:48, aluebs-webrtc wrote: > On 2016/03/09 13:25:43, peah-webrtc wrote: > > The naming rule > https://google.github.io/styleguide/cppguide.html#Function_Names > > should apply here I think. > > > > Furthermore, the output in this method is only used on the render side to > > determine if the render audio stream has been modified, right? > > > > I think it would be good if the name reflects where it is used and what it is > > asked. What about ReverseStreamModified() or IsReverseStreamModified()? > > I am pretty sure this naming was chosen assuming it is a "cheap" method. > Changing it might make sense, but in another CL. Acknowledged. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1149: bool AudioProcessingImpl::synthesis_needed() const { On 2016/03/09 14:27:48, aluebs-webrtc wrote: > On 2016/03/09 13:25:43, peah-webrtc wrote: > > The naming rule > https://google.github.io/styleguide/cppguide.html#Function_Names > > should apply here I think. > > > > Furthermore, this is a bit misleading since rev_synthesis_needed refers to the > > reverse stream. It would be good to specify that this method only refers to > the > > forward stream to avoid confusion. > > > > What about ForwardStreamBandSynthesisNeeded() or ForwardBandSynthesisNeeded() > > Again, I am pretty sure all these names were chosen because of the "cheap" > nature of the functions. And if we think they should change, we should change > all of them together in another CL. On the other hand, I will accept that adding > a "fwd_" to the name makes sense, since this CL introduces the "rev_" version of > it. Acknowledged. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1154: bool AudioProcessingImpl::analysis_needed() const { On 2016/03/09 14:27:48, aluebs-webrtc wrote: > On 2016/03/09 13:25:43, peah-webrtc wrote: > > As above, what about > > What about ForwardStreamBandAnalysisNeeded() or ForwardBandAnalysisNeeded() > > As above, if we change the naming of these functions I think we should do it all > in one CL, but I agree that it makes sense to add a "fwd_" to it. Acknowledged. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1175: is_multi_band(formats_.rev_proc_format.sample_rate_hz())); On 2016/03/09 14:27:48, aluebs-webrtc wrote: > On 2016/03/09 13:25:43, peah-webrtc wrote: > > As above, what about > > What about ReverseStreamBandSynthesisNeeded() or ReverseBandSynthesisNeeded() > > I would want these to have names that are consistent with the ones already in > place. If we think we should rename, we can do this for all of them together in > another CL. Acknowledged. https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:1178: bool AudioProcessingImpl::rev_analysis_needed() const { On 2016/03/09 14:27:48, aluebs-webrtc wrote: > On 2016/03/09 13:25:43, peah-webrtc wrote: > > As above, what about > > What about ReverseStreamBandAnalysisNeeded() or ReverseBandAnalysisNeeded()? > > I would want these to have names that are consistent with the ones already in > place. If we think we should rename, we can do this for all of them together in > another CL. Acknowledged. https://codereview.webrtc.org/1773173002/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/audio_processing_impl.cc:71: kNativeSampleRatesHz[AudioProcessing::kNumNativeSampleRates - 1]; Afaics, this means that the tri-band 48 kHz splitting filter can be used on any platform, right? What about the complexity for that on ARM, or is that addressed in another CL? https://codereview.webrtc.org/1773173002/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/audio_processing_impl.cc:389: formats_.api_format.reverse_output_stream().sample_rate_hz())); This change basically means that the tri-band 48 kHz splitting filter is activated for the reverse stream, right? I should know this, but as the CL description says "Dont always downsample to 16kHz in the reverse stream in APM" I'm a bit confused. The code did not always downsample to 16 lHz before the change either, right? For 32 kHz the 2-band splitting filter was used, right?
https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/audio_processing_impl.cc:78: bool is_multi_band(int sample_rate_hz) { On 2016/03/10 06:59:24, peah-webrtc wrote: > On 2016/03/09 14:27:48, aluebs-webrtc wrote: > > On 2016/03/09 13:25:43, peah-webrtc wrote: > > > I think the style guide warrants a different name > > > https://google.github.io/styleguide/cppguide.html#Function_Names. > > > > > > What about MultiBandSignal()? > > > > I thought 2 "==" and 1 "&&" could be considered "cheap". But I accept your > > suggestion. > > You are of course right about the cheapness. And having seen the new name in > action I actually preferred the former name. Please change back if you want. > Sorry for the mess! Done. https://codereview.webrtc.org/1773173002/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/audio_processing_impl.cc (right): https://codereview.webrtc.org/1773173002/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/audio_processing_impl.cc:71: kNativeSampleRatesHz[AudioProcessing::kNumNativeSampleRates - 1]; On 2016/03/10 06:59:24, peah-webrtc wrote: > Afaics, this means that the tri-band 48 kHz splitting filter can be used on any > platform, right? What about the complexity for that on ARM, or is that addressed > in another CL? That is adressed in this CL that already landed: https://codereview.webrtc.org/1766103002/ This CL is going to need a rebase and conflict fixing before landing :) https://codereview.webrtc.org/1773173002/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/audio_processing_impl.cc:389: formats_.api_format.reverse_output_stream().sample_rate_hz())); On 2016/03/10 06:59:24, peah-webrtc wrote: > This change basically means that the tri-band 48 kHz splitting filter is > activated for the reverse stream, right? > > I should know this, but as the CL description says "Dont always downsample to > 16kHz in the reverse stream in APM" I'm a bit confused. The code did not always > downsample to 16 lHz before the change either, right? For 32 kHz the 2-band > splitting filter was used, right? That is right. But in that case 32kHz was more of an exception, because the 2-band splitting filter was better than the downsampler.
Now we tested with peah on a AppRTCDemo call that this introduces no regression with the AECM. In fact it might have improved slightly because a splitting filter could be used in the reverse stream as well.
lgtm
On 2016/03/11 14:01:33, hlundin-webrtc wrote: > lgtm lgtm
The CQ bit was checked by aluebs@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from henrik.lundin@webrtc.org, peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/1773173002/#ps80001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773173002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773173002/80001
Description was changed from ========== Dont always downsample to 16kHz in the reverse stream in APM ========== to ========== Dont always downsample to 16kHz in the reverse stream in APM TBR=tina.legrand@webrtc.org ==========
The CQ bit was unchecked by aluebs@webrtc.org
The CQ bit was checked by aluebs@webrtc.org
The CQ bit was unchecked by aluebs@webrtc.org
The CQ bit was checked by aluebs@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773173002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773173002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/13550)
The CQ bit was checked by aluebs@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from henrik.lundin@webrtc.org, peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/1773173002/#ps100001 (title: "update output_data_mac.pb")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773173002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773173002/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/11752)
The CQ bit was checked by aluebs@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from henrik.lundin@webrtc.org, peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/1773173002/#ps120001 (title: "Fix android")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1773173002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1773173002/120001
Message was sent while issue was closed.
Description was changed from ========== Dont always downsample to 16kHz in the reverse stream in APM TBR=tina.legrand@webrtc.org ========== to ========== Dont always downsample to 16kHz in the reverse stream in APM TBR=tina.legrand@webrtc.org ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== Dont always downsample to 16kHz in the reverse stream in APM TBR=tina.legrand@webrtc.org ========== to ========== Dont always downsample to 16kHz in the reverse stream in APM TBR=tina.legrand@webrtc.org Committed: https://crrev.com/df6416aa502d5a9694875a22fcdf286c10f836ea Cr-Commit-Position: refs/heads/master@{#12024} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/df6416aa502d5a9694875a22fcdf286c10f836ea Cr-Commit-Position: refs/heads/master@{#12024} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
