Binding media stream audio track to speech recognition [renderer]

content/renderer/speech_recognition_dispatcher.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 "content/renderer/speech_recognition_dispatcher.h"
 
 #include "base/basictypes.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/common/speech_recognition_messages.h"
 #include "content/renderer/render_view_impl.h"
+#include "content/renderer/speech_recognition_audio_source_provider.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebVector.h"
 #include "third_party/WebKit/public/web/WebSpeechGrammar.h"
 #include "third_party/WebKit/public/web/WebSpeechRecognitionParams.h"
 #include "third_party/WebKit/public/web/WebSpeechRecognitionResult.h"
 #include "third_party/WebKit/public/web/WebSpeechRecognizerClient.h"
 
 using blink::WebVector;
 using blink::WebString;
 using blink::WebSpeechGrammar;
 using blink::WebSpeechRecognitionHandle;
 using blink::WebSpeechRecognitionResult;
 using blink::WebSpeechRecognitionParams;
 using blink::WebSpeechRecognizerClient;
 
 namespace content {
 
 SpeechRecognitionDispatcher::SpeechRecognitionDispatcher(
     RenderViewImpl* render_view)
     : RenderViewObserver(render_view),
       recognizer_client_(NULL),
+      audio_track_set_(false),
+      is_allowed_audio_track_(false),
+      render_loop_(base::MessageLoopProxy::current()),
       next_id_(1) {
 }
 
 SpeechRecognitionDispatcher::~SpeechRecognitionDispatcher() {
 }
 
 void SpeechRecognitionDispatcher::AbortAllRecognitions() {
+  audio_source_provider_.reset();
   Send(new SpeechRecognitionHostMsg_AbortAllRequests(
       routing_id()));
 }
 
 bool SpeechRecognitionDispatcher::OnMessageReceived(
     const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(SpeechRecognitionDispatcher, message)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_Started, OnRecognitionStarted)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_AudioStarted, OnAudioStarted)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_SoundStarted, OnSoundStarted)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_SoundEnded, OnSoundEnded)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_AudioEnded, OnAudioEnded)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_ErrorOccurred, OnErrorOccurred)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_Ended, OnRecognitionEnded)
     IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_ResultRetrieved,
                         OnResultsRetrieved)
+    IPC_MESSAGE_HANDLER(SpeechRecognitionMsg_AudioTrackReady,
+                        OnAudioTrackReady)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
+void SpeechRecognitionDispatcher::attach(
+    const blink::WebSpeechRecognitionHandle& handle,
+    const blink::WebMediaStreamTrack& audio_track,
+    blink::WebSpeechRecognizerClient* recognizer_client) {
+

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

remove this empty line

[burnik, 2014/08/29 13:26:17]

Done.

+  // Check if track is from an allowed source (microphone only for now)
+  is_allowed_audio_track_ =
+      SpeechRecognitionAudioSourceProvider::IsAllowedAudioTrack(audio_track);

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

would it make sense to have the IsAllowedAudioTrack() (or whatever it might get
renamed to) implemented in this file?  (in an anonymous namespace at the top of
this file)

This seems to be the only code that needs that functionality.

[burnik, 2014/08/29 13:26:18]

My intention is to move away as much logic as possible from the dispatcher.
Makes it much more easier to test. I agree that this static method should belong
perhaps to another class - such as |SpeechRecognitionAudioTrackPolicy| so I can
inject the policy rather than having it hardcoded.

On 2014/08/29 11:25:31, tommi wrote:

+  audio_track_ = audio_track;

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

first DCHECK that audio_track_ isn't valid?

[burnik, 2014/08/29 13:26:17]

Method needs refactoring.
On 2014/08/29 11:25:31, tommi wrote:

+  audio_track_set_ = true;

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

Should we DCHECK(!audio_track_set_); at the top of this function?

+}
+
+void SpeechRecognitionDispatcher::detach(
+    const blink::WebSpeechRecognitionHandle& handle,
+    blink::WebSpeechRecognizerClient* recognizer_client) {
+  audio_track_set_ = false;

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

reset/clear audio_track_ as well and set is_allowed_audio_track_ to false?

[burnik, 2014/08/29 13:26:17]

Method needs refactoring.
On 2014/08/29 11:25:31, tommi wrote:

+}
+
 void SpeechRecognitionDispatcher::start(
     const WebSpeechRecognitionHandle& handle,
     const WebSpeechRecognitionParams& params,
     WebSpeechRecognizerClient* recognizer_client) {
   DCHECK(!recognizer_client_ || recognizer_client_ == recognizer_client);
   recognizer_client_ = recognizer_client;
 
+  // destroy any previous instance not to starve it waiting on chunk ACKs

[no longer working on chromium, 2014/08/29 12:23:07]

Destroy

[burnik, 2014/08/29 13:26:17]

Done.

+  audio_source_provider_.reset();
+
+  if (audio_track_set_ && !is_allowed_audio_track_) {

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

do you really need is_allowed_audio_track_?  If the track isn't
allowed/supported, then shouldn't we simply not set audio_track_set_ to true?

[burnik, 2014/08/29 13:26:18]

The JS |webkitSpeechRecognition.audioTrack| is not a mandatory field and can
therefore be null. That indicates audio_track_set_ == false and falls back to
default implementation on the browser (using AudioInputController). However if
we set |webkitSpeechRecognition.audioTrack| to a WebAudio MediaStreamTrack, then
it is set (the reference holds in JS). We should then tell the JS dev that this
is not allowed but still fall back so SR session starts. That's why I need two
members.

However - I think I can get rid of audio_track_set_ and use a pointer for
audio_track_ instead.

BTW - This is a good topic of discussion regarding how this should behave
towards the user in these corner cases.

On 2014/08/29 11:25:31, tommi wrote:

+    // notify user that the track used is not supported
+    recognizer_client_->didReceiveError(
+          handle,
+          WebString("Provided audioTrack is not supported. Ignoring track."),
+          WebSpeechRecognizerClient::NotAllowedError);

[no longer working on chromium, 2014/08/29 12:23:07]

probably you should fail the start call in such case.

[burnik, 2014/08/29 13:26:17]

Good for discussion.
On 2014/08/29 12:23:07, xians1 wrote:

+  }
+
   SpeechRecognitionHostMsg_StartRequest_Params msg_params;
   for (size_t i = 0; i < params.grammars().size(); ++i) {
     const WebSpeechGrammar& grammar = params.grammars()[i];
     msg_params.grammars.push_back(
         SpeechRecognitionGrammar(grammar.src().spec(), grammar.weight()));
   }
   msg_params.language = base::UTF16ToUTF8(params.language());
   msg_params.max_hypotheses = static_cast<uint32>(params.maxAlternatives());
   msg_params.continuous = params.continuous();
   msg_params.interim_results = params.interimResults();
   msg_params.origin_url = params.origin().toString().utf8();
   msg_params.render_view_id = routing_id();
   msg_params.request_id = GetOrCreateIDForHandle(handle);
+  // fall back to default input when the track is not allowed
+  msg_params.using_audio_track = (audio_track_set_ && is_allowed_audio_track_);

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

looks like you always test these variables together..

actually, I'm wondering if you need either of them.  What if you change attach()
so that it does not assign to audio_track_ if the track isn't allowed/supported
and then simply check audio_track_.isNull() here (and elsewhere where you
currently check these two variables)?

detach() would of course have to call audio_track_.reset() then (which I think
it should anyway).

[burnik, 2014/08/29 13:26:18]

Same as comment above. The JS API behaviour is not set in stone. More discussion
should resolve this as well.

On 2014/08/29 11:25:31, tommi wrote:

+  msg_params.peer_process_handle = base::GetCurrentProcessHandle();
   // The handle mapping will be removed in |OnRecognitionEnd|.
   Send(new SpeechRecognitionHostMsg_StartRequest(msg_params));
 }
 
 void SpeechRecognitionDispatcher::stop(
     const WebSpeechRecognitionHandle& handle,
     WebSpeechRecognizerClient* recognizer_client) {
+  audio_source_provider_.reset();
   // Ignore a |stop| issued without a matching |start|.
   if (recognizer_client_ != recognizer_client || !HandleExists(handle))
     return;
   Send(new SpeechRecognitionHostMsg_StopCaptureRequest(
       routing_id(), GetOrCreateIDForHandle(handle)));
 }
 
 void SpeechRecognitionDispatcher::abort(
     const WebSpeechRecognitionHandle& handle,
     WebSpeechRecognizerClient* recognizer_client) {
+  audio_source_provider_.reset();
   // Ignore an |abort| issued without a matching |start|.
   if (recognizer_client_ != recognizer_client || !HandleExists(handle))
     return;
   Send(new SpeechRecognitionHostMsg_AbortRequest(
       routing_id(), GetOrCreateIDForHandle(handle)));
 }
 
 void SpeechRecognitionDispatcher::OnRecognitionStarted(int request_id) {
   recognizer_client_->didStart(GetHandleFromID(request_id));
 }
@@ skipping 39 matching lines @@
   NOTREACHED();
   return WebSpeechRecognizerClient::OtherError;
 }
 
 void SpeechRecognitionDispatcher::OnErrorOccurred(
     int request_id, const SpeechRecognitionError& error) {
   if (error.code == SPEECH_RECOGNITION_ERROR_NO_MATCH) {
     recognizer_client_->didReceiveNoMatch(GetHandleFromID(request_id),
                                           WebSpeechRecognitionResult());
   } else {
+    audio_source_provider_.reset();

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

I'm assuming that the browser side will be aware of the now 'stopped or aborted'
state.  Is that correct?

[burnik, 2014/08/29 13:26:18]

This message is received from the browser. Sockets should resolve the rest.
Although, I should inspect this event further.
On 2014/08/29 11:25:31, tommi wrote:

     recognizer_client_->didReceiveError(
         GetHandleFromID(request_id),
         WebString(),  // TODO(primiano): message?
         WebKitErrorCode(error.code));
   }
 }
 
 void SpeechRecognitionDispatcher::OnRecognitionEnded(int request_id) {
   // TODO(tommi): It is possible that the handle isn't found in the array if
   // the user just refreshed the page. It seems that we then get a notification
   // for the previously loaded instance of the page.
   HandleMap::iterator iter = handle_map_.find(request_id);
   if (iter == handle_map_.end()) {
     DLOG(ERROR) << "OnRecognitionEnded called for a handle that doesn't exist";
   } else {
     WebSpeechRecognitionHandle handle = iter->second;
     // Note: we need to erase the handle from the map *before* calling didEnd.
     // didEnd may call back synchronously to start a new recognition session,
     // and we don't want to delete the handle from the map after that happens.
     handle_map_.erase(request_id);
+    audio_source_provider_.reset();

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

same question here since this feels functionally comparable to stop() or abort()
minus the message sent to the browser.

[burnik, 2014/08/29 13:26:17]

This effectively kills of the sockets so the audio thread on the browser stops.
On 2014/08/29 11:25:31, tommi wrote:

     recognizer_client_->didEnd(handle);
   }
 }
 
 void SpeechRecognitionDispatcher::OnResultsRetrieved(
     int request_id, const SpeechRecognitionResults& results) {
   size_t provisional_count = 0;
   SpeechRecognitionResults::const_iterator it = results.begin();
   for (; it != results.end(); ++it) {
     if (it->is_provisional)
@@ skipping 17 matching lines @@
       transcripts[i] = result.hypotheses[i].utterance;
       confidences[i] = static_cast<float>(result.hypotheses[i].confidence);
     }
     webkit_result->assign(transcripts, confidences, !result.is_provisional);
   }
 
   recognizer_client_->didReceiveResults(
       GetHandleFromID(request_id), final, provisional);
 }
 
+void SpeechRecognitionDispatcher::OnAudioTrackReady(
+    int request_id,
+    const media::AudioParameters& params,
+    base::SharedMemoryHandle memory,
+    base::NativeSyncSocket::Descriptor socket,
+    uint32 length) {
+  // TODO(burnik): Log and DCHECK(!audio_source_provider_).

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

can you do this now?

[burnik, 2014/08/29 13:26:18]

It's still a bit fuzzy because we can only have one active session and I'm not
sure what multiple sessions would do. Tests will show.
On 2014/08/29 11:25:31, tommi wrote:

+  if (audio_track_.isNull()) {
+    audio_source_provider_.reset();

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

should this be done in detach() also?

[burnik, 2014/08/29 13:26:17]

I would have to dig in deeper to check if this would be needed. I will probably
make these methods more robust. Tests will show.
On 2014/08/29 11:25:31, tommi wrote:

+    return;
+  }
+  audio_source_provider_.reset(new SpeechRecognitionAudioSourceProvider(
+    audio_track_, params, memory, socket, length));

[tommi (sloooow) - chröme, 2014/08/29 11:25:31]

indent

[burnik, 2014/08/29 13:26:17]

Done.

+}
 
 int SpeechRecognitionDispatcher::GetOrCreateIDForHandle(
     const WebSpeechRecognitionHandle& handle) {
   // Search first for an existing mapping.
   for (HandleMap::iterator iter = handle_map_.begin();
       iter != handle_map_.end();
       ++iter) {
     if (iter->second.equals(handle))
       return iter->first;
   }
@@ skipping 16 matching lines @@
 }
 
 const WebSpeechRecognitionHandle& SpeechRecognitionDispatcher::GetHandleFromID(
     int request_id) {
   HandleMap::iterator iter = handle_map_.find(request_id);
   DCHECK(iter != handle_map_.end());
   return iter->second;
 }
 
 }  // namespace content