New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 618109 link

Starred by 4 users
Status: Fixed
Owner:
dalecur...@chromium.org
Closed: Jun 2016
Cc:
█ w...@chromium.org
sande...@chromium.org
liber...@chromium.org
foolip@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Falling back to WMPA if WMPI is redirected to a HLS url

Project Member Reported by w...@chromium.org, Jun 7 2016 
Today we decide in createWebMediaPlayer() whether to return a WebMediaPlayerAndroid or WebMediaPlayerImpl based on various conditions. One of which is whether the url is a HLS video, because only WMPA supports HLS. We detect whether a url is HLS by checking whether "m3u8" appears in the URL (copied from Android's MediaPlayer). However this doesn't work if the src is a redirect because we haven't yet made any network requests when making this choice.

espn is actually doing these redirects today, which means their videos are broken with Spitzer. We need a way to switch WebMediaPlayer implementations dynamically.

Comment 1 by dalecur...@chromium.org, Jun 8 2016 

I think the simplest way to solve this is to introduce a new method:

(KURL|GURL|std::string) WebMediaPlayer::alternativeSourceURL()

Which HTMLMediaElement::mediaLoadingFailed() can check and then trigger scheduleNextSourceChild(). Eventually that results in selectMediaResource() being called which can use that URL to provide to loadResource() before the old media player is destroyed (preserving the previous m_loadState type). Something like this:

https://codereview.chromium.org/2046253002

Warning: Untested, untried :)

Comment 2 by dalecur...@chromium.org, Jun 13 2016

Cc: -dalecur...@chromium.org
Owner: dalecur...@chromium.org
Status: Assigned (was: Available)

Comment 3 by dalecur...@chromium.org, Jun 13 2016

Labels: -Pri-2 ReleaseBlock-Stable M-52 Pri-1
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 24 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea27a3ed74612a72945993222b01f6aaddccbfe4

commit ea27a3ed74612a72945993222b01f6aaddccbfe4
Author: dalecurtis <dalecurtis@chromium.org>
Date: Fri Jun 24 01:41:30 2016

When HLS redirects are encountered recreate WebMediaPlayer.

Not all WebMediaPlayer implementations know how to play HLS, and
if the URL does not contain an HLS identifier originally, a later
redirect might break playback if the wrong implementation is
chosen.

When this occurs instead re-trigger Blink's loader with the
redirected source URL that contains the proper HLS tagging.

It's preferred to do this instead of either adding a shim WMP
implementation that choses the real WMP during load() or making
createWebMediaPlayer() block until redirects complete and then
throwing away the data from those connections.

BUG= 618109 
TEST=Manually tested espn.go.com and adds a new browser test.

Review-Url: https://codereview.chromium.org/2046253002
Cr-Commit-Position: refs/heads/master@{#401777}

[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/content/browser/media/media_browsertest.cc
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/content/browser/media/media_browsertest.h
[add] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/content/browser/media/media_redirect_browsertest.cc
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/content/content_tests.gypi
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/base/android/media_codec_util.cc
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/base/android/media_codec_util.h
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/blink/buffered_data_source.cc
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/blink/buffered_data_source.h
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/blink/multibuffer_data_source.cc
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/blink/multibuffer_data_source.h
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/blink/webmediaplayer_impl_unittest.cc
[add] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/test/data/bear.m3u8
[add] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/media/test/data/bear0.ts
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/third_party/WebKit/Source/core/html/HTMLMediaElement.h
[modify] https://crrev.com/ea27a3ed74612a72945993222b01f6aaddccbfe4/third_party/WebKit/public/platform/WebMediaPlayerClient.h

Comment 5 by dalecur...@chromium.org, Jun 24 2016

Labels: Merge-Request-52
Verified and tested, merge requesting.

Comment 6 by dimu@google.com, Jun 24 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 24 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46079771558372f7090b96de2c0c4596ac130696

commit 46079771558372f7090b96de2c0c4596ac130696
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Fri Jun 24 21:02:10 2016

Merge M52: "When HLS redirects are encountered recreate WebMediaPlayer."

Not all WebMediaPlayer implementations know how to play HLS, and
if the URL does not contain an HLS identifier originally, a later
redirect might break playback if the wrong implementation is
chosen.

When this occurs instead re-trigger Blink's loader with the
redirected source URL that contains the proper HLS tagging.

It's preferred to do this instead of either adding a shim WMP
implementation that choses the real WMP during load() or making
createWebMediaPlayer() block until redirects complete and then
throwing away the data from those connections.

BUG= 618109 
TEST=Manually tested espn.go.com and adds a new browser test.

Review-Url: https://codereview.chromium.org/2046253002
Cr-Commit-Position: refs/heads/master@{#401777}
(cherry picked from commit ea27a3ed74612a72945993222b01f6aaddccbfe4)

Review URL: https://codereview.chromium.org/2095053003 .

Cr-Commit-Position: refs/branch-heads/2743@{#470}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/content/browser/media/media_browsertest.cc
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/content/browser/media/media_browsertest.h
[add] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/content/browser/media/media_redirect_browsertest.cc
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/content/content_tests.gypi
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/content/renderer/media/webmediaplayer_ms_unittest.cc
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/base/android/media_codec_util.cc
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/base/android/media_codec_util.h
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/blink/buffered_data_source.cc
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/blink/buffered_data_source.h
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/blink/multibuffer_data_source.cc
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/blink/multibuffer_data_source.h
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/blink/webmediaplayer_impl_unittest.cc
[add] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/test/data/bear.m3u8
[add] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/media/test/data/bear0.ts
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/third_party/WebKit/Source/core/html/HTMLMediaElement.h
[modify] https://crrev.com/46079771558372f7090b96de2c0c4596ac130696/third_party/WebKit/public/platform/WebMediaPlayerClient.h

Comment 8 by dalecur...@chromium.org, Jun 24 2016

Status: Fixed (was: Assigned)

Comment 9 by w...@chromium.org, Jun 30 2016

Cc: liber...@chromium.org
 Issue 624832  has been merged into this issue.

Comment 10 by dalecur...@chromium.org, Jul 28 2016 

 Issue 632336  has been merged into this issue.
►

Sign in to add a comment