Support for providing media stream audio track to speech recognition

Support for providing media stream audio track to speech recognition - Blink implementation.

[ WebKit CL ]
  Renderer CL    : http://crrev.com/499233003
  Browser CL     : http://crrev.com/549373003

Introduces |webkitSpeechRecognition.audioTrack| attribute. Set to null by default.

When the attribute is set to null, SR is done by default implementation
(getting audio from the audio input device on the browser).

When the attribute is set to a MediaStreamTrack it should get audio data 
from that track after |start()|. The track is utilized once |start()| is called.

For unsupported tracks (e.g. from a file rather than a microphone)
the chromium implementation should bubble up the error once |start()| is called.

Changing of the attribute after |start()| is called should have no effect 
on the running session. This is the same behavioral pattern as for the other attributes.

Design doc: http://goo.gl/9Ot3PC

Intent to implement: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/G2lRTzZXmW4

BUG=408940
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182851

[no longer working on chromium, 2014-08-14 15:41:34]

Some thoughts and comments.

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognition.cpp (right):

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.cpp:60:
m_controller->clearAudioTrack(this);
you don't need clearAudioTrack, just do m_controller->setAudioTrack(this,
m_audioTrack) regardless if m_audioTrack is nullptr or not.

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.cpp:183: , m_stopping(false)
initialize m_audioTrack to null here.

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognition.idl (right):

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.idl:38: attribute MediaStreamTrack
audioTrack;
how about using attachWithAudioTrack

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognitionClient.h (right):

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognitionClient.h:41: virtual void
clearAudioTrack(SpeechRecognition*) = 0;
remove

https://codereview.chromium.org/448163003/diff/40001/Source/web/SpeechRecogni...
File Source/web/SpeechRecognitionClientProxy.cpp (right):

https://codereview.chromium.org/448163003/diff/40001/Source/web/SpeechRecogni...
Source/web/SpeechRecognitionClientProxy.cpp:58:
m_recognizer->setAudioTrack(WebSpeechRecognitionHandle(recognition),
audioTrack->component(), this);
how about
if (audioTrack) {
   m_recognizer->Attach(WebSpeechRecognitionHandle(recognition), audioTr
     ack->component(), this);
} else {
   m_recognizer->Detach(WebSpeechRecognitionHandle(recognition), this);
}

https://codereview.chromium.org/448163003/diff/40001/public/web/WebSpeechReco...
File public/web/WebSpeechRecognizer.h (right):

https://codereview.chromium.org/448163003/diff/40001/public/web/WebSpeechReco...
public/web/WebSpeechRecognizer.h:43: virtual void setAudioTrack(const
WebSpeechRecognitionHandle&, const WebMediaStreamTrack& audioTrack,
WebSpeechRecognizerClient*) { BLINK_ASSERT_NOT_REACHED(); }
rename it to Attach() and Detach()

[burnik, 2014-08-14 16:05:47]

Initial proposal to support WebMediaStreamTrack for SR.

[burnik, 2014-08-14 16:35:15]

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognition.cpp (right):

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.cpp:60:
m_controller->clearAudioTrack(this);
On 2014/08/14 15:41:34, xians1 wrote:
> you don't need clearAudioTrack, just do m_controller->setAudioTrack(this,
> m_audioTrack) regardless if m_audioTrack is nullptr or not.

Done.

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognition.idl (right):

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.idl:38: attribute MediaStreamTrack
audioTrack;
We should discuss more on the name.
On 2014/08/14 15:41:34, xians1 wrote:
> how about using attachWithAudioTrack

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognitionClient.h (right):

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognitionClient.h:41: virtual void
clearAudioTrack(SpeechRecognition*) = 0;
On 2014/08/14 15:41:34, xians1 wrote:
> remove

Done.

https://codereview.chromium.org/448163003/diff/40001/Source/web/SpeechRecogni...
File Source/web/SpeechRecognitionClientProxy.cpp (right):

https://codereview.chromium.org/448163003/diff/40001/Source/web/SpeechRecogni...
Source/web/SpeechRecognitionClientProxy.cpp:58:
m_recognizer->setAudioTrack(WebSpeechRecognitionHandle(recognition),
audioTrack->component(), this);
I wonder if it's a good design to introduce logic on the proxy - maybe this
should reside on the |SpeechRecognition|.

On 2014/08/14 15:41:34, xians1 wrote:
> how about
> if (audioTrack) {
>    m_recognizer->Attach(WebSpeechRecognitionHandle(recognition), audioTr
>      ack->component(), this);
> } else {
>    m_recognizer->Detach(WebSpeechRecognitionHandle(recognition), this);
> }
>

https://codereview.chromium.org/448163003/diff/40001/public/web/WebSpeechReco...
File public/web/WebSpeechRecognizer.h (right):

https://codereview.chromium.org/448163003/diff/40001/public/web/WebSpeechReco...
public/web/WebSpeechRecognizer.h:43: virtual void setAudioTrack(const
WebSpeechRecognitionHandle&, const WebMediaStreamTrack& audioTrack,
WebSpeechRecognizerClient*) { BLINK_ASSERT_NOT_REACHED(); }
On 2014/08/14 15:41:34, xians1 wrote:
> rename it to Attach() and Detach()

Done.

[burnik, 2014-08-14 16:38:44]

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognition.cpp (right):

https://codereview.chromium.org/448163003/diff/40001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.cpp:183: , m_stopping(false)
On 2014/08/14 15:41:34, xians1 wrote:
> initialize m_audioTrack to null here.

Done.

[no longer working on chromium, 2014-08-15 10:22:56]

Glen, could you please take a look at this CL?

Many thanks,
SX

https://codereview.chromium.org/448163003/diff/60001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognition.cpp (right):

https://codereview.chromium.org/448163003/diff/60001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.cpp:57: m_controller->start(this,
m_grammars.get(), m_lang, m_continuous, m_interimResults, m_maxAlternatives);
I am wondering if we should just pass the m_audioTrack in the start method. Lets
get Glen's opinion on this.

[burnik, 2014-08-15 11:29:41]

https://codereview.chromium.org/448163003/diff/60001/Source/modules/speech/Sp...
File Source/modules/speech/SpeechRecognition.cpp (right):

https://codereview.chromium.org/448163003/diff/60001/Source/modules/speech/Sp...
Source/modules/speech/SpeechRecognition.cpp:57: m_controller->start(this,
m_grammars.get(), m_lang, m_continuous, m_interimResults, m_maxAlternatives);
I've gone through that approach as well, and in order to follow the same path as
rest of the params, we would have to add the audio track to
WebSpeechRecognitionParams. That looks a bit messy and out of place to me. I
think it's cleaner this way since the audio track is an extension to existing
behaviour and all other params are API related, while m_audioTrack is not.

However, I do agree, Glen should help us decide.

On 2014/08/15 10:22:56, xians1 wrote:
> I am wondering if we should just pass the m_audioTrack in the start method.
Lets
> get Glen's opinion on this.

[gshires1, 2014-09-02 16:41:22]

On 2014/08/15 11:29:41, burnik wrote:
>
https://codereview.chromium.org/448163003/diff/60001/Source/modules/speech/Sp...
> File Source/modules/speech/SpeechRecognition.cpp (right):
> 
>
https://codereview.chromium.org/448163003/diff/60001/Source/modules/speech/Sp...
> Source/modules/speech/SpeechRecognition.cpp:57: m_controller->start(this,
> m_grammars.get(), m_lang, m_continuous, m_interimResults, m_maxAlternatives);
> I've gone through that approach as well, and in order to follow the same path
as
> rest of the params, we would have to add the audio track to
> WebSpeechRecognitionParams. That looks a bit messy and out of place to me. I
> think it's cleaner this way since the audio track is an extension to existing
> behaviour and all other params are API related, while m_audioTrack is not.
> 
> However, I do agree, Glen should help us decide.
> 
> On 2014/08/15 10:22:56, xians1 wrote:
> > I am wondering if we should just pass the m_audioTrack in the start method.
> Lets
> > get Glen's opinion on this.

I don't have a strong opinion here. Keeping the code clean is the primary
concern, so setAudioTrack seems a good way to go.
(Main thing is to ensure that it doesn't change the JavaScript API ... which of
course it does not.)

[burnik, 2014-09-02 19:54:28]

This is the only change proposed for the API to support WebRTC processing:
https://codereview.chromium.org/448163003/patch/60001/70003


I repeat the special use case here:

<script>

getUserMedia( { audio:true } , function( mediaStream ) {

  var micAudioTrack = mediaStream.getAudioTracks()[0];

  var sr = new webkitSpeechRecognition();
  sr.audioTrack = micAudioTrack;
  sr.onerror = function(err) { console.error(err); }  
  sr.start(); // would raise onerror event for a WebAudio Track and fallback to
default behavior (starts the SR)

 }, function( error ) {
  console.error("Cannot access media",error);
});

</script>

Other than that, existing behavior is in place.

[burnik, 2014-09-15 08:48:13]

burnik@chromium.org changed reviewers:
+ haraken@chromium.org, jochen@chromium.org

[burnik, 2014-09-15 08:48:46]

On 2014/09/02 19:54:28, burnik wrote:
> This is the only change proposed for the API to support WebRTC processing:
> https://codereview.chromium.org/448163003/patch/60001/70003
> 
> 
> I repeat the special use case here:
> 
> <script>
> 
> getUserMedia( { audio:true } , function( mediaStream ) {
> 
>   var micAudioTrack = mediaStream.getAudioTracks()[0];
> 
>   var sr = new webkitSpeechRecognition();
>   sr.audioTrack = micAudioTrack;
>   sr.onerror = function(err) { console.error(err); }  
>   sr.start(); // would raise onerror event for a WebAudio Track and fallback
to
> default behavior (starts the SR)
> 
>  }, function( error ) {
>   console.error("Cannot access media",error);
> });
> 
> </script>
> 
> Other than that, existing behavior is in place.

Added 2 reviewers. Please take a look.

[burnik, 2014-09-15 11:01:59]

Remove attach/detach - using overloaded start

[no longer working on chromium, 2014-09-17 09:29:26]

nice. lgtm

[no longer working on chromium, 2014-09-17 09:30:27]

Jochen, could you please review the details of this blink code change?

Thanks,
SX

[Mike West, 2014-09-17 10:20:26]

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org

[Mike West, 2014-09-17 10:20:27]

A few drive-by comments. Hope they're helpful.

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognition.h (right):

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.h:31: #include
"modules/mediastream/MediaStreamTrack.h"
I think you can forward-declare this here, and only pull the header in  when you
access MediaStreamTrack methods in SpeechRecognitionClient.cpp.

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.h:53: // Attributes.
Nit: Can you change this to note that these are publicly exposed via the IDL
file? Perhaps '// SpeechRecognition.idl implementation:'.

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognitionController.h:41: void
start(SpeechRecognition* recognition, const SpeechGrammarList* grammars, const
String& lang, bool continuous, bool interimResults, unsigned long
maxAlternatives, MediaStreamTrack* audioTrack)
Hrm. The various '::start()' methods are getting a bit unwieldy. Perhaps it
would make sense to create an initialization object (struct) that you could pass
in here, rather than relying on memory and the compiler's type checking to
ensure that everything is passed in and piped through in the right order?

https://codereview.chromium.org/448163003/diff/100001/Source/web/SpeechRecogn...
File Source/web/SpeechRecognitionClientProxy.cpp (right):

https://codereview.chromium.org/448163003/diff/100001/Source/web/SpeechRecogn...
Source/web/SpeechRecognitionClientProxy.cpp:64:
m_recognizer->start(WebSpeechRecognitionHandle(recognition), params,
audioTrack->component(), this);
Could the new MediaStreamComponent argument instead be added as a property on
the WebSpeechRecognitionParams object? That would avoid the requirement to
overload '::start()' here.

[burnik, 2014-09-17 11:17:21]

Thanks for observations, Mike! 

Could you also look at the previous patchset and tell me if that feels more
proper to you? Because, we have somewhat split opinions on how the change is to
be made.

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognition.h (right):

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.h:31: #include
"modules/mediastream/MediaStreamTrack.h"
Yes, actually in SpeechRecognitionClientProxy.cpp where I first access the
members.
Done.
On 2014/09/17 10:20:27, Mike West wrote:
> I think you can forward-declare this here, and only pull the header in  when
you
> access MediaStreamTrack methods in SpeechRecognitionClient.cpp.

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.h:53: // Attributes.
Not a bad idea. Although this affects other parts as well - not just attributes.
Added a comment above this line.
On 2014/09/17 10:20:27, Mike West wrote:
> Nit: Can you change this to note that these are publicly exposed via the IDL
> file? Perhaps '// SpeechRecognition.idl implementation:'.

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognitionController.h:41: void
start(SpeechRecognition* recognition, const SpeechGrammarList* grammars, const
String& lang, bool continuous, bool interimResults, unsigned long
maxAlternatives, MediaStreamTrack* audioTrack)
Could you take a look at the previous patchset? I've added attach/detach which
at least partially avoids this.

Controller and Client just pass thru data. No real logic until reaching to the
public web interface.

On 2014/09/17 10:20:27, Mike West wrote:
> Hrm. The various '::start()' methods are getting a bit unwieldy. Perhaps it
> would make sense to create an initialization object (struct) that you could
pass
> in here, rather than relying on memory and the compiler's type checking to
> ensure that everything is passed in and piped through in the right order?

https://codereview.chromium.org/448163003/diff/100001/Source/web/SpeechRecogn...
File Source/web/SpeechRecognitionClientProxy.cpp (right):

https://codereview.chromium.org/448163003/diff/100001/Source/web/SpeechRecogn...
Source/web/SpeechRecognitionClientProxy.cpp:64:
m_recognizer->start(WebSpeechRecognitionHandle(recognition), params,
audioTrack->component(), this);
Good point as well. Although that would require more changes to align with those
params. Also, notice that most of those params are actually related to the
remote API and have nothing to do with audio.

On 2014/09/17 10:20:27, Mike West wrote:
> Could the new MediaStreamComponent argument instead be added as a property on
> the WebSpeechRecognitionParams object? That would avoid the requirement to
> overload '::start()' here.

[Mike West, 2014-09-17 11:30:47]

On 2014/09/17 11:17:21, burnik wrote:
> Could you also look at the previous patchset and tell me if that feels more
> proper to you? Because, we have somewhat split opinions on how the change is
to
> be made.

I like the overloaded ::start approach more than attach/detach. I suspect that
you could avoid the overloading by extending the params object, as I suggest in
one of the comments below, but even with overloading, it feels less complex for
the callsite to kick everything off with a single method call.

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/448163003/diff/100001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognitionController.h:41: void
start(SpeechRecognition* recognition, const SpeechGrammarList* grammars, const
String& lang, bool continuous, bool interimResults, unsigned long
maxAlternatives, MediaStreamTrack* audioTrack)
On 2014/09/17 11:17:20, burnik wrote:
> Could you take a look at the previous patchset? I've added attach/detach which
> at least partially avoids this.

I like this approach more than the separate attach/detach steps. This seems like
the right direction to go, the argument list just seems to be getting pretty
long. I certainly won't insist on refactoring all the calls to ::start in this
CL, but I do think it's worth moving to a one-parameter model in the future to
keep things simple.

> Controller and Client just pass thru data. No real logic until reaching to the
> public web interface.

Yup. My claim is simply that bringing those parameters out into a separate
object a) makes it clear exactly what bits and pieces are required, and b) makes
it simpler to understand what needs to be passed around in order to kick off
recognition.

https://codereview.chromium.org/448163003/diff/100001/Source/web/SpeechRecogn...
File Source/web/SpeechRecognitionClientProxy.cpp (right):

https://codereview.chromium.org/448163003/diff/100001/Source/web/SpeechRecogn...
Source/web/SpeechRecognitionClientProxy.cpp:64:
m_recognizer->start(WebSpeechRecognitionHandle(recognition), params,
audioTrack->component(), this);
On 2014/09/17 11:17:20, burnik wrote:
> Good point as well. Although that would require more changes to align with
those
> params.

Sure. But I think it reduces the complexity of the system by ensuring that
there's one point where you make changes to the parameters necessary to
initialize the recognizer, rather than breaking the flow up into multiple
methods.

> Also, notice that most of those params are actually related to the
> remote API and have nothing to do with audio.

Well, it's called WebSpeechRecognitionParams, which suggests that it contains
bits and pieces which are necessary to do speech recognition. Adding an audio
track to that object doesn't seem like it's much of a stretch.

I don't think this sort of thing is a requirement, but it would be nice to keep
the public API simpler. *shrug*

[no longer working on chromium, 2014-09-17 11:38:20]

On 2014/09/17 11:30:47, Mike West wrote:
> On 2014/09/17 11:17:21, burnik wrote:
> > Could you also look at the previous patchset and tell me if that feels more
> > proper to you? Because, we have somewhat split opinions on how the change is
> to
> > be made.
> 
> I like the overloaded ::start approach more than attach/detach. I suspect that
> you could avoid the overloading by extending the params object, as I suggest
in
> one of the comments below, but even with overloading, it feels less complex
for
> the callsite to kick everything off with a single method call.
> 

+1, wrapping those parameters into a struct and avoid the overloaded start
method sounds a good idea to me.
When no audio track is set, you can create a dummy WebMediaStreamTrack variable
and pass it through the struct. In Chromium code, it tracks
WebMediaStreamTrack::isNull() to determine the use cases.

[burnik, 2014-09-17 13:47:54]

Ok. Wrapped into WebSpeechRecognition params and removed overloads. Please take
a look now.

https://codereview.chromium.org/448163003/diff/120001/Source/web/SpeechRecogn...
File Source/web/SpeechRecognitionClientProxy.h (right):

https://codereview.chromium.org/448163003/diff/120001/Source/web/SpeechRecogn...
Source/web/SpeechRecognitionClientProxy.h:38: class WebMediaStreamTrack;
This is removed for the next round.

[Mike West, 2014-09-17 14:32:02]

LGTM, thanks for taking another pass.

I'd recommend doing the same struct packing for
SpeechRecognitionClientProxy::start, et al, in a future CL, but this seems like
a good approach for this particular feature.

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognition.h (right):

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.h:54: // Attributes.
Nit: Fold the comments into one line

[Mike West, 2014-09-17 14:32:21]

(You'll still need a content OWNER to stamp the CL, of course)

[burnik, 2014-09-17 15:50:48]

burnik@chromium.org changed reviewers:
+ hans@chromium.org
- gshires@chromium.org

[burnik, 2014-09-17 15:54:02]

+hans as reviewer.

[hans, 2014-09-17 16:07:58]

lgtm, and I like this new functionality!

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognitionController.h:30: #include
"modules/mediastream/MediaStreamTrack.h"
nit: Would a forward declaration of MediaStreamTrack be sufficient here?

[jochen (gone - plz use gerrit), 2014-09-18 18:22:59]

who's maintaining the speech module btw? We should have somebody in the owner
file there

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognition.idl (right):

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.idl:38: attribute MediaStreamTrack
audioTrack;
should that be prefixed with [RuntimeEnabled=MediaStream] ?

[jochen (gone - plz use gerrit), 2014-09-18 18:23:56]

i'm also not sure whether we should add new features to a prefixed api just like
this.

can you please send an intent to implement to blink-dev?

[no longer working on chromium, 2014-09-19 07:36:17]

On 2014/09/18 18:22:59, jochen wrote:
> who's maintaining the speech module btw? We should have somebody in the owner
> file there
> 

Hi Jochen, WebRtc in Stockholm is maintaining the speech code. tommyw@ was the
speech guy for both blink and chromium code, but he is on sick leave now.


>
https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
> File Source/modules/speech/SpeechRecognition.idl (right):
> 
>
https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
> Source/modules/speech/SpeechRecognition.idl:38: attribute MediaStreamTrack
> audioTrack;
> should that be prefixed with [RuntimeEnabled=MediaStream] ?

[burnik, 2014-09-22 07:16:19]

On 2014/09/18 18:23:56, jochen (slow for reviews) wrote:
> i'm also not sure whether we should add new features to a prefixed api just
like
> this.
> 
> can you please send an intent to implement to blink-dev?

Sent intent to implement:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/G2lRTzZXmW4

[haraken, 2014-09-22 08:05:58]

LGTM in terms of oilpan.

[burnik, 2014-09-22 08:35:24]

Patchset added.

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognition.idl (right):

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.idl:38: attribute MediaStreamTrack
audioTrack;
On 2014/09/18 18:22:58, jochen (slow for reviews) wrote:
> should that be prefixed with [RuntimeEnabled=MediaStream] ?

I believe so. Adding it.

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/448163003/diff/120001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognitionController.h:30: #include
"modules/mediastream/MediaStreamTrack.h"
On 2014/09/17 16:07:58, hans wrote:
> nit: Would a forward declaration of MediaStreamTrack be sufficient here?

Yes. Added fwd declare.
Header is moved to SpeechRecognition.cpp (required by the Trace method ).

[burnik, 2014-09-22 08:37:33]

Patchset #8 (id:140001) has been deleted

[burnik, 2014-09-22 14:28:15]

Include WebMediaStreamTrack to enable build.

[burnik, 2014-09-22 14:29:46]

Temporarily added include of WebMediaStreamTrack in WebSpeechRecognitionParams
to be able to land without depending on the chromium code.

https://codereview.chromium.org/448163003/diff/160001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognition.cpp (right):

https://codereview.chromium.org/448163003/diff/160001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognition.cpp:34: #include
"modules/mediastream/MediaStreamTrack.h"
Required by |Trace(...)|

[jochen (gone - plz use gerrit), 2014-09-26 19:43:44]

in general, cross modules dependencies are not desirable.

however, webrtc should probably move to core eventually, so this lgtm

[burnik, 2014-09-29 12:15:52]

s/WebRTC/MediaStream/

[burnik, 2014-09-29 14:02:38]

The CQ bit was checked by burnik@chromium.org

[commit-bot: I haz the power, 2014-09-29 14:03:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/448163003/220001

[commit-bot: I haz the power, 2014-09-29 17:00:34]

Committed patchset #11 (id:220001) as 182851