Stop DecryptingDemuxerStream in AudioRendererImpl::Stop().

Stop DecryptingDemuxerStream in AudioRendererImpl::Stop().

BUG=343669, 343617
TEST=none
R=ddorwin@chromium.org

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

[xhwang, 2014-02-14 03:12:35]

This CL is based on http://codereview.chromium.org/165733002.

PTAL

[ddorwin, 2014-02-14 04:30:51]

Does this also fix BUG 343617?
LGTM with comment and possible BUG= update.

https://codereview.chromium.org/163433004/diff/30001/media/filters/audio_rend...
File media/filters/audio_renderer_impl.cc (right):

https://codereview.chromium.org/163433004/diff/30001/media/filters/audio_rend...
media/filters/audio_renderer_impl.cc:226: if (decoder_) {
I wonder if it makes sense to just call StopDecoder() in all cases then have it
check decoder_ and always ensures stop_cb_ is called. It would be a chain of
steps that are always executed instead of a bunch of branches, which could get
even more complex if another one was added.

It seems it would make things easier to understand (vs. we must have a decoder_
above so we can call StopDecoder, which is going to call stop_cb_. Just a
thought.

[xhwang, 2014-02-14 18:48:05]

Comments addressed (including the comments in
https://codereview.chromium.org/165733002/). PTAL again.

https://codereview.chromium.org/163433004/diff/30001/media/filters/audio_rend...
File media/filters/audio_renderer_impl.cc (right):

https://codereview.chromium.org/163433004/diff/30001/media/filters/audio_rend...
media/filters/audio_renderer_impl.cc:226: if (decoder_) {
On 2014/02/14 04:30:51, ddorwin wrote:
> I wonder if it makes sense to just call StopDecoder() in all cases then have
it
> check decoder_ and always ensures stop_cb_ is called. It would be a chain of
> steps that are always executed instead of a bunch of branches, which could get
> even more complex if another one was added.
> 
> It seems it would make things easier to understand (vs. we must have a
decoder_
> above so we can call StopDecoder, which is going to call stop_cb_. Just a
> thought.

Done.

[ddorwin, 2014-02-14 19:49:30]

LGTM with suggestion. Thanks.

https://codereview.chromium.org/163433004/diff/110001/media/filters/audio_ren...
File media/filters/audio_renderer_impl.h (right):

https://codereview.chromium.org/163433004/diff/110001/media/filters/audio_ren...
media/filters/audio_renderer_impl.h:187: void StopDecoder();
Add something like:
// Stops the decoder if present. Ensures stop_cb_ is called.

[xhwang, 2014-02-14 21:14:01]

comments addressed

[xhwang, 2014-02-14 21:14:37]

https://codereview.chromium.org/163433004/diff/110001/media/filters/audio_ren...
File media/filters/audio_renderer_impl.h (right):

https://codereview.chromium.org/163433004/diff/110001/media/filters/audio_ren...
media/filters/audio_renderer_impl.h:187: void StopDecoder();
On 2014/02/14 19:49:30, ddorwin wrote:
> Add something like:
> // Stops the decoder if present. Ensures stop_cb_ is called.

Done.

[xhwang, 2014-02-14 21:14:42]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-02-14 21:20:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/163433004/210001

[commit-bot: I haz the power, 2014-02-14 22:25:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-14 22:25:49]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[xhwang, 2014-02-14 22:26:12]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-02-14 22:35:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/163433004/210001

[xhwang, 2014-02-15 01:10:12]

Committed patchset #5 manually as r251469 (presubmit successful).