Add audio playback (all platforms) to cast_receiver_app.

Add audio playback (all platforms) to cast_receiver_app.

This change not only adds audio playback, but also adds an
implementation that delays rendering of video/audio frames to the
desired playout_times.  In addition, console logging has been added to
expose: 1) Whether frames were skipped because playback was running
behind; 2) Whether frames were skipped by the receiver because packets
were too late; 3) When frames are actually being played versus the
desired playout_time.

BUG=360776
R=imcheng@chromium.org

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

[miu, 2014-04-08 22:19:50]

imcheng: PTAL at everything.

hclam: OWNERS stamp.

[imcheng, 2014-04-09 01:34:45]

Looks good, mostly just comments with memory management and questions for my own
curiousity.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc
File media/cast/test/receiver.cc (right):

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:178: max_frame_age_(base::TimeDelta::FromSeconds(1)
* 3 /
Could you add a comment here about how you arrived at this number?

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:194: InProcessReceiver::Start();
Do we need to wait for StartAudioOutputOnAudioManagerThread to finish before
calling this, similar to Stop() below?

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:200:
DCHECK(!AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread());
Do we also need this DCHECK in Start()?

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:212: while (!audio_playout_queue_.empty()) {
If linked_ptr is used, then you won't have do this manually.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:223:
audio_output_stream_.reset(AudioManager::Get()->MakeAudioOutputStreamProxy(
It looks like you will need to call Stop() and Close() before resetting, just in
case this function is called while the stream is running.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:415: audio_playout_queue_.pop_front();
Do you need to destroy audio_playout_queue_.front().second in cases where you
are just throwing away the frames? Would it work beter if it uses linked_ptrs?
(see below)

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:439: typedef std::pair<base::TimeTicks, AudioBus*>
AudioQueueEntry;
Can you make this into <base::TimeTicks, linked_ptr<AudioBus> > ?

https://codereview.chromium.org/229463002/diff/1/media/cast/test/utility/in_p...
File media/cast/test/utility/in_process_receiver.h (right):

https://codereview.chromium.org/229463002/diff/1/media/cast/test/utility/in_p...
media/cast/test/utility/in_process_receiver.h:68: void DestroySoon();
Can you remove this function, and change comment above re: destruction? Seems
like there are no more callers of this according to code search.

[miu, 2014-04-09 04:07:19]

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc
File media/cast/test/receiver.cc (right):

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:178: max_frame_age_(base::TimeDelta::FromSeconds(1)
* 3 /
On 2014/04/09 01:34:45, imcheng1 wrote:
> Could you add a comment here about how you arrived at this number?

Done.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:194: InProcessReceiver::Start();
On 2014/04/09 01:34:45, imcheng1 wrote:
> Do we need to wait for StartAudioOutputOnAudioManagerThread to finish before
> calling this, similar to Stop() below?

Added comment to explain.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:200:
DCHECK(!AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread());
On 2014/04/09 01:34:45, imcheng1 wrote:
> Do we also need this DCHECK in Start()?

No, it doesn't matter there since the thread is not explicitly blocked on
anything.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:223:
audio_output_stream_.reset(AudioManager::Get()->MakeAudioOutputStreamProxy(
On 2014/04/09 01:34:45, imcheng1 wrote:
> It looks like you will need to call Stop() and Close() before resetting, just
in
> case this function is called while the stream is running.

Instead, I added a DCHECK(!audio_output_stream_) since it should be invalid to
call Start() more than once.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:415: audio_playout_queue_.pop_front();
On 2014/04/09 01:34:45, imcheng1 wrote:
> Do you need to destroy audio_playout_queue_.front().second in cases where you
> are just throwing away the frames? Would it work beter if it uses linked_ptrs?
> (see below)

Good catch.  Fixed this memory-leak.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/receiver.cc#...
media/cast/test/receiver.cc:439: typedef std::pair<base::TimeTicks, AudioBus*>
AudioQueueEntry;
On 2014/04/09 01:34:45, imcheng1 wrote:
> Can you make this into <base::TimeTicks, linked_ptr<AudioBus> > ?

Unfortunately, no.  AudioBus defines a private destructor that can only be
invoked by scoped_ptr<>.

https://codereview.chromium.org/229463002/diff/1/media/cast/test/utility/in_p...
File media/cast/test/utility/in_process_receiver.h (right):

https://codereview.chromium.org/229463002/diff/1/media/cast/test/utility/in_p...
media/cast/test/utility/in_process_receiver.h:68: void DestroySoon();
On 2014/04/09 01:34:45, imcheng1 wrote:
> Can you remove this function, and change comment above re: destruction? Seems
> like there are no more callers of this according to code search.

Done.

[imcheng, 2014-04-09 19:30:25]

lgtm

[miu, 2014-04-09 20:26:35]

Committed patchset #3 manually as r262796 (presubmit successful).

[miu, 2014-04-09 20:28:05]

I've dcommitted to unblock team members.  This shouldn't impact the Chromium
tree, as none of this code is executed in the browser.