Improve media::AudioRendererImplTest correctness via helper types.

Improve media::AudioRendererImplTest correctness via helper types.

While writing some new tests I ran into some hard to track down failures
that were a result of:
  1) Using AudioBufferConverter due to different input/output rates
  2) Using "int" for all input/output frame counts

For example, DeliverRemainingAudio() would actually over-deliver since
it didn't take the ratio of input/output rates into account.

To help make things better, use the type system to differentiate between
input and output frame counts. In addition, use simpler input/output
rates to make it easier to calculate expected time and frame values.

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

[scherkus (not reviewing), 2014-07-19 02:21:03]

WDYT?

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl_unittest.cc (left):

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl_unittest.cc:288:
SatisfyPendingRead(frames_remaining_in_buffer());
this was incorrect and would lead to situations where by over-delivering we
wouldn't get a pending read when we expected to

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl_unittest.cc (right):

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl_unittest.cc:150: next_timestamp_.reset(new
AudioTimestampHelper(kInputSamplesPerSecond));
this also looked like a bug but I don't think it mattered since everything gets
re-timestamped when converted

[DaleCurtis, 2014-07-21 16:29:29]

I like it, especially if it's already exposing bugs.

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl_unittest.cc (left):

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl_unittest.cc:288:
SatisfyPendingRead(frames_remaining_in_buffer());
On 2014/07/19 02:21:03, scherkus wrote:
> this was incorrect and would lead to situations where by over-delivering we
> wouldn't get a pending read when we expected to

Can you elaborate? It looks like before it was delivering exactly N frames but
now you're delivering ceil(N / 256) * 256 frames.

[DaleCurtis, 2014-07-21 16:33:18]

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl_unittest.cc (left):

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl_unittest.cc:288:
SatisfyPendingRead(frames_remaining_in_buffer());
On 2014/07/21 16:29:29, DaleCurtis wrote:
> On 2014/07/19 02:21:03, scherkus wrote:
> > this was incorrect and would lead to situations where by over-delivering we
> > wouldn't get a pending read when we expected to
> 
> Can you elaborate? It looks like before it was delivering exactly N frames but
> now you're delivering ceil(N / 256) * 256 frames.

Derp and I confused input / output frames.  The N was wrong before, but if you
want it to be closer to being exact instead of over delivering it should be N *
input_rate/output_rate.

[scherkus (not reviewing), 2014-07-21 19:20:21]

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl_unittest.cc (left):

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl_unittest.cc:288:
SatisfyPendingRead(frames_remaining_in_buffer());
On 2014/07/21 16:33:18, DaleCurtis wrote:
> On 2014/07/21 16:29:29, DaleCurtis wrote:
> > On 2014/07/19 02:21:03, scherkus wrote:
> > > this was incorrect and would lead to situations where by over-delivering
we
> > > wouldn't get a pending read when we expected to
> > 
> > Can you elaborate? It looks like before it was delivering exactly N frames
but
> > now you're delivering ceil(N / 256) * 256 frames.
> 
> Derp and I confused input / output frames.  The N was wrong before, but if you
> want it to be closer to being exact instead of over delivering it should be N
*
> input_rate/output_rate.

It looks like we can't use that as AudioBufferConverter keeps some frames around
in its internal buffer.

frames_remaining_in_buffer() = 8192
Deliver a 4096 frame buffer @ 5000 Hz
AudioBufferConverter spits out a 7936 buffer @ 10000 Hz
AudioBufferConverter reports 0 input frames left
AudioBufferConverter reports 128 *buffered* input frames left

Is that expected?

[DaleCurtis, 2014-07-21 19:24:57]

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl_unittest.cc (left):

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl_unittest.cc:288:
SatisfyPendingRead(frames_remaining_in_buffer());
On 2014/07/21 19:20:21, scherkus wrote:
> On 2014/07/21 16:33:18, DaleCurtis wrote:
> > On 2014/07/21 16:29:29, DaleCurtis wrote:
> > > On 2014/07/19 02:21:03, scherkus wrote:
> > > > this was incorrect and would lead to situations where by over-delivering
> we
> > > > wouldn't get a pending read when we expected to
> > > 
> > > Can you elaborate? It looks like before it was delivering exactly N frames
> but
> > > now you're delivering ceil(N / 256) * 256 frames.
> > 
> > Derp and I confused input / output frames.  The N was wrong before, but if
you
> > want it to be closer to being exact instead of over delivering it should be
N
> *
> > input_rate/output_rate.
> 
> It looks like we can't use that as AudioBufferConverter keeps some frames
around
> in its internal buffer.
> 
> frames_remaining_in_buffer() = 8192
> Deliver a 4096 frame buffer @ 5000 Hz
> AudioBufferConverter spits out a 7936 buffer @ 10000 Hz
> AudioBufferConverter reports 0 input frames left
> AudioBufferConverter reports 128 *buffered* input frames left
> 
> Is that expected?

Ah, yeah, you can't get those frames out without a Flush().  It's expected, but
looking through the code I wonder if Flush() is called prior to stopping.  It's
possible we're losing a small number of frames off the back side when ABC is
used.

[scherkus (not reviewing), 2014-07-21 19:30:05]

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl_unittest.cc (left):

https://codereview.chromium.org/403003004/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl_unittest.cc:288:
SatisfyPendingRead(frames_remaining_in_buffer());
On 2014/07/21 19:24:56, DaleCurtis wrote:
> On 2014/07/21 19:20:21, scherkus wrote:
> > On 2014/07/21 16:33:18, DaleCurtis wrote:
> > > On 2014/07/21 16:29:29, DaleCurtis wrote:
> > > > On 2014/07/19 02:21:03, scherkus wrote:
> > > > > this was incorrect and would lead to situations where by
over-delivering
> > we
> > > > > wouldn't get a pending read when we expected to
> > > > 
> > > > Can you elaborate? It looks like before it was delivering exactly N
frames
> > but
> > > > now you're delivering ceil(N / 256) * 256 frames.
> > > 
> > > Derp and I confused input / output frames.  The N was wrong before, but if
> you
> > > want it to be closer to being exact instead of over delivering it should
be
> N
> > *
> > > input_rate/output_rate.
> > 
> > It looks like we can't use that as AudioBufferConverter keeps some frames
> around
> > in its internal buffer.
> > 
> > frames_remaining_in_buffer() = 8192
> > Deliver a 4096 frame buffer @ 5000 Hz
> > AudioBufferConverter spits out a 7936 buffer @ 10000 Hz
> > AudioBufferConverter reports 0 input frames left
> > AudioBufferConverter reports 128 *buffered* input frames left
> > 
> > Is that expected?
> 
> Ah, yeah, you can't get those frames out without a Flush().  It's expected,
but
> looking through the code I wonder if Flush() is called prior to stopping. 
It's
> possible we're losing a small number of frames off the back side when ABC is
> used.

I see.

If we want to keep using the ABC code path for these tests, do you have any
other suggestions/feedback for DeliverRemainingAudio() and the rest of this CL?

[DaleCurtis, 2014-07-21 19:34:01]

This is fine,  lgtm.

[scherkus (not reviewing), 2014-07-21 19:36:02]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-07-21 19:39:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/scherkus@chromium.org/403003004/1

[commit-bot: I haz the power, 2014-07-21 22:56:19]

Change committed as 284529