Stream speech audio to server as it gets recorded, instead of waiting until end of recording.

Stream speech audio to server as it gets recorded, instead of waiting until end of recording.

This CL depends on http://codereview.chromium.org/6292013/ which is in review.

BUG=61677
TEST=manual, invoke speech input and verify that recognition results appear as expected.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76993

[bulach, 2011-03-03 20:05:00]

hi satish,

I have some suggestions below, not sure if they are doable...

also, you'd probably need to change speech_recognition_request_unittest :) and
then while at it, it'd be nice to test the new functionality..

http://codereview.chromium.org/6615020/diff/1/content/browser/speech/speech_r...
File content/browser/speech/speech_recognizer.cc (right):

http://codereview.chromium.org/6615020/diff/1/content/browser/speech/speech_r...
content/browser/speech/speech_recognizer.cc:163: // recorded yet, we just send a
whitespace string.
hmm, I haven't followed the other CL this one depends, but wouldn't it be
simpler to have just a "FinishChunk" (or something similar)? worst case, it may
be simpler to move the "emptiness" test down to UploadAudioChunk rather than
here?

also, why is it necessary to send previoud_audio_chunk again?

if both we can't work around these limitations, please also clarify why repeat
the last chunk.

[Satish, 2011-03-04 12:26:49]

Thanks for the quick review. As we discussed offline, I have removed the caching
of previous audio chunk and instead sending a silence packet at the end. I have
also extended TestUrlFetcher so that tests can read back the chunks sent out and
updated the relevant unit tests to use the new code.

[bulach, 2011-03-04 12:41:08]

LGTM

thanks for the tests!

mostly nits and suggestions:

http://codereview.chromium.org/6615020/diff/7001/chrome/common/net/url_fetcher.h
File chrome/common/net/url_fetcher.h (right):

http://codereview.chromium.org/6615020/diff/7001/chrome/common/net/url_fetche...
chrome/common/net/url_fetcher.h:143: virtual void AppendChunkToUpload(const
std::string& data, bool is_last_chunk);
this file will be merged with the other change, right?

http://codereview.chromium.org/6292013/diff/42008/chrome/common/net/url_fetch...

http://codereview.chromium.org/6615020/diff/7001/content/browser/speech/speec...
File content/browser/speech/speech_recognizer.cc (right):

http://codereview.chromium.org/6615020/diff/7001/content/browser/speech/speec...
content/browser/speech/speech_recognizer.cc:147: // UploadAudioChunk requires a
non-empty buffer. So we encode a packet of
maybe: non-empty final buffer

http://codereview.chromium.org/6615020/diff/7001/content/browser/speech/speec...
content/browser/speech/speech_recognizer.cc:151: memset(samples.get(), 0,
sizeof(short) * num_samples);
would this work?

vector<short> sample((kAudioSampleRate * kAudioPacketIntervalMs) / 1000);
encoder_->Encode(sample.data(), sample.size());

http://codereview.chromium.org/6615020/diff/7001/content/browser/speech/speec...
content/browser/speech/speech_recognizer.cc:165:
request_->UploadAudioChunk(encoded_data, true);
nit: true /* final */

http://codereview.chromium.org/6615020/diff/7001/content/browser/speech/speec...
content/browser/speech/speech_recognizer.cc:233:
request_->UploadAudioChunk(encoded_data, false);
nit: false /* final */

http://codereview.chromium.org/6615020/diff/7001/content/browser/speech/speec...
File content/browser/speech/speech_recognizer_unittest.cc (right):

http://codereview.chromium.org/6615020/diff/7001/content/browser/speech/speec...
content/browser/speech/speech_recognizer_unittest.cc:147: for (int i = 0; i <
kNumChunks; ++i) {
size_t both here and the const above (I think 153 would probably throw a
warning).

[Satish, 2011-03-04 13:03:26]

Thanks, all comments addressed

http://codereview.chromium.org/6615020/diff/7001/chrome/common/net/url_fetcher.h
File chrome/common/net/url_fetcher.h (right):

http://codereview.chromium.org/6615020/diff/7001/chrome/common/net/url_fetche...
chrome/common/net/url_fetcher.h:143: virtual void AppendChunkToUpload(const
std::string& data, bool is_last_chunk);
On 2011/03/04 12:41:08, bulach wrote:
> this file will be merged with the other change, right?
> 
>
http://codereview.chromium.org/6292013/diff/42008/chrome/common/net/url_fetch...

Yes.