Fix implicit configuration changes in MediaCodecAudioDecoder.

Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG=650735
TEST=new unittest

Committed: https://crrev.com/5769876ade1eadde031ff44094a30f31196e7cf1
Cr-Commit-Position: refs/heads/master@{#421318}

[DaleCurtis, 2016-09-26 20:12:51]

dalecurtis@chromium.org changed reviewers:
+ timav@chromium.org

[DaleCurtis, 2016-09-26 20:12:52]

Tima, do you remember the details around this code? Specifically why we didn't
pass channel count changes through?

[Tima Vaisburd, 2016-09-26 20:38:05]

On 2016/09/26 20:12:52, DaleCurtis wrote:
> Tima, do you remember the details around this code? Specifically why we didn't
> pass channel count changes through?

I do not remember myself... I found the original CL,
please take a look at the discussion in
https://codereview.chromium.org/1932093002.

I think in this case we'd need to test that it works, or the situation this code
tried to fix
(discrepancy from what audio pipeline sees and what MediaCodec sees) is not
relevant any more.

[DaleCurtis, 2016-09-26 21:46:37]

On 2016/09/26 at 20:38:05, timav wrote:
> On 2016/09/26 20:12:52, DaleCurtis wrote:
> > Tima, do you remember the details around this code? Specifically why we
didn't
> > pass channel count changes through?
> 
> I do not remember myself... I found the original CL,
> please take a look at the discussion in
https://codereview.chromium.org/1932093002.
> 
> I think in this case we'd need to test that it works, or the situation this
code tried to fix
> (discrepancy from what audio pipeline sees and what MediaCodec sees) is not
relevant any more.

Hmm, I'm confused about the original issue now. Passing buffer configuration
changes will cause a pipeline error for EME + SRC=, but for MSE + EME this
should work fine. I don't think anyone is relying on EME+SRC=, so that's
probably okay for now.

[AndyWu, 2016-09-26 23:22:18]

Description was changed from

==========
Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG=...
TEST=...
==========

to

==========
Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG=...
TEST=...
==========

[Tima Vaisburd, 2016-09-27 00:14:57]

lgtm

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
File media/filters/android/media_codec_audio_decoder.cc (right):

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
media/filters/android/media_codec_audio_decoder.cc:440: void
MediaCodecAudioDecoder::ResetTimestampState() {
nit: s/ResetTimestampState/GuessParamsFromConfig/ ?

[DaleCurtis, 2016-09-27 18:13:15]

Description was changed from

==========
Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG=...
TEST=...
==========

to

==========
Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG=650735
TEST=new unittest
==========

[DaleCurtis, 2016-09-27 18:13:17]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-09-27 18:13:21]

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
File media/filters/android/media_codec_audio_decoder.cc (right):

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
media/filters/android/media_codec_audio_decoder.cc:440: void
MediaCodecAudioDecoder::ResetTimestampState() {
On 2016/09/27 at 00:14:57, Tima Vaisburd wrote:
> nit: s/ResetTimestampState/GuessParamsFromConfig/ ?

Hmm, don't like GuessParamsFromConfig and can't think of anything better. We use
ResetTimestampState in other audio decoders so this seems fine. 

https://cs.chromium.org/search/?q=ResetTimestampState&type=cs&sq=package:chro...

[commit-bot: I haz the power, 2016-09-27 18:13:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 20:22:59]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-27 20:27:14]

Description was changed from

==========
Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG=650735
TEST=new unittest
==========

to

==========
Fix implicit configuration changes in MediaCodecAudioDecoder.

This allows channel and sample rate changes to pass through to the
renderer where they should be handled by AudioRendererImpl.

BUG=650735
TEST=new unittest

Committed: https://crrev.com/5769876ade1eadde031ff44094a30f31196e7cf1
Cr-Commit-Position: refs/heads/master@{#421318}
==========

[commit-bot: I haz the power, 2016-09-27 20:27:15]

Patchset 5 (id:??) landed as
https://crrev.com/5769876ade1eadde031ff44094a30f31196e7cf1
Cr-Commit-Position: refs/heads/master@{#421318}

[AndyWu, 2016-09-28 18:28:47]

tsunghung@chromium.org changed reviewers:
+ tsunghung@chromium.org

[AndyWu, 2016-09-28 18:28:48]

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
File media/filters/android/media_codec_audio_decoder.cc (right):

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
media/filters/android/media_codec_audio_decoder.cc:96: ResetTimestampState();
We should move this line before line 83. Sorry I didn't catch this issue sooner.
When I resume playback from middle of a episode on Hulu, The CL works fine.
However, if I play a new episode from beginning, it fails. I think the function
returns in line 88, so |channel_count_| is kept 0.

[AndyWu, 2016-09-28 19:15:59]

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
File media/filters/android/media_codec_audio_decoder.cc (right):

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
media/filters/android/media_codec_audio_decoder.cc:96: ResetTimestampState();
Sorry, this is not the root cause, just a workaround. The root cause is in line
414.

On 2016/09/28 18:28:48, AndyWu wrote:
> We should move this line before line 83. Sorry I didn't catch this issue
sooner.
> When I resume playback from middle of a episode on Hulu, The CL works fine.
> However, if I play a new episode from beginning, it fails. I think the
function
> returns in line 88, so |channel_count_| is kept 0.

https://codereview.chromium.org/2376513002/diff/80001/media/filters/android/m...
media/filters/android/media_codec_audio_decoder.cc:414:
timestamp_helper_->base_timestamp() == kNoTimestamp
We should check |timestamp_helper_| is null or not.