From e0eeb14423a840cfaec05a0218897c3bf35a4548 Mon Sep 17 00:00:00 2001 From: dfalcantara Date: Fri, 25 Mar 2016 16:45:45 -0700 Subject: Revert of media: Enable Unified Media Pipeline for MSE and EME on Android (patchset #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 f92f4e5c849c028db73fbe06912685a77b978ee4 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} --- content/renderer/render_frame_impl.cc | 62 +++++++++------------- content/renderer/render_frame_impl.h | 1 + media/base/media.cc | 9 ++++ media/base/media.h | 6 +++ media/filters/stream_parser_factory.cc | 8 +-- media/media_options.gni | 2 +- .../WebKit/Source/core/html/HTMLMediaElement.cpp | 2 +- .../Source/core/html/HTMLVideoElementTest.cpp | 2 +- .../WebKit/Source/core/loader/EmptyClients.cpp | 3 +- .../WebKit/Source/core/loader/EmptyClients.h | 3 +- .../WebKit/Source/core/loader/FrameLoaderClient.h | 4 +- .../WebKit/Source/web/FrameLoaderClientImpl.cpp | 5 +- .../WebKit/Source/web/FrameLoaderClientImpl.h | 2 +- third_party/WebKit/public/web/WebFrameClient.h | 4 +- 14 files changed, 60 insertions(+), 53 deletions(-) diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index e1b14d9..d7eaf80 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -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 @@ -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) @@ -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()) { @@ -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 @@ -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, @@ -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) @@ -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(); } diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index 729c949..f392f2d 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h @@ -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, diff --git a/media/base/media.cc b/media/base/media.cc index 954fb19..b2e1c45 100644 --- a/media/base/media.cc +++ b/media/base/media.cc @@ -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() diff --git a/media/base/media.h b/media/base/media.h index ca5a8175..b6cccc4 100644 --- a/media/base/media.h +++ b/media/base/media.h @@ -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 diff --git a/media/filters/stream_parser_factory.cc b/media/filters/stream_parser_factory.cc index 0247098..92ccbf4 100644 --- a/media/filters/stream_parser_factory.cc +++ b/media/filters/stream_parser_factory.cc @@ -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; } @@ -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 diff --git a/media/media_options.gni b/media/media_options.gni index 0390f21..cedcdf7 100644 --- a/media/media_options.gni +++ b/media/media_options.gni @@ -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. diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp index 6dcd96d..4d4e353 100644 --- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp @@ -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; diff --git a/third_party/WebKit/Source/core/html/HTMLVideoElementTest.cpp b/third_party/WebKit/Source/core/html/HTMLVideoElementTest.cpp index 70a4498..4a605ce 100644 --- a/third_party/WebKit/Source/core/html/HTMLVideoElementTest.cpp +++ b/third_party/WebKit/Source/core/html/HTMLVideoElementTest.cpp @@ -63,7 +63,7 @@ public: return adoptPtrWillBeNoop(new StubFrameLoaderClient); } - PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) override + PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) override { return adoptPtr(new MockWebMediaPlayer); } diff --git a/third_party/WebKit/Source/core/loader/EmptyClients.cpp b/third_party/WebKit/Source/core/loader/EmptyClients.cpp index 9b44813..592a5c7 100644 --- a/third_party/WebKit/Source/core/loader/EmptyClients.cpp +++ b/third_party/WebKit/Source/core/loader/EmptyClients.cpp @@ -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" @@ -153,7 +152,7 @@ PassRefPtrWillBeRawPtr EmptyFrameLoaderClient::createPlugin(HTMLPlugInEl return nullptr; } -PassOwnPtr EmptyFrameLoaderClient::createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) +PassOwnPtr EmptyFrameLoaderClient::createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) { return nullptr; } diff --git a/third_party/WebKit/Source/core/loader/EmptyClients.h b/third_party/WebKit/Source/core/loader/EmptyClients.h index da0e924..911a8c5 100644 --- a/third_party/WebKit/Source/core/loader/EmptyClients.h +++ b/third_party/WebKit/Source/core/loader/EmptyClients.h @@ -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 @@ -246,7 +247,7 @@ public: PassRefPtrWillBeRawPtr createFrame(const FrameLoadRequest&, const AtomicString&, HTMLFrameOwnerElement*) override; PassRefPtrWillBeRawPtr createPlugin(HTMLPlugInElement*, const KURL&, const Vector&, const Vector&, const String&, bool, DetachedPluginPolicy) override; bool canCreatePluginWithoutRenderer(const String& mimeType) const override { return false; } - PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) override; + PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) override; PassOwnPtr createWebMediaSession() override; ObjectContentType getObjectContentType(const KURL&, const String&, bool) override { return ObjectContentType(); } diff --git a/third_party/WebKit/Source/core/loader/FrameLoaderClient.h b/third_party/WebKit/Source/core/loader/FrameLoaderClient.h index fb99716..4e0afca 100644 --- a/third_party/WebKit/Source/core/loader/FrameLoaderClient.h +++ b/third_party/WebKit/Source/core/loader/FrameLoaderClient.h @@ -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 @@ -67,7 +68,6 @@ class SubstituteData; class WebApplicationCacheHost; class WebApplicationCacheHostClient; class WebCookieJar; -class WebMediaPlayer; class WebMediaPlayerClient; class WebMediaSession; class WebRTCPeerConnectionHandler; @@ -161,7 +161,7 @@ public: virtual bool canCreatePluginWithoutRenderer(const String& mimeType) const = 0; virtual PassRefPtrWillBeRawPtr createPlugin(HTMLPlugInElement*, const KURL&, const Vector&, const Vector&, const String&, bool loadManually, DetachedPluginPolicy) = 0; - virtual PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) = 0; + virtual PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) = 0; virtual PassOwnPtr createWebMediaSession() = 0; diff --git a/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp b/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp index 06eb954..a89a59d 100644 --- a/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp +++ b/third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp @@ -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" @@ -820,6 +821,7 @@ PassRefPtrWillBeRawPtr FrameLoaderClientImpl::createPlugin( PassOwnPtr FrameLoaderClientImpl::createWebMediaPlayer( HTMLMediaElement& htmlMediaElement, + WebMediaPlayer::LoadType loadType, const WebURL& url, WebMediaPlayerClient* client) { @@ -835,7 +837,8 @@ PassOwnPtr 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)); } diff --git a/third_party/WebKit/Source/web/FrameLoaderClientImpl.h b/third_party/WebKit/Source/web/FrameLoaderClientImpl.h index 9ff9d19..ee342a9 100644 --- a/third_party/WebKit/Source/web/FrameLoaderClientImpl.h +++ b/third_party/WebKit/Source/web/FrameLoaderClientImpl.h @@ -129,7 +129,7 @@ public: HTMLPlugInElement*, const KURL&, const Vector&, const Vector&, const WTF::String&, bool loadManually, DetachedPluginPolicy) override; - PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, const WebURL&, WebMediaPlayerClient*) override; + PassOwnPtr createWebMediaPlayer(HTMLMediaElement&, WebMediaPlayer::LoadType, const WebURL&, WebMediaPlayerClient*) override; PassOwnPtr createWebMediaSession() override; ObjectContentType getObjectContentType( const KURL&, const WTF::String& mimeType, bool shouldPreferPlugInsForImages) override; diff --git a/third_party/WebKit/public/web/WebFrameClient.h b/third_party/WebKit/public/web/WebFrameClient.h index 8ac6ce0f..b136f79 100644 --- a/third_party/WebKit/public/web/WebFrameClient.h +++ b/third_party/WebKit/public/web/WebFrameClient.h @@ -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" @@ -75,7 +76,6 @@ class WebExternalPopupMenuClient; class WebFormElement; class WebGeolocationClient; class WebInstalledAppClient; -class WebMediaPlayer; class WebMediaPlayerClient; class WebMediaPlayerEncryptedMediaClient; class WebMediaSession; @@ -114,7 +114,7 @@ public: // 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; } -- cgit v1.1