Skip to content

Commit

Permalink
Revert of media: Enable Unified Media Pipeline for MSE and EME on And…
Browse files Browse the repository at this point in the history
…roid (patchset chromium#3 id:40001 of https://chromiumcodereview.appspot.com/1825763002/ )

Reason for revert:
Android tests on many bots started failing right after this; these are the most obvious:

https://build.chromium.org/p/chromium.android/builders/Lollipop%20Phone%20Tester/builds/3714
* RenderFrameImplTest.LoFiNotUpdatedOnSubframeCommits
* RenderViewImplTest.OnSetAccessibilityMode

I reverted locally multiple times to make sure, but RenderFrameImplTest.LoFiNotUpdatedOnSubframeCommits only starts failing when this CL is committed, AFAICT.

Original issue's description:
> media: Enable Unified Media Pipeline for MSE and EME on Android
>
> Enables Mojo Media on Android to support EME in the unified media
> pipeline. This introduces MojoCdm, MojoAudioDecoder and encrytped
> stream support in AndroidVideoDecodeAccelerator.
>
> This CL also enables MSE in the unified media pipeline. The fallback
> logic for MSE (IsUnifiedMediaPipelineEnabledForMse()) is removed.
>
> Also partially reverts f92f4e5 which
> added "LoadType" in createMediaPlayer() to implement the fallback
> logic for MSE.
>
> BUG=455905,521731
> TEST=Encrypted audio/video plays in default Chrome for Android build
> with and without unified media pipeline.
>
> Committed: https://crrev.com/92d0fffc36695c099005bf05862145a89d918f28
> Cr-Commit-Position: refs/heads/master@{#383331}

TBR=dalecurtis@chromium.org,ddorwin@chromium.org,timav@chromium.org,wolenetz@chromium.org,pfeldman@chromium.org,xhwang@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=455905,521731

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

Cr-Commit-Position: refs/heads/master@{#383409}
  • Loading branch information
dfalcantara authored and Commit bot committed Mar 25, 2016
1 parent d1c68aa commit e0eeb14
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 53 deletions.
62 changes: 25 additions & 37 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,8 @@
#endif

#if defined(ENABLE_PEPPER_CDMS)
#include "content/renderer/media/cdm/render_cdm_factory.h"
#include "content/renderer/media/cdm/pepper_cdm_wrapper_impl.h"
#elif defined(ENABLE_BROWSER_CDMS)
#include "content/renderer/media/cdm/render_cdm_factory.h"
#include "content/renderer/media/cdm/renderer_cdm_manager.h"
#endif

Expand All @@ -227,6 +225,8 @@

#if defined(ENABLE_MOJO_CDM)
#include "media/mojo/services/mojo_cdm_factory.h" // nogncheck
#else
#include "content/renderer/media/cdm/render_cdm_factory.h"
#endif

#if defined(ENABLE_MOJO_RENDERER)
Expand Down Expand Up @@ -755,13 +755,17 @@ bool IsContentWithCertificateErrorsRelevantToUI(
//
// Note that HLS and MP4 detection are pre-redirect and path-based. It is
// possible to load such a URL and find different content.
bool UseWebMediaPlayerImpl(const GURL& url) {
bool UseWebMediaPlayerImpl(blink::WebMediaPlayer::LoadType load_type,
const GURL& url) {
if (load_type == blink::WebMediaPlayer::LoadTypeMediaSource)
return media::IsUnifiedMediaPipelineEnabledForMse();

// WMPI does not support HLS.
if (media::MediaCodecUtil::IsHLSPath(url))
return false;

// Don't use WMPI if the container likely contains a codec we can't decode in
// software and platform decoders are not available.
// software and hardware decoders are not available.
if (base::EndsWith(url.path(), ".mp4",
base::CompareCase::INSENSITIVE_ASCII) &&
!media::HasPlatformDecoderSupport()) {
Expand All @@ -773,24 +777,6 @@ bool UseWebMediaPlayerImpl(const GURL& url) {
}
#endif // defined(OS_ANDROID)

#if defined(ENABLE_MOJO_CDM)
// Returns whether mojo CDM should be used at runtime. Note that even when mojo
// CDM is enabled at compile time (ENABLE_MOJO_CDM is defined), there are cases
// where we want to choose other CDM types. For example, on Android when we use
// WebMediaPlayerAndroid, we still want to use ProxyMediaKeys. In the future,
// when we experiment mojo CDM on desktop, we will choose between mojo CDM and
// pepper CDM at runtime.
// TODO(xhwang): Remove this when we use mojo CDM for all remote CDM cases by
// default.
bool UseMojoCdm() {
#if defined(OS_ANDROID)
return media::IsUnifiedMediaPipelineEnabled();
#else
return true;
#endif
}
#endif // defined(ENABLE_MOJO_CDM)

} // namespace

// static
Expand Down Expand Up @@ -2397,6 +2383,7 @@ blink::WebPlugin* RenderFrameImpl::createPlugin(
}

blink::WebMediaPlayer* RenderFrameImpl::createMediaPlayer(
blink::WebMediaPlayer::LoadType load_type,
const blink::WebURL& url,
WebMediaPlayerClient* client,
WebMediaPlayerEncryptedMediaClient* encrypted_client,
Expand Down Expand Up @@ -2443,7 +2430,7 @@ blink::WebMediaPlayer* RenderFrameImpl::createMediaPlayer(
initial_cdm, media_surface_manager_, media_session);

#if defined(OS_ANDROID)
if (!UseWebMediaPlayerImpl(url))
if (!UseWebMediaPlayerImpl(load_type, url))
return CreateAndroidWebMediaPlayer(client, encrypted_client, params);
#endif // defined(OS_ANDROID)

Expand Down Expand Up @@ -5915,25 +5902,26 @@ bool RenderFrameImpl::AreSecureCodecsSupported() {
}

media::CdmFactory* RenderFrameImpl::GetCdmFactory() {
if (cdm_factory_)
return cdm_factory_.get();
#if defined(ENABLE_BROWSER_CDMS)
if (!cdm_manager_)
cdm_manager_ = new RendererCdmManager(this);
#endif // defined(ENABLE_BROWSER_CDMS)

if (!cdm_factory_) {
DCHECK(frame_);

#if defined(ENABLE_MOJO_CDM)
if (UseMojoCdm()) {
cdm_factory_.reset(new media::MojoCdmFactory(GetMediaInterfaceProvider()));
return cdm_factory_.get();
}
#endif // defined(ENABLE_MOJO_CDM)

#else
cdm_factory_.reset(new RenderCdmFactory(
#if defined(ENABLE_PEPPER_CDMS)
DCHECK(frame_);
cdm_factory_.reset(
new RenderCdmFactory(base::Bind(&PepperCdmWrapperImpl::Create, frame_)));
base::Bind(&PepperCdmWrapperImpl::Create, frame_)
#elif defined(ENABLE_BROWSER_CDMS)
if (!cdm_manager_)
cdm_manager_ = new RendererCdmManager(this);
cdm_factory_.reset(new RenderCdmFactory(cdm_manager_));
#endif // defined(ENABLE_PEPPER_CDMS)
cdm_manager_
#endif
));
#endif // defined(ENABLE_MOJO_CDM)
}

return cdm_factory_.get();
}
Expand Down
1 change: 1 addition & 0 deletions content/renderer/render_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ class CONTENT_EXPORT RenderFrameImpl
blink::WebPlugin* createPlugin(blink::WebLocalFrame* frame,
const blink::WebPluginParams& params) override;
blink::WebMediaPlayer* createMediaPlayer(
blink::WebMediaPlayer::LoadType load_type,
const blink::WebURL& url,
blink::WebMediaPlayerClient* client,
blink::WebMediaPlayerEncryptedMediaClient* encrypted_client,
Expand Down
9 changes: 9 additions & 0 deletions media/base/media.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ bool IsUnifiedMediaPipelineEnabled() {
base::StartsWith(group_name, "Enabled", base::CompareCase::SENSITIVE);
}

bool IsUnifiedMediaPipelineEnabledForMse() {
// Don't check IsUnifiedMediaPipelineEnabled() here since we don't want MSE to
// be enabled via experiment yet; only when the existing implementation can't
// be used (i.e. MediaCodec unavailable).
return base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableUnifiedMediaPipeline) ||
!MediaCodecUtil::IsMediaCodecAvailable();
}

bool ArePlatformDecodersAvailable() {
return IsUnifiedMediaPipelineEnabled()
? HasPlatformDecoderSupport()
Expand Down
6 changes: 6 additions & 0 deletions media/base/media.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ MEDIA_EXPORT bool PlatformHasVp9Support();
// unified media pipeline is supported everywhere. http://crbug.com/580626.
MEDIA_EXPORT bool IsUnifiedMediaPipelineEnabled();

// Similar to IsUnifiedMediaPipelineEnabled() but will also return true if
// MediaCodec is not available (allowing the unified pipeline to take over for
// cases where existing pipeline has no support). As above, codecs requiring
// platform support may not be available.
MEDIA_EXPORT bool IsUnifiedMediaPipelineEnabledForMse();

// Returns whether the platform decoders are available for use.
// This includes decoders being available on the platform and accessible, such
// as via the GPU process. Should only be used for actual decoders
Expand Down
8 changes: 4 additions & 4 deletions media/filters/stream_parser_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ static bool VerifyCodec(
// TODO(wolenetz, dalecurtis): This should instead use MimeUtil() to avoid
// duplication of subtle Android behavior. http://crbug.com/587303.
if (codec_info->tag == CodecInfo::HISTOGRAM_H264) {
if (media::IsUnifiedMediaPipelineEnabled() &&
if (media::IsUnifiedMediaPipelineEnabledForMse() &&
!media::HasPlatformDecoderSupport()) {
return false;
}
Expand All @@ -344,17 +344,17 @@ static bool VerifyCodec(
}
if (codec_info->tag == CodecInfo::HISTOGRAM_VP8 &&
!media::MediaCodecUtil::IsVp8DecoderAvailable() &&
!media::IsUnifiedMediaPipelineEnabled()) {
!media::IsUnifiedMediaPipelineEnabledForMse()) {
return false;
}
if (codec_info->tag == CodecInfo::HISTOGRAM_VP9 &&
!media::PlatformHasVp9Support() &&
!media::IsUnifiedMediaPipelineEnabled()) {
!media::IsUnifiedMediaPipelineEnabledForMse()) {
return false;
}
if (codec_info->tag == CodecInfo::HISTOGRAM_OPUS &&
!media::PlatformHasOpusSupport() &&
!media::IsUnifiedMediaPipelineEnabled()) {
!media::IsUnifiedMediaPipelineEnabledForMse()) {
return false;
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion media/media_options.gni
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ declare_args() {
# |mojo_media_services|). When enabled, selected mojo paths will be enabled in
# the media pipeline and corresponding services will hosted in the selected
# remote process (e.g. "utility" process, see |mojo_media_host|).
enable_mojo_media = is_android
enable_mojo_media = false

# Enable the TestMojoMediaClient to be used in MojoMediaApplication. This is
# for testing only and will override the default platform MojoMediaClient.
Expand Down
2 changes: 1 addition & 1 deletion third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ void HTMLMediaElement::startPlayerLoad()
}

KURL kurl(ParsedURLString, requestURL);
m_webMediaPlayer = frame->loader().client()->createWebMediaPlayer(*this, kurl, this);
m_webMediaPlayer = frame->loader().client()->createWebMediaPlayer(*this, loadType(), kurl, this);
if (!m_webMediaPlayer) {
mediaLoadingFailed(WebMediaPlayer::NetworkStateFormatError);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class StubFrameLoaderClient : public EmptyFrameLoaderClient {
return adoptPtrWillBeNoop(new StubFrameLoaderClient);
}

PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) override
PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) override
{
return adoptPtr(new MockWebMediaPlayer);
}
Expand Down
3 changes: 1 addition & 2 deletions third_party/WebKit/Source/core/loader/EmptyClients.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "platform/Widget.h"
#include "public/platform/Platform.h"
#include "public/platform/WebApplicationCacheHost.h"
#include "public/platform/WebMediaPlayer.h"
#include "public/platform/modules/mediasession/WebMediaSession.h"
#include "public/platform/modules/serviceworker/WebServiceWorkerProvider.h"
#include "public/platform/modules/serviceworker/WebServiceWorkerProviderClient.h"
Expand Down Expand Up @@ -153,7 +152,7 @@ PassRefPtrWillBeRawPtr<Widget> EmptyFrameLoaderClient::createPlugin(HTMLPlugInEl
return nullptr;
}

PassOwnPtr<WebMediaPlayer> EmptyFrameLoaderClient::createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*)
PassOwnPtr<WebMediaPlayer> EmptyFrameLoaderClient::createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*)
{
return nullptr;
}
Expand Down
3 changes: 2 additions & 1 deletion third_party/WebKit/Source/core/loader/EmptyClients.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "platform/text/TextCheckerClient.h"
#include "public/platform/WebFocusType.h"
#include "public/platform/WebFrameScheduler.h"
#include "public/platform/WebMediaPlayer.h"
#include "public/platform/WebScreenInfo.h"
#include "wtf/Forward.h"
#include <v8.h>
Expand Down Expand Up @@ -246,7 +247,7 @@ class CORE_EXPORT EmptyFrameLoaderClient : public FrameLoaderClient {
PassRefPtrWillBeRawPtr<LocalFrame> createFrame(const FrameLoadRequest&, const AtomicString&, HTMLFrameOwnerElement*) override;
PassRefPtrWillBeRawPtr<Widget> createPlugin(HTMLPlugInElement*, const KURL&, const Vector<String>&, const Vector<String>&, const String&, bool, DetachedPluginPolicy) override;
bool canCreatePluginWithoutRenderer(const String& mimeType) const override { return false; }
PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) override;
PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) override;
PassOwnPtr<WebMediaSession> createWebMediaSession() override;

ObjectContentType getObjectContentType(const KURL&, const String&, bool) override { return ObjectContentType(); }
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/core/loader/FrameLoaderClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "platform/heap/Handle.h"
#include "platform/network/ResourceLoadPriority.h"
#include "platform/weborigin/Referrer.h"
#include "public/platform/WebMediaPlayer.h"
#include "wtf/Forward.h"
#include "wtf/Vector.h"
#include <v8.h>
Expand All @@ -67,7 +68,6 @@ class SubstituteData;
class WebApplicationCacheHost;
class WebApplicationCacheHostClient;
class WebCookieJar;
class WebMediaPlayer;
class WebMediaPlayerClient;
class WebMediaSession;
class WebRTCPeerConnectionHandler;
Expand Down Expand Up @@ -161,7 +161,7 @@ class CORE_EXPORT FrameLoaderClient : public FrameClient {
virtual bool canCreatePluginWithoutRenderer(const String& mimeType) const = 0;
virtual PassRefPtrWillBeRawPtr<Widget> createPlugin(HTMLPlugInElement*, const KURL&, const Vector<String>&, const Vector<String>&, const String&, bool loadManually, DetachedPluginPolicy) = 0;

virtual PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) = 0;
virtual PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) = 0;

virtual PassOwnPtr<WebMediaSession> createWebMediaSession() = 0;

Expand Down
5 changes: 4 additions & 1 deletion third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
#include "platform/plugins/PluginData.h"
#include "public/platform/Platform.h"
#include "public/platform/WebApplicationCacheHost.h"
#include "public/platform/WebMediaPlayer.h"
#include "public/platform/WebMimeRegistry.h"
#include "public/platform/WebRTCPeerConnectionHandler.h"
#include "public/platform/WebSecurityOrigin.h"
Expand Down Expand Up @@ -820,6 +821,7 @@ PassRefPtrWillBeRawPtr<Widget> FrameLoaderClientImpl::createPlugin(

PassOwnPtr<WebMediaPlayer> FrameLoaderClientImpl::createWebMediaPlayer(
HTMLMediaElement& htmlMediaElement,
WebMediaPlayer::LoadType loadType,
const WebURL& url,
WebMediaPlayerClient* client)
{
Expand All @@ -835,7 +837,8 @@ PassOwnPtr<WebMediaPlayer> FrameLoaderClientImpl::createWebMediaPlayer(

HTMLMediaElementEncryptedMedia& encryptedMedia = HTMLMediaElementEncryptedMedia::from(htmlMediaElement);
WebString sinkId(HTMLMediaElementAudioOutputDevice::sinkId(htmlMediaElement));
return adoptPtr(webFrame->client()->createMediaPlayer(url, client, &encryptedMedia,
return adoptPtr(webFrame->client()->createMediaPlayer(loadType, url,
client, &encryptedMedia,
encryptedMedia.contentDecryptionModule(), sinkId, webMediaSession));
}

Expand Down
2 changes: 1 addition & 1 deletion third_party/WebKit/Source/web/FrameLoaderClientImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class FrameLoaderClientImpl final : public FrameLoaderClient {
HTMLPlugInElement*, const KURL&,
const Vector<WTF::String>&, const Vector<WTF::String>&,
const WTF::String&, bool loadManually, DetachedPluginPolicy) override;
PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) override;
PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) override;
PassOwnPtr<WebMediaSession> createWebMediaSession() override;
ObjectContentType getObjectContentType(
const KURL&, const WTF::String& mimeType, bool shouldPreferPlugInsForImages) override;
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/public/web/WebFrameClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "public/platform/WebCommon.h"
#include "public/platform/WebFileSystem.h"
#include "public/platform/WebFileSystemType.h"
#include "public/platform/WebMediaPlayer.h"
#include "public/platform/WebSecurityOrigin.h"
#include "public/platform/WebSetSinkIdCallbacks.h"
#include "public/platform/WebStorageQuotaCallbacks.h"
Expand Down Expand Up @@ -75,7 +76,6 @@ class WebExternalPopupMenuClient;
class WebFormElement;
class WebGeolocationClient;
class WebInstalledAppClient;
class WebMediaPlayer;
class WebMediaPlayerClient;
class WebMediaPlayerEncryptedMediaClient;
class WebMediaSession;
Expand Down Expand Up @@ -114,7 +114,7 @@ class WebFrameClient {

// May return null.
// WebContentDecryptionModule* may be null if one has not yet been set.
virtual WebMediaPlayer* createMediaPlayer(const WebURL&, WebMediaPlayerClient*, WebMediaPlayerEncryptedMediaClient*, WebContentDecryptionModule*, const WebString& sinkId, WebMediaSession*) { return 0; }
virtual WebMediaPlayer* createMediaPlayer(WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*, WebMediaPlayerEncryptedMediaClient*, WebContentDecryptionModule*, const WebString& sinkId, WebMediaSession*) { return 0; }

// May return null.
virtual WebMediaSession* createMediaSession() { return 0; }
Expand Down

0 comments on commit e0eeb14

Please sign in to comment.