Skip to content

Commit

Permalink
Merge M50: "Don't log PipelineStatus when there is no Pipeline..."
Browse files Browse the repository at this point in the history
When MSE started logging these events we accidentally started
getting them for the Android MediaSource players.  These metrics
should only be recorded for Pipeline based players.

BUG=589717
TEST=Android MediaSourcePlayer doesn't generate UMA.

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

Cr-Commit-Position: refs/heads/master@{#378083}
(cherry picked from commit a14620d)

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

Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#6}
Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
  • Loading branch information
dalecurtis committed Feb 29, 2016
1 parent 3d66147 commit 73024c2
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions content/browser/media/media_internals.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,20 +281,15 @@ class MediaInternals::MediaInternalsUMAHandler {

private:
struct PipelineInfo {
media::PipelineStatus last_pipeline_status;
bool has_audio;
bool has_video;
bool video_dds;
bool video_decoder_changed;
bool has_pipeline = false;
media::PipelineStatus last_pipeline_status = media::PIPELINE_OK;
bool has_audio = false;
bool has_video = false;
bool video_dds = false;
bool video_decoder_changed = false;
std::string audio_codec_name;
std::string video_codec_name;
std::string video_decoder;
PipelineInfo()
: last_pipeline_status(media::PIPELINE_OK),
has_audio(false),
has_video(false),
video_dds(false),
video_decoder_changed(false) {}
};

// Helper function to report PipelineStatus associated with a player to UMA.
Expand Down Expand Up @@ -324,6 +319,10 @@ void MediaInternals::MediaInternalsUMAHandler::SavePlayerState(
DCHECK_CURRENTLY_ON(BrowserThread::UI);
PlayerInfoMap& player_info = renderer_info_[render_process_id];
switch (event.type) {
case media::MediaLogEvent::PIPELINE_STATE_CHANGED: {
player_info[event.id].has_pipeline = true;
break;
}
case media::MediaLogEvent::PIPELINE_ERROR: {
int status;
event.params.GetInteger("pipeline_error", &status);
Expand Down Expand Up @@ -404,6 +403,12 @@ std::string MediaInternals::MediaInternalsUMAHandler::GetUMANameForAVStream(
void MediaInternals::MediaInternalsUMAHandler::ReportUMAForPipelineStatus(
const PipelineInfo& player_info) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);

// Don't log pipeline status for players which don't actually have a pipeline;
// e.g., the Android MediaSourcePlayer implementation.
if (!player_info.has_pipeline)
return;

if (player_info.has_video && player_info.has_audio) {
base::LinearHistogram::FactoryGet(
GetUMANameForAVStream(player_info), 1, media::PIPELINE_STATUS_MAX,
Expand Down

0 comments on commit 73024c2

Please sign in to comment.