Fixes issue where Web Speech API drops a frame every 5.1 seconds

content/browser/speech/speech_recognizer_impl.cc

 // Copyright (c) 2013 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 "content/browser/speech/speech_recognizer_impl.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/time/time.h"
 #include "content/browser/browser_main_loop.h"
@@ skipping 37 matching lines @@
 
   // Handles resampling, buffering, and channel mixing between input and output
   // parameters.
   AudioConverter audio_converter_;
 
   scoped_ptr<AudioBus> input_bus_;
   scoped_ptr<AudioBus> output_bus_;
   const AudioParameters input_parameters_;
   const AudioParameters output_parameters_;
   bool waiting_for_input_;
+  int convert_count_;
 
   DISALLOW_COPY_AND_ASSIGN(OnDataConverter);
 };
 
 namespace {
 
 // The following constants are related to the volume level indicator shown in
 // the UI for recorded audio.
 // Multiplier used when new volume is greater than previous level.
 const float kUpSmoothingFactor = 1.0f;
@@ skipping 44 matching lines @@
 // SpeechRecognizerImpl::OnDataConverter implementation
 
 SpeechRecognizerImpl::OnDataConverter::OnDataConverter(
     const AudioParameters& input_params,
     const AudioParameters& output_params)
     : audio_converter_(input_params, output_params, false),
       input_bus_(AudioBus::Create(input_params)),
       output_bus_(AudioBus::Create(output_params)),
       input_parameters_(input_params),
       output_parameters_(output_params),
-      waiting_for_input_(false) {
+      waiting_for_input_(false),
+      convert_count_(0) {
   audio_converter_.AddInput(this);
+  DVLOG(1) << "SRI::AudioConverter::ChunkSize " << audio_converter_.ChunkSize();
+
+  // Initial priming with zeros...
+  // waiting_for_input_ = true;
+  // input_bus_->Zero();
+  // audio_converter_.Convert(output_bus_.get());
+
+  audio_converter_.PrimeWithSilence();
+  DVLOG(1) << "SRI::AudioConverter::ChunkSize " << audio_converter_.ChunkSize();
 }
 
 SpeechRecognizerImpl::OnDataConverter::~OnDataConverter() {
   // It should now be safe to unregister the converter since no more OnData()
   // callbacks are outstanding at this point.
   audio_converter_.RemoveInput(this);
 }
 
 scoped_refptr<AudioChunk> SpeechRecognizerImpl::OnDataConverter::Convert(
     const AudioBus* data) {
   CHECK_EQ(data->frames(), input_parameters_.frames_per_buffer());
-
+  DVLOG(1) << "SRI::ODC::Convert...";
+  // Data should always have been provided by ProvideInput(). If not, the
+  // previous call to Convert() could produce converted data using cached
+  // data. But that means that we will miss one large frame of recorded audio
+  // samples.
+  CHECK(!waiting_for_input_);
   data->CopyTo(input_bus_.get());
 
   waiting_for_input_ = true;
+  convert_count_++;
   audio_converter_.Convert(output_bus_.get());

[DaleCurtis, 2015/07/07 16:38:17]

As mentioned in the email thread, you should probably just have a "bool
provide_input_called_" and if you have data and it isn't, then call convert
again and generate another AudioChunk.

[henrika (OOO until Aug 14), 2015/07/07 19:30:05]

Smart. But note that I have modified the input size to 100ms (used to be 102) in
this patch as well. And it will result in that ProvideInput is always called. Do
you want me to restore 102 and use "double convert when needed" or keep 100. I
don't remember the exact reason why we selected 102 in the first place.

[DaleCurtis, 2015/07/07 20:40:44]

Well, I'm not confident your modification always ensures a ProvideInput() call;
due to rounding over time you may end up without a ProvideInput() call, possibly
speech input is short enough that this isn't a problem though.  I'm not sure
it's possible to make a stricter guarantee than  "at most 1 call."

[henrika (OOO until Aug 14), 2015/07/07 21:14:08]

Got it. I will test your scheme using 102 where I know it will be triggered once
every 5th seconds. Then go back to 100 but keep the scheme just in case.

[DaleCurtis, 2015/07/07 21:44:15]

I think it's fine to use 100ms like you do w/ prime with silence, you just need
to also handle the extra-convert case too.

[henrika (OOO until Aug 14), 2015/07/08 12:22:59]

Done. Hope you are OK with how I handle the extra convert case. I wanted to make
it clear that it should be a rare event.

+  DVLOG(1) << "SRI::ODC::Convert done (" << convert_count_ << ")";

[tommi (sloooow) - chröme, 2015/07/07 12:40:21]

seems like this will always log a value higher than 0 (up to max int of course)
and convert_count_ is otherwise not used (or maybe I'm missing something).  It
seems like that if we really need this variable, we should get rid of
waiting_for_input_.

[henrika (OOO until Aug 14), 2015/07/07 12:56:54]

Got it. Plan is to remove this counter. Just wanted to see where (~5.1) we end
up in a state where we don't ask for new input data but instead read from the
internal storage inside the Converter (that is when we miss 102ms of input data
currently).

 
   scoped_refptr<AudioChunk> chunk(
       new AudioChunk(output_parameters_.GetBytesPerBuffer(),
                      output_parameters_.bits_per_sample() / 8));
   output_bus_->ToInterleaved(output_bus_->frames(),
                              output_parameters_.bits_per_sample() / 8,
                              chunk->writable_data());
   return chunk;
 }
 
 double SpeechRecognizerImpl::OnDataConverter::ProvideInput(
     AudioBus* dest, base::TimeDelta buffer_delay) {
+  DVLOG(1) << "SRI::ODC::ProvideInput";
   // The audio converted should never ask for more than one bus in each call
   // to Convert(). If so, we have a serious issue in our design since we might
   // miss recorded chunks of 100 ms audio data.
   CHECK(waiting_for_input_);
 
   // Read from the input bus to feed the converter.
   input_bus_->CopyTo(dest);
 
   // |input_bus_| should only be provide once.
   waiting_for_input_ = false;
@@ skipping 354 matching lines @@
     return Abort(
         SpeechRecognitionError(SPEECH_RECOGNITION_ERROR_AUDIO_CAPTURE));
   }
 
   // Audio converter shall provide audio based on these parameters as output.
   // Hard coded, WebSpeech specific parameters are utilized here.
   int frames_per_buffer = (kAudioSampleRate * chunk_duration_ms) / 1000;
   AudioParameters output_parameters = AudioParameters(
       AudioParameters::AUDIO_PCM_LOW_LATENCY, kChannelLayout, kAudioSampleRate,
       kNumBitsPerAudioSample, frames_per_buffer);
+  DVLOG(1) << "SRI::output_parameters: "
+           << output_parameters.AsHumanReadableString();
 
   // Audio converter will receive audio based on these parameters as input.
   // On Windows we start by verifying that Core Audio is supported. If not,
   // the WaveIn API is used and we might as well avoid all audio conversations
   // since WaveIn does the conversion for us.
   // TODO(henrika): this code should be moved to platform dependent audio
   // managers.
   bool use_native_audio_params = true;
 #if defined(OS_WIN)

[DaleCurtis, 2015/07/07 20:40:43]

I'm surprised we're not using the native params for all platforms, but I guess
if it works it works.

[henrika (OOO until Aug 14), 2015/07/07 21:14:08]

Hmm, but we are on all but Win XP. All others use native. Clear, or am I missing
something?

[DaleCurtis, 2015/07/07 21:44:15]

Ah no, I misread and inverted the check.

   use_native_audio_params = media::CoreAudioUtil::IsSupported();
   DVLOG_IF(1, !use_native_audio_params) << "Reverting to WaveIn for WebSpeech";
 #endif
 
   AudioParameters input_parameters = output_parameters;
   if (use_native_audio_params && !unit_test_is_active) {
     // Use native audio parameters but avoid opening up at the native buffer
     // size. Instead use same frame size (in milliseconds) as WebSpeech uses.
     // We rely on internal buffers in the audio back-end to fulfill this request
     // and the idea is to simplify the audio conversion since each Convert()
     // call will then render exactly one ProvideInput() call.
     // Due to implementation details in the audio converter, 2 milliseconds
     // are added to the default frame size (100 ms) to ensure there is enough
     // data to generate 100 ms of output when resampling.
+    // frames_per_buffer =
+    //    ((in_params.sample_rate() * (chunk_duration_ms + 2)) / 1000.0) + 0.5;
     frames_per_buffer =
-        ((in_params.sample_rate() * (chunk_duration_ms + 2)) / 1000.0) + 0.5;
+        ((in_params.sample_rate() * chunk_duration_ms) / 1000.0) + 0.5;
     input_parameters.Reset(in_params.format(),
                            in_params.channel_layout(),
                            in_params.channels(),
                            in_params.sample_rate(),
                            in_params.bits_per_sample(),
                            frames_per_buffer);
+    DVLOG(1) << "SRI::input_parameters: "
+             << input_parameters.AsHumanReadableString();
   }
 
   // Create an audio converter which converts data between native input format
   // and WebSpeech specific output format.
   audio_converter_.reset(
       new OnDataConverter(input_parameters, output_parameters));
 
   audio_controller_ = AudioInputController::Create(
       audio_manager, this, input_parameters, device_id_, NULL);
 
@@ skipping 245 matching lines @@
 SpeechRecognizerImpl::FSMEventArgs::FSMEventArgs(FSMEvent event_value)
     : event(event_value),
       audio_data(NULL),
       engine_error(SPEECH_RECOGNITION_ERROR_NONE) {
 }
 
 SpeechRecognizerImpl::FSMEventArgs::~FSMEventArgs() {
 }
 
 }  // namespace content