Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(159)

Unified Diff: content/browser/media/android/browser_media_player_manager.cc

Issue 1372203002: Throttle media decoding after excessive Android media server crashes (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/media/android/browser_media_player_manager.cc
diff --git a/content/browser/media/android/browser_media_player_manager.cc b/content/browser/media/android/browser_media_player_manager.cc
index e52732360555fd4e32d36f6de6640f2752fd67b3..b1e11d3f835112a76c5b979d1c22419362d4658d 100644
--- a/content/browser/media/android/browser_media_player_manager.cc
+++ b/content/browser/media/android/browser_media_player_manager.cc
@@ -10,6 +10,7 @@
#include "content/browser/media/android/browser_demuxer_android.h"
#include "content/browser/media/android/media_resource_getter_impl.h"
#include "content/browser/media/android/media_session.h"
+#include "content/browser/media/android/media_throttler.h"
#include "content/browser/media/media_web_contents_observer.h"
#include "content/browser/renderer_host/render_view_host_impl.h"
#include "content/browser/web_contents/web_contents_view_android.h"
@@ -144,16 +145,17 @@ MediaPlayerAndroid* BrowserMediaPlayerManager::CreateMediaPlayer(
user_agent,
hide_url_log,
this,
- base::Bind(&BrowserMediaPlayerManager::OnMediaResourcesRequested,
+ base::Bind(&BrowserMediaPlayerManager::OnPlayerReleased,
weak_ptr_factory_.GetWeakPtr()),
media_player_params.frame_url,
media_player_params.allow_credentials);
ContentViewCoreImpl* content_view_core_impl =
static_cast<ContentViewCoreImpl*>(ContentViewCore::FromWebContents(
web_contents_));
- if (!content_view_core_impl) {
- // May reach here due to prerendering. Don't extract the metadata
- // since it is expensive.
+ if (!content_view_core_impl
+ || !RequestDecoderResources(media_player_params.player_id, true)) {
Ted C 2015/09/28 22:42:29 why is this temporary? maybe my question from the
qinmin 2015/09/29 23:16:22 This is because calling Initialize() will decode a
+ // May reach here due to prerendering or throttling. Don't extract the
+ // metadata since it is expensive.
// TODO(qinmin): extract the metadata once the user decided to load
// the page.
OnMediaMetadataChanged(
@@ -171,7 +173,7 @@ MediaPlayerAndroid* BrowserMediaPlayerManager::CreateMediaPlayer(
return new MediaCodecPlayer(
media_player_params.player_id,
weak_ptr_factory_.GetWeakPtr(),
- base::Bind(&BrowserMediaPlayerManager::OnMediaResourcesRequested,
+ base::Bind(&BrowserMediaPlayerManager::OnPlayerReleased,
weak_ptr_factory_.GetWeakPtr()),
demuxer->CreateDemuxer(media_player_params.demuxer_client_id),
media_player_params.frame_url);
@@ -179,7 +181,7 @@ MediaPlayerAndroid* BrowserMediaPlayerManager::CreateMediaPlayer(
return new MediaSourcePlayer(
media_player_params.player_id,
this,
- base::Bind(&BrowserMediaPlayerManager::OnMediaResourcesRequested,
+ base::Bind(&BrowserMediaPlayerManager::OnPlayerReleased,
weak_ptr_factory_.GetWeakPtr()),
demuxer->CreateDemuxer(media_player_params.demuxer_client_id),
media_player_params.frame_url);
@@ -321,8 +323,10 @@ void BrowserMediaPlayerManager::OnSeekComplete(
void BrowserMediaPlayerManager::OnError(int player_id, int error) {
Send(new MediaPlayerMsg_MediaError(RoutingID(), player_id, error));
- if (fullscreen_player_id_ == player_id)
+ if (fullscreen_player_id_ == player_id &&
+ error != MediaPlayerAndroid::MEDIA_ERROR_INVALID_CODE) {
video_view_->OnMediaPlayerError(error);
+ }
}
void BrowserMediaPlayerManager::OnVideoSizeChanged(
@@ -478,6 +482,11 @@ void BrowserMediaPlayerManager::OnRequestExternalSurface(
base::Unretained(this)));
}
}
+
+void BrowserMediaPlayerManager::ReleaseExternalSurface(int player_id) {
+ if (external_video_surface_container_)
+ external_video_surface_container_->ReleaseExternalVideoSurface(player_id);
+}
#endif // defined(VIDEO_HOLE)
void BrowserMediaPlayerManager::OnEnterFullscreen(int player_id) {
@@ -536,10 +545,14 @@ void BrowserMediaPlayerManager::OnStart(int player_id) {
MediaPlayerAndroid* player = GetPlayer(player_id);
Tima Vaisburd 2015/09/28 22:09:12 Do you need to get the |player| here and at the be
qinmin 2015/09/29 23:16:22 Yes, both places. Here we will show the infobar, b
if (!player)
return;
- player->Start();
- if (fullscreen_player_id_ == player_id && fullscreen_player_is_released_) {
- video_view_->OpenVideo();
- fullscreen_player_is_released_ = false;
+
+ if (RequestDecoderResources(player_id, false)) {
+ StartInternal(player_id);
+ } else if (WebContentsDelegate* delegate = web_contents_->GetDelegate()){
+ delegate->RequestMediaDecodePermission(
+ web_contents_,
+ base::Bind(&BrowserMediaPlayerManager::OnPlaybackPermissionGranted,
+ weak_ptr_factory_.GetWeakPtr(), player_id));
}
}
@@ -610,13 +623,16 @@ void BrowserMediaPlayerManager::RemovePlayer(int player_id) {
for (ScopedVector<MediaPlayerAndroid>::iterator it = players_.begin();
it != players_.end(); ++it) {
if ((*it)->player_id() == player_id) {
- ReleaseMediaResources(player_id);
+#if defined(VIDEO_HOLE)
+ ReleaseExternalSurface(player_id);
+#endif
(*it)->DeleteOnCorrectThread();
players_.weak_erase(it);
MediaSession::Get(web_contents())->RemovePlayer(this, player_id);
break;
}
}
+ active_players_.erase(player_id);
}
scoped_ptr<media::MediaPlayerAndroid> BrowserMediaPlayerManager::SwapPlayer(
@@ -626,7 +642,9 @@ scoped_ptr<media::MediaPlayerAndroid> BrowserMediaPlayerManager::SwapPlayer(
it != players_.end(); ++it) {
if ((*it)->player_id() == player_id) {
previous_player = *it;
- ReleaseMediaResources(player_id);
+#if defined(VIDEO_HOLE)
+ ReleaseExternalSurface(player_id);
+#endif
players_.weak_erase(it);
players_.push_back(player);
break;
@@ -648,44 +666,75 @@ void BrowserMediaPlayerManager::ReleaseFullscreenPlayer(
ReleasePlayer(player);
}
-void BrowserMediaPlayerManager::OnMediaResourcesRequested(int player_id) {
- int num_active_player = 0;
- ScopedVector<MediaPlayerAndroid>::iterator it;
- for (it = players_.begin(); it != players_.end(); ++it) {
- if (!(*it)->IsPlayerReady())
- continue;
+bool BrowserMediaPlayerManager::RequestDecoderResources(
+ int player_id, bool temporary) {
+ if (!MediaThrottler::GetInstance()->RequestToDecodeData())
+ return false;
- // The player is already active, ignore it.
- if ((*it)->player_id() == player_id)
- return;
- else
- num_active_player++;
- }
+ // The player is already active, ignore it. A long running player should not
+ // request temporary permissions.
Ted C 2015/09/28 22:42:29 should there be an assert about that?
qinmin 2015/09/29 23:16:22 Done.
+ if (active_players_.find(player_id) != active_players_.end())
+ return true;
- // Number of active players are less than the threshold, do nothing.
- if (num_active_player < kMediaPlayerThreshold)
- return;
+ if (!temporary) {
+ int long_running_player = 0;
+ ActivePlayerMap::iterator it;
+ for (it = active_players_.begin(); it != active_players_.end(); ++it) {
+ if (!it->second)
+ long_running_player++;
+ }
+
+ // Number of active players are less than the threshold, do nothing.
+ if (long_running_player < kMediaPlayerThreshold)
+ return true;
- for (it = players_.begin(); it != players_.end(); ++it) {
- if ((*it)->IsPlayerReady() && !(*it)->IsPlaying() &&
- fullscreen_player_id_ != (*it)->player_id()) {
- ReleasePlayer(*it);
- Send(new MediaPlayerMsg_MediaPlayerReleased(RoutingID(),
- (*it)->player_id()));
+ for (it = active_players_.begin(); it != active_players_.end(); ++it) {
+ if (!it->second && !GetPlayer(it->first)->IsPlaying() &&
+ fullscreen_player_id_ != it->first) {
+ ReleasePlayer(GetPlayer(it->first));
+ Send(new MediaPlayerMsg_MediaPlayerReleased(RoutingID(),
+ (it->first)));
+ }
}
}
+
+ active_players_[player_id] = temporary;
+ return true;
}
-void BrowserMediaPlayerManager::ReleaseMediaResources(int player_id) {
-#if defined(VIDEO_HOLE)
- if (external_video_surface_container_)
- external_video_surface_container_->ReleaseExternalVideoSurface(player_id);
-#endif // defined(VIDEO_HOLE)
+void BrowserMediaPlayerManager::OnPlayerReleased(int player_id) {
+ if (active_players_.find(player_id) == active_players_.end())
+ return;
+
+ active_players_.erase(player_id);
+ MediaThrottler::GetInstance()->OnDecodeRequestFinished();
}
void BrowserMediaPlayerManager::ReleasePlayer(MediaPlayerAndroid* player) {
player->Release();
- ReleaseMediaResources(player->player_id());
+#if defined(VIDEO_HOLE)
+ ReleaseExternalSurface(player->player_id());
+#endif
+}
+
+void BrowserMediaPlayerManager::OnPlaybackPermissionGranted(
+ int player_id, bool granted) {
+ if (!granted)
+ return;
Ted C 2015/09/28 22:42:29 is there any cleanup to be done here?
qinmin 2015/09/29 23:16:22 not needed, the player has to acquire permission f
+
+ MediaThrottler::GetInstance()->Reset();
+ StartInternal(player_id);
+}
+
+void BrowserMediaPlayerManager::StartInternal(int player_id) {
+ MediaPlayerAndroid* player = GetPlayer(player_id);
+ if (!player)
+ return;
+ player->Start();
+ if (fullscreen_player_id_ == player_id && fullscreen_player_is_released_) {
+ video_view_->OpenVideo();
+ fullscreen_player_is_released_ = false;
+ }
}
} // namespace content

Powered by Google App Engine
This is Rietveld 408576698