Make media::AudioRenderer inherit from media::TimeSource.

Make media::AudioRenderer return a media::TimeSource.

Mostly mechanical change to finish migrating time-related
methods from AudioRenderer to TimeSource.

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

[scherkus (not reviewing), 2014-07-21 17:07:53]

looking for some preliminary feedback on how to introduce TimeSource to
AudioRenderer and how it ties into your plans

https://codereview.chromium.org/403723006/diff/1/media/base/audio_renderer.h
File media/base/audio_renderer.h (right):

https://codereview.chromium.org/403723006/diff/1/media/base/audio_renderer.h#...
media/base/audio_renderer.h:18: class MEDIA_EXPORT AudioRenderer : public
TimeSource {
alternatively, we can have this interface have a getter so that we don't force
inheritance

I feel this also might make for an easier transition to Renderer

virtual TimeSource* GetTimeSource() = 0;

[xhwang, 2014-07-21 22:10:06]

lgtm

https://codereview.chromium.org/403723006/diff/1/media/base/audio_renderer.h
File media/base/audio_renderer.h (right):

https://codereview.chromium.org/403723006/diff/1/media/base/audio_renderer.h#...
media/base/audio_renderer.h:18: class MEDIA_EXPORT AudioRenderer : public
TimeSource {
On 2014/07/21 17:07:53, scherkus wrote:
> alternatively, we can have this interface have a getter so that we don't force
> inheritance
> 
> I feel this also might make for an easier transition to Renderer
> 
> virtual TimeSource* GetTimeSource() = 0;

The Renderer will do pretty much the same as the AudioRenderer does here: either
inherit TimeSource or provide a getter. In that sense, using a getter does seem
simpler. On the desktop case, you can just do something like 

TimeSource* Renderer::GetTimeSource() {
  if (audio_renderer_)
    return audio_renderer_->GetTimeSource();
  ...
}

https://codereview.chromium.org/403723006/diff/1/media/filters/audio_renderer...
File media/filters/audio_renderer_impl.h (right):

https://codereview.chromium.org/403723006/diff/1/media/filters/audio_renderer...
media/filters/audio_renderer_impl.h:69: // TimeSource implementation.
Since AudioRenderer inherits TimeSource, this is not necessarily true. Strictly
speaking this is included in AudioRenderer impl on l.77

[scherkus (not reviewing), 2014-07-22 00:07:50]

can you PTAL?

I decided to go the composition route. For example, I think it'll be possible to
split AudioRendererImpl into an AudioTimeSource component.

[xhwang, 2014-07-22 00:29:28]

lgtm++

https://codereview.chromium.org/403723006/diff/80001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/403723006/diff/80001/media/base/pipeline.cc#n...
media/base/pipeline.cc:410: if (time_source_)
Once we use WallClockTimeSource, we should be able to drop this condition check?

https://codereview.chromium.org/403723006/diff/80001/media/base/pipeline.cc#n...
media/base/pipeline.cc:902: time_source_->StopTicking();
This is a bit odd here. Maybe time_source_ should be renamed to
audio_time_source_. But it's probably okay if you unify the
interpolator_/time_source_ code in the next CL.

[scherkus (not reviewing), 2014-07-22 00:30:59]

thanks for the review!

https://codereview.chromium.org/403723006/diff/80001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/403723006/diff/80001/media/base/pipeline.cc#n...
media/base/pipeline.cc:410: if (time_source_)
On 2014/07/22 00:29:28, xhwang wrote:
> Once we use WallClockTimeSource, we should be able to drop this condition
check?

correct! all condition checking for time_source_ will go away

https://codereview.chromium.org/403723006/diff/80001/media/base/pipeline.cc#n...
media/base/pipeline.cc:902: time_source_->StopTicking();
On 2014/07/22 00:29:28, xhwang wrote:
> This is a bit odd here. Maybe time_source_ should be renamed to
> audio_time_source_. But it's probably okay if you unify the
> interpolator_/time_source_ code in the next CL.

yeah it'll get cleaned up

[scherkus (not reviewing), 2014-07-22 01:11:05]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-07-22 01:12:22]

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

[commit-bot: I haz the power, 2014-07-22 02:30:12]

Change committed as 284598