Ensures that WASAPI audio output does not go silent in rare cases

Ensures that WASAPI audio output does not go silent in rare cases

BUG=170498
TEST=youtube, <audio> tests and content_unittests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181527

[tommi (sloooow) - chröme, 2013-02-08 15:03:07]

https://codereview.chromium.org/12220076/diff/1/media/audio/win/audio_low_lat...
File media/audio/win/audio_low_latency_output_win.cc (right):

https://codereview.chromium.org/12220076/diff/1/media/audio/win/audio_low_lat...
media/audio/win/audio_low_latency_output_win.cc:500: size_t num_packets =
(num_available_frames / packet_size_frames_);
should we also have a DCHECK here?
DCHECK_EQ(num_available_frames % packet_size_frames_, 0u);

https://codereview.chromium.org/12220076/diff/1/media/audio/win/audio_low_lat...
media/audio/win/audio_low_latency_output_win.cc:502: for (size_t n = 0; n <
num_packets; ++n) {
This function was big before bug now it's huge! :)
Can we pull this loop out into its own function?
I think that Run() could be made more readable by not having the whole
implementation for each case label inlined.

[tommi (sloooow) - chröme, 2013-02-08 15:11:43]

https://codereview.chromium.org/12220076/diff/2001/media/audio/win/audio_low_...
File media/audio/win/audio_low_latency_output_win.cc (right):

https://codereview.chromium.org/12220076/diff/2001/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:494: DLOG_IF(ERROR,
num_available_frames % packet_size_frames_ == 0)
!=

[henrika (OOO until Aug 14), 2013-02-08 15:25:12]

https://codereview.chromium.org/12220076/diff/1/media/audio/win/audio_low_lat...
File media/audio/win/audio_low_latency_output_win.cc (right):

https://codereview.chromium.org/12220076/diff/1/media/audio/win/audio_low_lat...
media/audio/win/audio_low_latency_output_win.cc:500: size_t num_packets =
(num_available_frames / packet_size_frames_);
See second patch. I'd like a log as well so we can track it.

https://codereview.chromium.org/12220076/diff/1/media/audio/win/audio_low_lat...
media/audio/win/audio_low_latency_output_win.cc:502: for (size_t n = 0; n <
num_packets; ++n) {
Stay tuned...

[tommi (sloooow) - chröme, 2013-02-08 15:37:54]

lgtm

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
File media/audio/win/audio_low_latency_output_win.cc (right):

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:478: TRACE_EVENT0("audio",
"WASAPIAudioOutputStream::Run");
change to WASAPIAudioOutputStream::RenderAudioFromSource?
(or even just "RenderAudioFromSource"?)

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:576: const int bytes_per_sample
= format_.Format.wBitsPerSample >> 3;
nit: move this variable down to where it's used.

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:580: num_filled_bytes =
frames_filled * format_.Format.nBlockAlign;
nit: declare num_filled_bytes here

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:601: flags);
no need to wrap?

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
File media/audio/win/audio_low_latency_output_win.h (right):

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.h:166: // Checks availiable amount
of space in the endpoint buffer and reads
s/availiable/available

[henrika (OOO until Aug 14), 2013-02-08 15:49:36]

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
File media/audio/win/audio_low_latency_output_win.cc (right):

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:478: TRACE_EVENT0("audio",
"WASAPIAudioOutputStream::Run");
On 2013/02/08 15:37:54, tommi wrote:
> change to WASAPIAudioOutputStream::RenderAudioFromSource?
> (or even just "RenderAudioFromSource"?)

Done.

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:576: const int bytes_per_sample
= format_.Format.wBitsPerSample >> 3;
On 2013/02/08 15:37:54, tommi wrote:
> nit: move this variable down to where it's used.

Done.

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:580: num_filled_bytes =
frames_filled * format_.Format.nBlockAlign;
On 2013/02/08 15:37:54, tommi wrote:
> nit: declare num_filled_bytes here

Done.

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.cc:601: flags);
On 2013/02/08 15:37:54, tommi wrote:
> no need to wrap?

Done.

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
File media/audio/win/audio_low_latency_output_win.h (right):

https://codereview.chromium.org/12220076/diff/4002/media/audio/win/audio_low_...
media/audio/win/audio_low_latency_output_win.h:166: // Checks availiable amount
of space in the endpoint buffer and reads
On 2013/02/08 15:37:54, tommi wrote:
> s/availiable/available

Done.

[commit-bot: I haz the power, 2013-02-08 16:14:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/henrika@chromium.org/12220076/5004

[commit-bot: I haz the power, 2013-02-08 16:53:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/henrika@chromium.org/12220076/5004

[commit-bot: I haz the power, 2013-02-08 18:28:46]

Change committed as 181527