Skip to content

Commit

Permalink
Improve MediaPlayerBridge seek resiliance
Browse files Browse the repository at this point in the history
crbug.com/677280 manifested whenever we tried seeking on a non-seekable
live stream, causing the MediaPlayer to crash, and the stream to never
start.

Dropping seeks based off of unknown or infinite media duration (for
livestreams) introduced crbug.com/679482. When we resume a tab, the
MediaPlayerRenderer is re-created, and if the preliminary attempt to
extract the media's metadata failed, we drop the first seek. This causes
the media restart after every tab switch.

It turns out that there is a better fix for 677280, which prevents
679482 entirely. Currently, in MediaPlayerBridge::OnMediaPrepared(), we
complete pending seeks before updating allowable operations. Updating
allowable operations as a first action properly sets "CanSeekForward"
and "CanSeekBackards", which then aborts any invalid seek attempts
without any additional logic.

BUG=679482, 677280
TEST= Manual test on a live HLS stream and non live HLS video.

Review-Url: https://codereview.chromium.org/2626443003
Cr-Commit-Position: refs/heads/master@{#442441}
  • Loading branch information
tguilbert-google authored and Commit bot committed Jan 10, 2017
1 parent 9bb8cf2 commit 0d32da5
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 6 deletions.
6 changes: 1 addition & 5 deletions content/browser/media/android/media_player_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,9 @@ void MediaPlayerRenderer::StartPlayingFrom(base::TimeDelta time) {
if (has_error_)
return;

media_player_->SeekTo(time);
media_player_->Start();

// Drop the seek if we are live streaming or if we don't have any duration
// information yet.
if (duration_ != media::kInfiniteDuration)
media_player_->SeekTo(time);

// WMPI needs to receive a BUFFERING_HAVE_ENOUGH data before sending a
// playback_rate > 0. The MediaPlayer manages its own buffering and will pause
// internally if ever it runs out of data. Sending BUFFERING_HAVE_ENOUGH here
Expand Down
3 changes: 2 additions & 1 deletion media/base/android/media_player_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,8 @@ void MediaPlayerBridge::OnMediaPrepared() {
prepared_ = true;
duration_ = GetDuration();

UpdateAllowedOperations();

// If media player was recovered from a saved state, consume all the pending
// events.
if (should_seek_on_prepare_) {
Expand All @@ -484,7 +486,6 @@ void MediaPlayerBridge::OnMediaPrepared() {
pending_play_ = false;
}

UpdateAllowedOperations();
manager()->OnMediaMetadataChanged(
player_id(), duration_, width_, height_, true);
}
Expand Down

0 comments on commit 0d32da5

Please sign in to comment.