MediaSourcePlayer: Only StopInterpolating() when it's interpolating.

MediaSourcePlayer: Only StopInterpolating() when it's interpolating.

BUG=417399
Committed: https://crrev.com/35a199f199f54cdc74e54f905198a0324da9e477
Cr-Commit-Position: refs/heads/master@{#296844}

[xhwang, 2014-09-25 00:01:32]

xhwang@chromium.org changed reviewers:
+ qinmin@chromium.org

[xhwang, 2014-09-25 00:01:39]

PTAL

[xhwang, 2014-09-25 00:02:40]

https://codereview.chromium.org/601783004/diff/1/media/base/android/media_sou...
File media/base/android/media_source_player.cc (right):

https://codereview.chromium.org/601783004/diff/1/media/base/android/media_sou...
media/base/android/media_source_player.cc:580:
interpolator_.StopInterpolating();
About the bug: PlaybackCompleted() are called twice, one on audio and one on
video, so we end up calling this twice, which is not allowed.

[qinmin, 2014-09-25 00:19:11]

https://codereview.chromium.org/601783004/diff/1/media/base/android/media_sou...
File media/base/android/media_source_player.cc (right):

https://codereview.chromium.org/601783004/diff/1/media/base/android/media_sou...
media/base/android/media_source_player.cc:580:
interpolator_.StopInterpolating();
Maybe we should move this if logic to 
MediaSourcePlayer::MediaDecoderCallback():

if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM)
  PlaybackCompleted(is_audio);
  if (is_clock_manager)
     interpolator_.StopInterpolating();
}

That makes sure only the clock manager will mess with the interpolator_
On 2014/09/25 00:02:40, xhwang wrote:
> About the bug: PlaybackCompleted() are called twice, one on audio and one on
> video, so we end up calling this twice, which is not allowed.

[xhwang, 2014-09-25 19:42:15]

Thanks for the suggestions. PTAL again.

https://codereview.chromium.org/601783004/diff/1/media/base/android/media_sou...
File media/base/android/media_source_player.cc (right):

https://codereview.chromium.org/601783004/diff/1/media/base/android/media_sou...
media/base/android/media_source_player.cc:580:
interpolator_.StopInterpolating();
On 2014/09/25 00:19:11, qinmin wrote:
> Maybe we should move this if logic to 
> MediaSourcePlayer::MediaDecoderCallback():
> 
> if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM)
>   PlaybackCompleted(is_audio);
>   if (is_clock_manager)
>      interpolator_.StopInterpolating();
> }
> 
> That makes sure only the clock manager will mess with the interpolator_
> On 2014/09/25 00:02:40, xhwang wrote:
> > About the bug: PlaybackCompleted() are called twice, one on audio and one on
> > video, so we end up calling this twice, which is not allowed.
> 

Done.

[qinmin, 2014-09-25 20:23:17]

lgtm

https://codereview.chromium.org/601783004/diff/20001/media/base/android/media...
File media/base/android/media_source_player.cc (right):

https://codereview.chromium.org/601783004/diff/20001/media/base/android/media...
media/base/android/media_source_player.cc:486: if (is_clock_manager &&
interpolator_.interpolating())
we probably don't need interpolator_.interpolating() as it should always be true
when reaching here.
If it is not true, then there is definitely some bugs.

[xhwang, 2014-09-25 23:32:46]

comments addressed

[xhwang, 2014-09-25 23:33:12]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-09-25 23:34:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/601783004/40001

[commit-bot: I haz the power, 2014-09-26 00:52:17]

Committed patchset #3 (id:40001) as e2aff7ae3c24f896e2c3fd5160fff114e7fccb82

[commit-bot: I haz the power, 2014-09-26 00:52:58]

Patchset 3 (id:??) landed as
https://crrev.com/35a199f199f54cdc74e54f905198a0324da9e477
Cr-Commit-Position: refs/heads/master@{#296844}