Throttle media decoding after excessive Android media server crashes

media/base/android/media_codec_player.cc

Index: media/base/android/media_codec_player.cc
diff --git a/media/base/android/media_codec_player.cc b/media/base/android/media_codec_player.cc
index 4e1e8ff4fb9a52269e0eb4037f2d5bba5938883d..2f0303a22aeaf3fd66c2e7a81a1bf37ee2c96316 100644
--- a/media/base/android/media_codec_player.cc
+++ b/media/base/android/media_codec_player.cc
@@ -49,12 +49,12 @@ scoped_refptr<base::SingleThreadTaskRunner> GetMediaTaskRunner() {
 MediaCodecPlayer::MediaCodecPlayer(
     int player_id,
     base::WeakPtr<MediaPlayerManager> manager,
-    const RequestMediaResourcesCB& request_media_resources_cb,
+    const OnPlayerReleasedCB& on_player_released_cb,
     scoped_ptr<DemuxerAndroid> demuxer,
     const GURL& frame_url)
     : MediaPlayerAndroid(player_id,
                          manager.get(),
-                         request_media_resources_cb,
+                         on_player_released_cb,
                          frame_url),
       ui_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       demuxer_(demuxer.Pass()),
@@ -67,8 +67,6 @@ MediaCodecPlayer::MediaCodecPlayer(
 
   DVLOG(1) << "MediaCodecPlayer::MediaCodecPlayer: player_id:" << player_id;
 
-  request_resources_cb_ = base::Bind(request_media_resources_cb_, player_id);
-
   completion_cb_ =
       base::Bind(&MediaPlayerManager::OnPlaybackComplete, manager, player_id);
   seek_done_cb_ =
@@ -299,6 +297,9 @@ void MediaCodecPlayer::SeekTo(base::TimeDelta timestamp) {
 }
 
 void MediaCodecPlayer::Release() {
+  if (ui_task_runner_->BelongsToCurrentThread())
+    on_player_released_cb_.Run(player_id());

[Tima Vaisburd, 2015/09/28 22:09:12]

Did you consider posting this callback to UI thread? Is it important to run this
cb before the body of the Release()?

[Ted C, 2015/09/28 22:42:30]

And should we be doing something in the else?

[Tima Vaisburd, 2015/09/28 23:04:47]

In the 'else' we continue :-)

Each method with RUN_ON_MEDIA_THREAD is called twice. First on UI thread, on
which the macro re-posts the very same method onto the Media thread, and then on
the Media thread.

In this patch we report that the player is released on the first run, before the
resources are actually released (it will happen on the Media thread later).

[qinmin, 2015/09/29 23:16:23]

The problem to post the task is that the MediaPlayerManager can go away when the
task is posted on the UI thread. As a result, the callback will not be executed
and the singleton MediaThrottler will not know that the player has been
released.

An alternative is to pass MediaThrottler::OnDecodeRequestFinished() to
MediaPlayerAndroid. However, in that case, the MediaPlayerManager won't know
that the decoder resources are released.

Added a todo there.

+
   RUN_ON_MEDIA_THREAD(Release);
 
   DVLOG(1) << __FUNCTION__;
@@ -783,13 +784,6 @@ void MediaCodecPlayer::OnTimeIntervalUpdate(DemuxerStream::Type type,
   }
 }
 
-void MediaCodecPlayer::OnVideoCodecCreated() {
-  DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
-
-  // This callback requests resources by releasing other players.
-  ui_task_runner_->PostTask(FROM_HERE, request_resources_cb_);
-}
-
 void MediaCodecPlayer::OnVideoResolutionChanged(const gfx::Size& size) {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
 
@@ -1143,8 +1137,8 @@ void MediaCodecPlayer::CreateDecoders() {
       internal_error_cb_,
       base::Bind(&MediaCodecPlayer::OnTimeIntervalUpdate, media_weak_this_,
                  DemuxerStream::VIDEO),
-      base::Bind(&MediaCodecPlayer::OnVideoResolutionChanged, media_weak_this_),
-      base::Bind(&MediaCodecPlayer::OnVideoCodecCreated, media_weak_this_)));
+      base::Bind(&MediaCodecPlayer::OnVideoResolutionChanged,
+                 media_weak_this_)));
 }
 
 bool MediaCodecPlayer::AudioFinished() const {