New issue
Advanced search Search tips

Issue 739914 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Remove media metadata extraction from MediaResourceGetter.

Project Member Reported by dalecur...@chromium.org, Jul 6 2017

Issue description

Metadata parsing is one of the riskiest things we can do on older Android versions; we should stop doing that.

For most users we'll only fall back to MediaPlayer for HLS playback. Currently the way the code is written we end up hitting MediaResourceGetter for extracting cookies and metadata then immediately playing the file and getting the metadata again through playback. We still need MediaResourceGetter for cookie extraction, but the metadata work is actually slowing down time to playback for HLS media.

For blacklisted users, we should instead rely on the container metadata provided by ffmpeg prior to switching over to the MediaPlayer path or decide that blacklisted devices should always use preload=none.

For unsupported webview content we should just disable metadata extraction; i.e. force preload=none. I'll add a UMA to see if we can figure out how often we actually attempt unsupported content in webview.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 7 2017

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

commit 2fe03e902371cafe3421893bf64f0f12f97136e0
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Fri Jul 07 20:40:19 2017

Remove flac from unsupported WebView containers. Add UMA for rest.

We've had flac support for a while now, so this should go through the
normal playback pipeline. To see if the rest are actually used anywhere
lets add a UMA to figure out if we really need this code.

BUG= 739914 
TEST=none

Change-Id: I18ddbc724b82d34a0174692b46a0a154e1d4cbf3
Reviewed-on: https://chromium-review.googlesource.com/562621
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485033}
[modify] https://crrev.com/2fe03e902371cafe3421893bf64f0f12f97136e0/android_webview/renderer/aw_content_renderer_client.cc
[modify] https://crrev.com/2fe03e902371cafe3421893bf64f0f12f97136e0/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/2fe03e902371cafe3421893bf64f0f12f97136e0/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 4 2018

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

commit b625d909d4bf5d8c8b59da9c8ea2d571c00971a8
Author: Thomas Guilbert <tguilbert@chromium.org>
Date: Wed Apr 04 22:32:21 2018

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/988681
Reviewed-by: Peter Wen <wnwen@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548221}
[modify] https://crrev.com/b625d909d4bf5d8c8b59da9c8ea2d571c00971a8/build/android/lint/suppressions.xml
[modify] https://crrev.com/b625d909d4bf5d8c8b59da9c8ea2d571c00971a8/content/browser/media/android/media_resource_getter_impl.cc
[modify] https://crrev.com/b625d909d4bf5d8c8b59da9c8ea2d571c00971a8/content/browser/media/android/media_resource_getter_impl.h
[modify] https://crrev.com/b625d909d4bf5d8c8b59da9c8ea2d571c00971a8/content/public/android/BUILD.gn
[delete] https://crrev.com/b5db77f579b9fa956042c299f8b2a6e4336a60d8/content/public/android/java/src/org/chromium/content/browser/MediaResourceGetter.java
[delete] https://crrev.com/b5db77f579b9fa956042c299f8b2a6e4336a60d8/content/public/android/javatests/src/org/chromium/content/browser/MediaResourceGetterTest.java
[modify] https://crrev.com/b625d909d4bf5d8c8b59da9c8ea2d571c00971a8/media/base/android/media_player_bridge.cc
[modify] https://crrev.com/b625d909d4bf5d8c8b59da9c8ea2d571c00971a8/media/base/android/media_resource_getter.h

Sign in to add a comment