Reduce jitter from uneven SincResampler buffer size requests.

remoting/codec/audio_encoder_opus.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/codec/audio_encoder_opus.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/time.h"
 #include "media/base/audio_bus.h"
@@ skipping 62 matching lines @@
 
   frame_size_ = sampling_rate_ * kFrameSizeMs /
       base::Time::kMillisecondsPerSecond;
 
   if (sampling_rate_ != kOpusSamplingRate) {
     resample_buffer_.reset(
         new char[kFrameSamples * kBytesPerSample * channels_]);
     resampler_.reset(new media::MultiChannelResampler(
         channels_,
         static_cast<double>(sampling_rate_) / kOpusSamplingRate,
+        media::SincResampler::kDefaultBlockSize,

[Sergey Ulanov, 2013/04/29 06:23:37]

I think it would be better to mach default frame size we use in opus. Can you
please define kFrameSamples = 960 in this file and use it here instead of
kDefaultBlockSize? (the frame size is defined in audio_decoder_opus.cc, it's
worth adding a reference to that file).

[Sergey Ulanov, 2013/04/29 07:16:54]

Sorry, just realized that kFrameSamples is already defined in this file, just
use it here, please :-) 

[DaleCurtis, 2013/04/29 22:12:31]

kFrameSamples appears to refer to the output sample size, not the input sample
size.  Is this still what you want?  If the packet size provided to Encode() is
constant, it seems you'd want to use that value.

[Sergey Ulanov, 2013/05/01 02:06:25]

Oh, right. Then it should be frame_size_, which is frame size in input samples.

[DaleCurtis, 2013/05/07 23:49:41]

Hmm, I'm not sure about this, so I've just kept the original behavior for now
and added a TODO(sergeyu) :)

         base::Bind(&AudioEncoderOpus::FetchBytesToResample,
                    base::Unretained(this))));
     resampler_bus_ = media::AudioBus::Create(channels_, kFrameSamples);
   }
 
   // Drop leftover data because it's for different sampling rate.
   leftover_samples_ = 0;
   leftover_buffer_size_ =
-      frame_size_ + media::SincResampler::kMaximumLookAheadSize;
+      frame_size_ + media::SincResampler::kDefaultBlockSize;
   leftover_buffer_.reset(
       new int16[leftover_buffer_size_ * channels_]);
 }
 
 void AudioEncoderOpus::DestroyEncoder() {
   if (encoder_) {
     opus_encoder_destroy(encoder_);
     encoder_ = NULL;
   }
 
@@ skipping 49 matching lines @@
   int samples_in_packet = packet->data(0).size() / kBytesPerSample / channels_;
   const int16* next_sample =
       reinterpret_cast<const int16*>(packet->data(0).data());
 
   // Create a new packet of encoded data.
   scoped_ptr<AudioPacket> encoded_packet(new AudioPacket());
   encoded_packet->set_encoding(AudioPacket::ENCODING_OPUS);
   encoded_packet->set_sampling_rate(kOpusSamplingRate);
   encoded_packet->set_channels(channels_);
 
   int prefetch_samples =

[Sergey Ulanov, 2013/05/01 02:06:25]

Not sure if prefetching is still required. If not (i.e. if resampler never
requests more than it needs) then please remove it.

-      resampler_.get() ? media::SincResampler::kMaximumLookAheadSize : 0;
+      resampler_.get() ? media::SincResampler::kDefaultBlockSize : 0;
   int samples_wanted = frame_size_ + prefetch_samples;
 
   while (leftover_samples_ + samples_in_packet >= samples_wanted) {
     const int16* pcm_buffer = NULL;
 
     // Combine the packet with the leftover samples, if any.
     if (leftover_samples_ > 0) {
       pcm_buffer = leftover_buffer_.get();
       int samples_to_copy = samples_wanted - leftover_samples_;
       memcpy(leftover_buffer_.get() + leftover_samples_ * channels_,
@@ skipping 59 matching lines @@
   }
 
   // Return NULL if there's nothing in the packet.
   if (encoded_packet->data_size() == 0)
     return scoped_ptr<AudioPacket>();
 
   return encoded_packet.Pass();
 }
 
 }  // namespace remoting