Refactor audio renderer algorithms to use BufferQueue.

Refactor audio renderer algorithms to use BufferQueue. Also cleaned up some comments and unnecessary functions/fields.
BUG=16011
TEST=none

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

[kylep, 2009-07-09 17:27:15]

Alpha, I'd appreciate your comments on this change. Especially with regards to
filling up the queue again. Basically, the model has turned to ignoring buffers
and their boundaries, but that's how the data come packaged, so I coded up
something in AdvanceInput() to try to refill it, but it's ugly and I don't like
it. Do you have a better idea? Thanks, -Kyle

[kylep, 2009-07-09 20:36:08]

Alpha, my original question is no longer valid. Instead of trying to maintain a
number of buffers, I try to maintain a number of bytes and each time I advance
my pointer or enqueue a new buffer, I test to see if I now have enough. If I
don't, I request more.

[Alpha Left Google, 2009-07-09 23:02:18]

I just have some style nits.

http://codereview.chromium.org/155255/diff/37/1036
File media/filters/audio_renderer_algorithm_base.cc (right):

http://codereview.chromium.org/155255/diff/37/1036#newcode12
Line 12: const size_t kDefaultMaxQueueSizeInBytes = 65536;
Explain why this is a sane number.

http://codereview.chromium.org/155255/diff/37/1036#newcode37
Line 37: request_read_callback_->Run();
Please explain this line that says something like "do the initial read"

http://codereview.chromium.org/155255/diff/37/1036#newcode66
Line 66: if (bytes > queue_.SizeInBytes())
It looks strange to me that reading than what the buffer queue has is handled
here. I think this case is better handled by the queue itself.

http://codereview.chromium.org/155255/diff/37/1036#newcode76
Line 76: if (bytes > queue_.SizeInBytes())
Here too, if you want to read more than it has I think this should be handled by
the buffer queue itself so you don't need to check size here.

http://codereview.chromium.org/155255/diff/37/1038
File media/filters/audio_renderer_algorithm_base.h (right):

http://codereview.chromium.org/155255/diff/37/1038#newcode74
Line 74: // Advances |queue_|'s internal pointer |bytes| bytes.
can just say "by |bytes|."

I also think it is better named AdvanceInputPosition

http://codereview.chromium.org/155255/diff/37/1038#newcode77
Line 77: // Tries to copy |bytes| bytes from our data to |dest|. Returns the
number
copy |bytes| from |queue_|

What about CopyFromInput?

[kylep, 2009-07-10 00:30:29]

Take another look?

http://codereview.chromium.org/155255/diff/37/1036
File media/filters/audio_renderer_algorithm_base.cc (right):

http://codereview.chromium.org/155255/diff/37/1036#newcode12
Line 12: const size_t kDefaultMaxQueueSizeInBytes = 65536;
On 2009/07/09 23:02:18, Alpha wrote:
> Explain why this is a sane number.

Legacy reasons. The old explanation no longer applied because speedup fills
fewer output buffers with 16 queued Buffers (of size 4Kb), but the performance
did not seem to suffer so I decided to maintain that number of bytes. Is my new
description sufficient?

http://codereview.chromium.org/155255/diff/37/1036#newcode37
Line 37: request_read_callback_->Run();
On 2009/07/09 23:02:18, Alpha wrote:
> Please explain this line that says something like "do the initial read"

Done.

http://codereview.chromium.org/155255/diff/37/1036#newcode66
Line 66: if (bytes > queue_.SizeInBytes())
On 2009/07/09 23:02:18, Alpha wrote:
> It looks strange to me that reading than what the buffer queue has is handled
> here. I think this case is better handled by the queue itself.

Logic moved down to subclasses of ARAB. BufferQueue was kept as "dumb" as
possible.

http://codereview.chromium.org/155255/diff/37/1036#newcode76
Line 76: if (bytes > queue_.SizeInBytes())
On 2009/07/09 23:02:18, Alpha wrote:
> Here too, if you want to read more than it has I think this should be handled
by
> the buffer queue itself so you don't need to check size here.

Logic moved down to subclasses of ARAB. BufferQueue was kept as "dumb" as
possible.

http://codereview.chromium.org/155255/diff/37/1038
File media/filters/audio_renderer_algorithm_base.h (right):

http://codereview.chromium.org/155255/diff/37/1038#newcode74
Line 74: // Advances |queue_|'s internal pointer |bytes| bytes.
On 2009/07/09 23:02:18, Alpha wrote:
> can just say "by |bytes|."
> 
> I also think it is better named AdvanceInputPosition

Done.

http://codereview.chromium.org/155255/diff/37/1038#newcode77
Line 77: // Tries to copy |bytes| bytes from our data to |dest|. Returns the
number
On 2009/07/09 23:02:18, Alpha wrote:
> copy |bytes| from |queue_|
> 
> What about CopyFromInput?

Done.

[Alpha Left Google, 2009-07-10 00:49:07]

ok. LGTM.