Commit b625d909 authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Commit Bot

Remove MediaResourceGetter

Before playing media using the MediaPlayerBridge, we try to parse the
metadata, using the MediaResourceGetter. This class uses the Android
MediaMetadataRetriever API, which can be be resource intensive. We
eventually get all of the metadata we need from the Android
MediaPlayer, and pre-parsing the metadata sometimes does nothing but
delay the start of playback.

This CL removes the java MediaResourceGetter, and removes metadata
extraction capabilities from the C++ portion of the
MediaResourceGetter interface.

NB: This is unlikely to have a performance effect on HLS playback,
since the MediaResourceGetter did not support parsing HLS.

Bug: 739914
Change-Id: I460d5aea02bf1a0bf099f66a781a40ca6374d4d0
Reviewed-on: https://chromium-review.googlesource.com/988681Reviewed-by: default avatarPeter Wen <wnwen@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548221}
parent b5db77f5
...@@ -267,9 +267,6 @@ Still reading? ...@@ -267,9 +267,6 @@ Still reading?
<issue id="RtlCompat" severity="ignore"/> <issue id="RtlCompat" severity="ignore"/>
<issue id="RtlEnabled" severity="ignore"/> <issue id="RtlEnabled" severity="ignore"/>
<issue id="RtlSymmetry" severity="ignore"/> <issue id="RtlSymmetry" severity="ignore"/>
<issue id="SdCardPath">
<ignore regexp="content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java"/>
</issue>
<issue id="SetJavaScriptEnabled" severity="ignore"/> <issue id="SetJavaScriptEnabled" severity="ignore"/>
<issue id="SignatureOrSystemPermissions" severity="ignore"/> <issue id="SignatureOrSystemPermissions" severity="ignore"/>
<issue id="SpUsage" severity="Error"> <issue id="SpUsage" severity="Error">
......
...@@ -4,9 +4,6 @@ ...@@ -4,9 +4,6 @@
#include "content/browser/media/android/media_resource_getter_impl.h" #include "content/browser/media/android/media_resource_getter_impl.h"
#include "base/android/jni_android.h"
#include "base/android/jni_string.h"
#include "base/android/scoped_java_ref.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/path_service.h" #include "base/path_service.h"
...@@ -20,7 +17,6 @@ ...@@ -20,7 +17,6 @@
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "content/public/common/content_client.h" #include "content/public/common/content_client.h"
#include "content/public/common/url_constants.h" #include "content/public/common/url_constants.h"
#include "jni/MediaResourceGetter_jni.h"
#include "media/base/android/media_url_interceptor.h" #include "media/base/android/media_url_interceptor.h"
#include "net/base/auth.h" #include "net/base/auth.h"
#include "net/cookies/canonical_cookie.h" #include "net/cookies/canonical_cookie.h"
...@@ -31,10 +27,6 @@ ...@@ -31,10 +27,6 @@
#include "net/url_request/url_request_context_getter.h" #include "net/url_request/url_request_context_getter.h"
#include "url/gurl.h" #include "url/gurl.h"
using base::android::ConvertUTF8ToJavaString;
using base::android::JavaRef;
using base::android::ScopedJavaLocalRef;
namespace content { namespace content {
namespace { namespace {
...@@ -112,57 +104,6 @@ static void RequestPlaformPathFromFileSystemURL( ...@@ -112,57 +104,6 @@ static void RequestPlaformPathFromFileSystemURL(
ReturnResultOnUIThread(std::move(callback), std::string()); ReturnResultOnUIThread(std::move(callback), std::string());
} }
// Posts a task to the UI thread to run the callback function.
static void PostMediaMetadataCallbackTask(
media::MediaResourceGetter::ExtractMediaMetadataCB callback,
JNIEnv* env,
ScopedJavaLocalRef<jobject>& j_metadata) {
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::BindOnce(
std::move(callback),
base::TimeDelta::FromMilliseconds(
Java_MediaMetadata_getDurationInMilliseconds(env, j_metadata)),
Java_MediaMetadata_getWidth(env, j_metadata),
Java_MediaMetadata_getHeight(env, j_metadata),
Java_MediaMetadata_isSuccess(env, j_metadata)));
}
// Gets the metadata from a media URL. When finished, a task is posted to the UI
// thread to run the callback function.
static void GetMediaMetadata(
const std::string& url,
const std::string& cookies,
const std::string& user_agent,
media::MediaResourceGetter::ExtractMediaMetadataCB callback) {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jstring> j_url_string = ConvertUTF8ToJavaString(env, url);
ScopedJavaLocalRef<jstring> j_cookies = ConvertUTF8ToJavaString(env, cookies);
ScopedJavaLocalRef<jstring> j_user_agent = ConvertUTF8ToJavaString(
env, user_agent);
ScopedJavaLocalRef<jobject> j_metadata =
Java_MediaResourceGetter_extractMediaMetadata(env, j_url_string,
j_cookies, j_user_agent);
PostMediaMetadataCallbackTask(std::move(callback), env, j_metadata);
}
// Gets the metadata from a file descriptor. When finished, a task is posted to
// the UI thread to run the callback function.
static void GetMediaMetadataFromFd(
const int fd,
const int64_t offset,
const int64_t size,
media::MediaResourceGetter::ExtractMediaMetadataCB callback) {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> j_metadata =
Java_MediaResourceGetter_extractMediaMetadataFromFd(
env, fd, offset, size);
PostMediaMetadataCallbackTask(std::move(callback), env, j_metadata);
}
// The task object that retrieves media resources on the IO thread. // The task object that retrieves media resources on the IO thread.
// TODO(qinmin): refactor this class to make the code reusable by others as // TODO(qinmin): refactor this class to make the code reusable by others as
// there are lots of duplicated functionalities elsewhere. // there are lots of duplicated functionalities elsewhere.
...@@ -310,28 +251,4 @@ void MediaResourceGetterImpl::GetPlatformPathCallback( ...@@ -310,28 +251,4 @@ void MediaResourceGetterImpl::GetPlatformPathCallback(
std::move(callback).Run(platform_path); std::move(callback).Run(platform_path);
} }
void MediaResourceGetterImpl::ExtractMediaMetadata(
const std::string& url,
const std::string& cookies,
const std::string& user_agent,
ExtractMediaMetadataCB callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits(FROM_HERE,
{base::MayBlock(), base::TaskPriority::USER_VISIBLE},
base::BindOnce(&GetMediaMetadata, url, cookies,
user_agent, std::move(callback)));
}
void MediaResourceGetterImpl::ExtractMediaMetadata(
const int fd,
const int64_t offset,
const int64_t size,
ExtractMediaMetadataCB callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits(FROM_HERE,
{base::MayBlock(), base::TaskPriority::USER_VISIBLE},
base::BindOnce(&GetMediaMetadataFromFd, fd, offset,
size, std::move(callback)));
}
} // namespace content } // namespace content
...@@ -49,14 +49,6 @@ class MediaResourceGetterImpl : public media::MediaResourceGetter { ...@@ -49,14 +49,6 @@ class MediaResourceGetterImpl : public media::MediaResourceGetter {
GetCookieCB callback) override; GetCookieCB callback) override;
void GetPlatformPathFromURL(const GURL& url, void GetPlatformPathFromURL(const GURL& url,
GetPlatformPathCB callback) override; GetPlatformPathCB callback) override;
void ExtractMediaMetadata(const std::string& url,
const std::string& cookies,
const std::string& user_agent,
ExtractMediaMetadataCB callback) override;
void ExtractMediaMetadata(const int fd,
const int64_t offset,
const int64_t size,
ExtractMediaMetadataCB callback) override;
private: private:
// Called when GetAuthCredentials() finishes. // Called when GetAuthCredentials() finishes.
......
...@@ -131,7 +131,6 @@ android_library("content_java") { ...@@ -131,7 +131,6 @@ android_library("content_java") {
"java/src/org/chromium/content/browser/JavascriptInterface.java", "java/src/org/chromium/content/browser/JavascriptInterface.java",
"java/src/org/chromium/content/browser/JoystickHandler.java", "java/src/org/chromium/content/browser/JoystickHandler.java",
"java/src/org/chromium/content/browser/LauncherThread.java", "java/src/org/chromium/content/browser/LauncherThread.java",
"java/src/org/chromium/content/browser/MediaResourceGetter.java",
"java/src/org/chromium/content/browser/MediaSessionImpl.java", "java/src/org/chromium/content/browser/MediaSessionImpl.java",
"java/src/org/chromium/content/browser/MemoryMonitorAndroid.java", "java/src/org/chromium/content/browser/MemoryMonitorAndroid.java",
"java/src/org/chromium/content/browser/MotionEventSynthesizer.java", "java/src/org/chromium/content/browser/MotionEventSynthesizer.java",
...@@ -366,7 +365,6 @@ generate_jni("content_jni_headers") { ...@@ -366,7 +365,6 @@ generate_jni("content_jni_headers") {
"java/src/org/chromium/content/browser/InterstitialPageDelegateAndroid.java", "java/src/org/chromium/content/browser/InterstitialPageDelegateAndroid.java",
"java/src/org/chromium/content/browser/JavascriptInjectorImpl.java", "java/src/org/chromium/content/browser/JavascriptInjectorImpl.java",
"java/src/org/chromium/content/browser/LauncherThread.java", "java/src/org/chromium/content/browser/LauncherThread.java",
"java/src/org/chromium/content/browser/MediaResourceGetter.java",
"java/src/org/chromium/content/browser/MediaSessionImpl.java", "java/src/org/chromium/content/browser/MediaSessionImpl.java",
"java/src/org/chromium/content/browser/MemoryMonitorAndroid.java", "java/src/org/chromium/content/browser/MemoryMonitorAndroid.java",
"java/src/org/chromium/content/browser/NfcHost.java", "java/src/org/chromium/content/browser/NfcHost.java",
...@@ -470,7 +468,6 @@ android_library("content_javatests") { ...@@ -470,7 +468,6 @@ android_library("content_javatests") {
"javatests/src/org/chromium/content/browser/JavaBridgeCoercionTest.java", "javatests/src/org/chromium/content/browser/JavaBridgeCoercionTest.java",
"javatests/src/org/chromium/content/browser/JavaBridgeFieldsTest.java", "javatests/src/org/chromium/content/browser/JavaBridgeFieldsTest.java",
"javatests/src/org/chromium/content/browser/JavaBridgeReturnValuesTest.java", "javatests/src/org/chromium/content/browser/JavaBridgeReturnValuesTest.java",
"javatests/src/org/chromium/content/browser/MediaResourceGetterTest.java",
"javatests/src/org/chromium/content/browser/MediaSessionTest.java", "javatests/src/org/chromium/content/browser/MediaSessionTest.java",
"javatests/src/org/chromium/content/browser/NavigationTest.java", "javatests/src/org/chromium/content/browser/NavigationTest.java",
"javatests/src/org/chromium/content/browser/PopupZoomerTest.java", "javatests/src/org/chromium/content/browser/PopupZoomerTest.java",
......
...@@ -90,30 +90,15 @@ void MediaPlayerBridge::Initialize() { ...@@ -90,30 +90,15 @@ void MediaPlayerBridge::Initialize() {
return; return;
} }
if (url_.SchemeIsFile() || url_.SchemeIs("data")) { if (allow_credentials_) {
ExtractMediaMetadata(url_.spec()); media::MediaResourceGetter* resource_getter =
return; manager()->GetMediaResourceGetter();
}
resource_getter->GetCookies(
media::MediaResourceGetter* resource_getter = url_, site_for_cookies_,
manager()->GetMediaResourceGetter(); base::BindOnce(&MediaPlayerBridge::OnCookiesRetrieved,
if (url_.SchemeIsFileSystem()) { weak_factory_.GetWeakPtr()));
resource_getter->GetPlatformPathFromURL(
url_, base::BindOnce(&MediaPlayerBridge::ExtractMediaMetadata,
weak_factory_.GetWeakPtr()));
return;
}
// Start extracting the metadata immediately if the request is anonymous.
// Otherwise, wait for user credentials to be retrieved first.
if (!allow_credentials_) {
ExtractMediaMetadata(url_.spec());
return;
} }
resource_getter->GetCookies(url_, site_for_cookies_,
base::Bind(&MediaPlayerBridge::OnCookiesRetrieved,
weak_factory_.GetWeakPtr()));
} }
void MediaPlayerBridge::CreateJavaMediaPlayerBridge() { void MediaPlayerBridge::CreateJavaMediaPlayerBridge() {
...@@ -263,42 +248,6 @@ void MediaPlayerBridge::OnAuthCredentialsRetrieved( ...@@ -263,42 +248,6 @@ void MediaPlayerBridge::OnAuthCredentialsRetrieved(
replacements.SetPasswordStr(password); replacements.SetPasswordStr(password);
url_ = url_.ReplaceComponents(replacements); url_ = url_.ReplaceComponents(replacements);
} }
ExtractMediaMetadata(url_.spec());
}
void MediaPlayerBridge::ExtractMediaMetadata(const std::string& url) {
if (url.empty()) {
OnMediaError(MEDIA_ERROR_FORMAT);
on_decoder_resources_released_cb_.Run(player_id());
return;
}
int fd;
int64_t offset;
int64_t size;
if (InterceptMediaUrl(url, &fd, &offset, &size)) {
manager()->GetMediaResourceGetter()->ExtractMediaMetadata(
fd, offset, size,
base::Bind(&MediaPlayerBridge::OnMediaMetadataExtracted,
weak_factory_.GetWeakPtr()));
} else {
manager()->GetMediaResourceGetter()->ExtractMediaMetadata(
url, cookies_, user_agent_,
base::Bind(&MediaPlayerBridge::OnMediaMetadataExtracted,
weak_factory_.GetWeakPtr()));
}
}
void MediaPlayerBridge::OnMediaMetadataExtracted(
base::TimeDelta duration, int width, int height, bool success) {
if (success) {
duration_ = duration;
width_ = width;
height_ = height;
}
manager()->OnMediaMetadataChanged(
player_id(), duration_, width_, height_, success);
on_decoder_resources_released_cb_.Run(player_id());
} }
void MediaPlayerBridge::Start() { void MediaPlayerBridge::Start() {
......
...@@ -49,20 +49,6 @@ class MEDIA_EXPORT MediaResourceGetter { ...@@ -49,20 +49,6 @@ class MEDIA_EXPORT MediaResourceGetter {
// Method for getting the platform path from a file system URL. // Method for getting the platform path from a file system URL.
virtual void GetPlatformPathFromURL(const GURL& url, virtual void GetPlatformPathFromURL(const GURL& url,
GetPlatformPathCB callback) = 0; GetPlatformPathCB callback) = 0;
// Extracts the metadata from a media URL. Once completed, the provided
// callback function will be run.
virtual void ExtractMediaMetadata(const std::string& url,
const std::string& cookies,
const std::string& user_agent,
ExtractMediaMetadataCB callback) = 0;
// Extracts the metadata from a file descriptor. Once completed, the
// provided callback function will be run.
virtual void ExtractMediaMetadata(const int fd,
const int64_t offset,
const int64_t size,
ExtractMediaMetadataCB callback) = 0;
}; };
} // namespace media } // namespace media
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment