summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authortimav <timav@chromium.org>2016-02-19 21:19:40 -0800
committerCommit bot <commit-bot@chromium.org>2016-02-20 05:20:45 +0000
commit58e9361d766646213fca23e5761313f9ff502667 (patch)
tree59907acca6c16b302ea38215054322dc74239c4d
parentedb920a2e81a6be07d4ef30da09e3f2e9debe0c1 (diff)
downloadchromium_src-58e9361d766646213fca23e5761313f9ff502667.zip
chromium_src-58e9361d766646213fca23e5761313f9ff502667.tar.gz
chromium_src-58e9361d766646213fca23e5761313f9ff502667.tar.bz2
Add exit status UMA metrics for MediaPlayerBridge
Upon a MediaPlayer destruction report to UMA whether this player had errors durng playback. The implementation records all errors and reports the status only if Start() has ever been called. BUG=582159 Review URL: https://codereview.chromium.org/1705573002 Cr-Commit-Position: refs/heads/master@{#376626}
Diffstat
-rw-r--r--media/base/android/java/src/org/chromium/media/MediaPlayerListener.java4
-rw-r--r--media/base/android/media_player_android.h1
-rw-r--r--media/base/android/media_player_bridge.cc51
-rw-r--r--media/base/android/media_player_bridge.h14
-rw-r--r--tools/metrics/histograms/histograms.xml10
5 files changed, 80 insertions, 0 deletions
diff --git a/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java b/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
index 0cac2e0..403860b 100644
--- a/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
+++ b/media/base/android/java/src/org/chromium/media/MediaPlayerListener.java
@@ -25,6 +25,7 @@ class MediaPlayerListener implements MediaPlayer.OnPreparedListener,
private static final int MEDIA_ERROR_DECODE = 1;
private static final int MEDIA_ERROR_NOT_VALID_FOR_PROGRESSIVE_PLAYBACK = 2;
private static final int MEDIA_ERROR_INVALID_CODE = 3;
+ private static final int MEDIA_ERROR_SERVER_DIED = 4;
// These values are copied from android media player.
public static final int MEDIA_ERROR_MALFORMED = -1007;
@@ -59,6 +60,9 @@ class MediaPlayerListener implements MediaPlayer.OnPreparedListener,
case MediaPlayer.MEDIA_ERROR_NOT_VALID_FOR_PROGRESSIVE_PLAYBACK:
errorType = MEDIA_ERROR_NOT_VALID_FOR_PROGRESSIVE_PLAYBACK;
break;
+ case MediaPlayer.MEDIA_ERROR_SERVER_DIED:
+ errorType = MEDIA_ERROR_SERVER_DIED;
+ break;
default:
// There are some undocumented error codes for android media player.
// For example, when surfaceTexture got deleted before we setVideoSuface
diff --git a/media/base/android/media_player_android.h b/media/base/android/media_player_android.h
index a474c08..16982d6 100644
--- a/media/base/android/media_player_android.h
+++ b/media/base/android/media_player_android.h
@@ -36,6 +36,7 @@ class MEDIA_EXPORT MediaPlayerAndroid {
MEDIA_ERROR_DECODE,
MEDIA_ERROR_NOT_VALID_FOR_PROGRESSIVE_PLAYBACK,
MEDIA_ERROR_INVALID_CODE,
+ MEDIA_ERROR_SERVER_DIED,
};
static const double kDefaultVolumeMultiplier;
diff --git a/media/base/android/media_player_bridge.cc b/media/base/android/media_player_bridge.cc
index 5db0499..dc9d147 100644
--- a/media/base/android/media_player_bridge.cc
+++ b/media/base/android/media_player_bridge.cc
@@ -10,6 +10,7 @@
#include "base/android/jni_android.h"
#include "base/android/jni_string.h"
#include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h"
#include "jni/MediaPlayerBridge_jni.h"
#include "media/base/android/media_common_android.h"
@@ -23,6 +24,16 @@ using base::android::ScopedJavaLocalRef;
namespace media {
+namespace {
+
+enum UMAExitStatus {
+ UMA_EXIT_SUCCESS = 0,
+ UMA_EXIT_ERROR,
+ UMA_EXIT_STATUS_MAX = UMA_EXIT_ERROR,
+};
+
+} // namespace
+
MediaPlayerBridge::MediaPlayerBridge(
int player_id,
const GURL& url,
@@ -50,6 +61,9 @@ MediaPlayerBridge::MediaPlayerBridge(
can_seek_forward_(true),
can_seek_backward_(true),
allow_credentials_(allow_credentials),
+ is_active_(false),
+ has_error_(false),
+ has_ever_started_(false),
weak_factory_(this) {}
MediaPlayerBridge::~MediaPlayerBridge() {
@@ -59,6 +73,12 @@ MediaPlayerBridge::~MediaPlayerBridge() {
Java_MediaPlayerBridge_destroy(env, j_media_player_bridge_.obj());
}
Release();
+
+ if (has_ever_started_) {
+ UMA_HISTOGRAM_ENUMERATION("Media.Android.MediaPlayerSuccess",
+ has_error_ ? UMA_EXIT_ERROR : UMA_EXIT_SUCCESS,
+ UMA_EXIT_STATUS_MAX + 1);
+ }
}
void MediaPlayerBridge::Initialize() {
@@ -274,6 +294,17 @@ void MediaPlayerBridge::OnMediaMetadataExtracted(
}
void MediaPlayerBridge::Start() {
+ // A second Start() call after an error is considered another attempt for UMA
+ // and causes UMA reporting.
+ if (has_ever_started_ && has_error_) {
+ UMA_HISTOGRAM_ENUMERATION("Media.Android.MediaPlayerSuccess",
+ UMA_EXIT_ERROR, UMA_EXIT_STATUS_MAX + 1);
+ }
+
+ has_ever_started_ = true;
+ has_error_ = false;
+ is_active_ = true;
+
if (j_media_player_bridge_.is_null()) {
pending_play_ = true;
Prepare();
@@ -294,6 +325,8 @@ void MediaPlayerBridge::Pause(bool is_media_related_action) {
else
pending_play_ = false;
}
+
+ is_active_ = false;
}
bool MediaPlayerBridge::IsPlaying() {
@@ -364,6 +397,8 @@ base::TimeDelta MediaPlayerBridge::GetDuration() {
}
void MediaPlayerBridge::Release() {
+ is_active_ = false;
+
on_decoder_resources_released_cb_.Run(player_id());
if (j_media_player_bridge_.is_null())
return;
@@ -401,6 +436,22 @@ void MediaPlayerBridge::OnVideoSizeChanged(int width, int height) {
MediaPlayerAndroid::OnVideoSizeChanged(width, height);
}
+void MediaPlayerBridge::OnMediaError(int error_type) {
+ // Gather errors for UMA only in the active state.
+ // The MEDIA_ERROR_INVALID_CODE is reported by MediaPlayerListener.java in
+ // the situations that are considered normal, and is ignored by upper level.
+ if (is_active_ && error_type != MEDIA_ERROR_INVALID_CODE)
+ has_error_ = true;
+
+ // Do not propagate MEDIA_ERROR_SERVER_DIED. If it happens in the active state
+ // we want the playback to stall. It can be recovered by pressing the Play
+ // button again.
+ if (error_type == MEDIA_ERROR_SERVER_DIED)
+ error_type = MEDIA_ERROR_INVALID_CODE;
+
+ MediaPlayerAndroid::OnMediaError(error_type);
+}
+
void MediaPlayerBridge::OnPlaybackComplete() {
time_update_timer_.Stop();
MediaPlayerAndroid::OnPlaybackComplete();
diff --git a/media/base/android/media_player_bridge.h b/media/base/android/media_player_bridge.h
index 3f5b1a6..0652bc1 100644
--- a/media/base/android/media_player_bridge.h
+++ b/media/base/android/media_player_bridge.h
@@ -95,6 +95,7 @@ class MEDIA_EXPORT MediaPlayerBridge : public MediaPlayerAndroid {
// MediaPlayerAndroid implementation.
void OnVideoSizeChanged(int width, int height) override;
+ void OnMediaError(int error_type) override;
void OnPlaybackComplete() override;
void OnMediaInterrupted() override;
void OnMediaPrepared() override;
@@ -199,6 +200,19 @@ class MEDIA_EXPORT MediaPlayerBridge : public MediaPlayerAndroid {
// Whether user credentials are allowed to be passed.
bool allow_credentials_;
+ // Helper variables for UMA reporting.
+
+ // Whether the preparation for playback or the playback is currently going on.
+ // This flag is set in Start() and cleared in Pause() and Release(). Used for
+ // UMA reporting only.
+ bool is_active_;
+
+ // Whether there has been any errors in the active state.
+ bool has_error_;
+
+ // The flag is set if Start() has been called at least once.
+ bool has_ever_started_;
+
// NOTE: Weak pointers must be invalidated before all other member variables.
base::WeakPtrFactory<MediaPlayerBridge> weak_factory_;
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index c4a7d91..89ec16f 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -18835,6 +18835,11 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
</summary>
</histogram>
+<histogram name="Media.Android.MediaPlayerSuccess" enum="MediaPlayerExitStatus">
+ <owner>timav@chromium.org</owner>
+ <summary>Android: Whether MediaPlayer exited without errors.</summary>
+</histogram>
+
<histogram name="Media.Android.NumMediaServerCrashes">
<owner>qinmin@chromium.org</owner>
<summary>
@@ -72613,6 +72618,11 @@ To add a new entry, add it with any value and run test to compute valid value.
<int value="2" label="All external links protected"/>
</enum>
+<enum name="MediaPlayerExitStatus" type="int">
+ <int value="0" label="No errors"/>
+ <int value="1" label="Has errors"/>
+</enum>
+
<enum name="MediaRouteProviderWakeReason" type="int">
<int value="0" label="CreateRoute"/>
<int value="1" label="JoinRoute"/>
generated by cgit v1.1 at 2025-03-20 04:30:50 +0000