Refactoring of chrome speech recognition architecture (CL1.3)

content/browser/speech/speech_recognition_engine.h

+// 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.
+
+#ifndef CONTENT_BROWSER_SPEECH_SPEECH_RECOGNITION_ENGINE_H_
+#define CONTENT_BROWSER_SPEECH_SPEECH_RECOGNITION_ENGINE_H_
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "content/public/common/speech_recognition_result.h"
+
+namespace content {
+struct SpeechRecognitionResult;
+struct SpeechRecognitionError;
+}
+
+namespace speech {
+
+class AudioChunk;
+
+// This interface models the basic contract that a speech recognition engine,
+// either working locally or relying on a remote web-service, must obey.
+// The expected call sequence for exported methods is:
+// SpeechRecognitionBegins       Mandatory at beginning of SR.
+//   PushSpeechAudio             For every audio chunk pushed.
+//   SpeechAudioStreamComplete   Finalize the audio stream (if no errors).
+// SpeechRecognitionEnds         Mandatory at end of SR (even on errors).
+class SpeechRecognitionEngine {
+ public:
+  // Interface for receiving callbacks from this object.
+  class Delegate {
+   public:
+    virtual void OnSpeechEngineResult(

[Satish, 2012/03/16 17:00:35]

OnSpeech -> OnSpeechRecognition ?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

I'd prefer OnSpeechEngine since this interface models the events notified by the
Engine. It makes it more easy to understand the code in the class implementing
this delegate (recognizer).

On 2012/03/16 17:00:35, Satish wrote:

+        const content::SpeechRecognitionResult& result) = 0;
+    virtual void OnSpeechEngineError(

[Satish, 2012/03/16 17:00:35]

ditto

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

Same as above.
On 2012/03/16 17:00:35, Satish wrote:

+        const content::SpeechRecognitionError& error) = 0;
+   protected:
+    virtual ~Delegate() {}
+  };
+
+  virtual ~SpeechRecognitionEngine() {}
+
+  // Notify the engine that we started the recognition, so it can prepare for
+  // the upcoming PushSpeechAudio() calls.
+  virtual void SpeechRecognitionBegins() = 0;

[Satish, 2012/03/16 17:00:35]

perhaps 'StartSession' or something similar?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

Agree with the ambiguity of names.
However I don't like using the term session, since there is no notion of session
in the speech engine. Sessions are introduced into the manager/recognizer in
order to distinguish different requests.
I renamed all the engine methods, now should be more clear.

On 2012/03/16 17:00:35, Satish wrote:

+
+  // Notify the engine that we have ended (either gracefully or aborting) the

[Satish, 2012/03/16 17:00:35]

I don't see a way to abort in this class. Should there be one?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

Basically, Cleanup() does it, since can be called at any time (after an
initialize)
On 2012/03/16 17:00:35, Satish wrote:

+  // recognition, so it can cleanup.
+  virtual void SpeechRecognitionEnds() = 0;

[Satish, 2012/03/16 17:00:35]

EndSession ?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

Cleanup?

On 2012/03/16 17:00:35, Satish wrote:

+
+  // Push a chunk of uncompressed audio data, where the chunk length agrees with
+  // DesiredAudioChunkDurationMs(), and bytes_per_sample agrees to the engine
+  // configuration.
+  virtual void PushSpeechAudio(const AudioChunk& data) = 0;

[Satish, 2012/03/16 17:00:35]

AddAudioChunk or something similar?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

TakeAudioChunk?
On 2012/03/16 17:00:35, Satish wrote:

+
+  // Notifies the engine that audio capture has completed and no more chunks
+  // will be pushed. The engine can finalize the recognition in order to provide

[Satish, 2012/03/16 17:00:35]

"finalize the recognition in order to provide a final result" - sounds quite
abstract :) Can you clarify more or if we can't just remove this line?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

Done.

+  // a final result.
+  virtual void SpeechAudioStreamComplete() = 0;

[Satish, 2012/03/16 17:00:35]

AudioCompleted or something similar?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

AudioChunksEnded?
On 2012/03/16 17:00:35, Satish wrote:

+
+  // Checks wheter recognition of pushed audio data is pending.
+  virtual bool IsRecognitionPending() const = 0;
+
+  // Retrieves the desired duration, in milliseconds, of pushed AudioChunk(s).
+  virtual int DesiredAudioChunkDurationMs() const = 0;

[Satish, 2012/03/16 17:00:35]

Desired -> GetDesired ?

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

Done.

+
+  // set_delegate detached from constructor for lazy dependency injection.
+  void set_delegate(Delegate* delegate) { delegate_ = delegate; }
+
+ protected:
+   Delegate* delegate() const { return delegate_; }
+
+ private:

[Satish, 2012/03/16 17:00:35]

add DISALLOW_COPY_AND_ASSIGN(SpeechRecognitionEngine);

[Primiano Tucci (use gerrit), 2012/03/20 13:14:50]

Done.

+   Delegate* delegate_;
+};
+
+// This typedef is to workaround the issue with certain versions of
+// Visual Studio where it gets confused between multiple Delegate
+// classes and gives a C2500 error. (I saw this error on the try bots -
+// the workaround was not needed for my machine).
+typedef SpeechRecognitionEngine::Delegate SpeechRecognitionEngineDelegate;
+
+}  // namespace speech
+
+#endif  // CONTENT_BROWSER_SPEECH_SPEECH_RECOGNITION_ENGINE_H_