Skip to content

Commit

Permalink
[media] Add WMPI watch time histogram.
Browse files Browse the repository at this point in the history
This new histogram follows the structure of the MediaPlayer watch time
histogram, so that the two can be directly comparable. As a result it
does not record watch time when the rate is significantly larger than
1.

Bug: 985144
Change-Id: Id5a52cf12a015a2e2a232ccc89fc6e9a4ee6d84f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1745503
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686254}
  • Loading branch information
Dan Sanders authored and Commit Bot committed Aug 13, 2019
1 parent c25b198 commit 6edfd78
Show file tree
Hide file tree
Showing 11 changed files with 200 additions and 62 deletions.
2 changes: 2 additions & 0 deletions media/base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ jumbo_source_set("base") {
"silent_sink_suspender.h",
"simple_sync_token_client.cc",
"simple_sync_token_client.h",
"simple_watch_timer.cc",
"simple_watch_timer.h",
"sinc_resampler.cc",
"sinc_resampler.h",
"stream_parser.cc",
Expand Down
55 changes: 9 additions & 46 deletions media/base/android/media_player_bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ enum UMAExitStatus {

const double kDefaultVolume = 1.0;

constexpr base::TimeDelta kWatchTimeReportingInterval =
base::TimeDelta::FromMilliseconds(750);

const char kWatchTimeHistogram[] = "Media.Android.MediaPlayerWatchTime";

// These values are persisted to logs. Entries should not be renumbered and
Expand Down Expand Up @@ -90,8 +87,10 @@ MediaPlayerBridge::MediaPlayerBridge(const GURL& url,
has_error_(false),
has_ever_started_(false),
is_hls_(is_hls),
unreported_watch_time_ms_(0),
last_current_time_(kNoTimestamp),
watch_timer_(base::BindRepeating(&MediaPlayerBridge::OnWatchTimerTick,
base::Unretained(this)),
base::BindRepeating(&MediaPlayerBridge::GetCurrentTime,
base::Unretained(this))),
client_(client),
weak_factory_(this) {
listener_ = std::make_unique<MediaPlayerListener>(
Expand Down Expand Up @@ -354,7 +353,7 @@ base::TimeDelta MediaPlayerBridge::GetDuration() {
}

void MediaPlayerBridge::Release() {
StopWatchTimeTimer();
watch_timer_.Stop();
is_active_ = false;

if (j_media_player_bridge_.is_null())
Expand Down Expand Up @@ -477,11 +476,11 @@ void MediaPlayerBridge::UpdateAllowedOperations() {
void MediaPlayerBridge::StartInternal() {
JNIEnv* env = base::android::AttachCurrentThread();
Java_MediaPlayerBridge_start(env, j_media_player_bridge_);
StartWatchTimeTimer();
watch_timer_.Start();
}

void MediaPlayerBridge::PauseInternal() {
StopWatchTimeTimer();
watch_timer_.Stop();
JNIEnv* env = base::android::AttachCurrentThread();
Java_MediaPlayerBridge_pause(env, j_media_player_bridge_);
}
Expand Down Expand Up @@ -525,44 +524,8 @@ GURL MediaPlayerBridge::GetSiteForCookies() {
return site_for_cookies_;
}

void MediaPlayerBridge::StartWatchTimeTimer() {
if (watch_time_timer_.IsRunning())
return;

last_current_time_ = GetCurrentTime();
watch_time_timer_.Start(FROM_HERE, kWatchTimeReportingInterval, this,
&MediaPlayerBridge::UpdateWatchTime);
}

void MediaPlayerBridge::StopWatchTimeTimer() {
if (!watch_time_timer_.IsRunning())
return;

UpdateWatchTime();
watch_time_timer_.Stop();
}

void MediaPlayerBridge::UpdateWatchTime() {
if (!watch_time_timer_.IsRunning())
return;

// Note: this calculation assumes that the playback rate is 1.0, which
// currently it always is.
base::TimeDelta current_time = GetCurrentTime();
base::TimeDelta duration = current_time - last_current_time_;
last_current_time_ = current_time;

// Discard crazy values.
if (duration > base::TimeDelta() &&
duration < kWatchTimeReportingInterval * 2) {
unreported_watch_time_ms_ += duration.InMilliseconds();
}

// Report to the nearest 1s.
if (unreported_watch_time_ms_ >= 500) {
RecordWatchTimeUMA(is_hls_, height_ > 0);
unreported_watch_time_ms_ -= 1000;
}
void MediaPlayerBridge::OnWatchTimerTick() {
RecordWatchTimeUMA(is_hls_, height_ > 0);
}

} // namespace media
10 changes: 3 additions & 7 deletions media/base/android/media_player_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/timer/timer.h"
#include "media/base/android/media_player_listener.h"
#include "media/base/media_export.h"
#include "media/base/simple_watch_timer.h"
#include "ui/gl/android/scoped_java_surface.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -189,10 +190,7 @@ class MEDIA_EXPORT MediaPlayerBridge {
// Sets the underlying MediaPlayer's volume.
void UpdateVolumeInternal();

// Watch time reporting.
void StartWatchTimeTimer();
void StopWatchTimeTimer();
void UpdateWatchTime();
void OnWatchTimerTick();

base::WeakPtr<MediaPlayerBridge> WeakPtrForUIThread();

Expand Down Expand Up @@ -256,9 +254,7 @@ class MEDIA_EXPORT MediaPlayerBridge {

// State for watch time reporting.
bool is_hls_;
int unreported_watch_time_ms_;
base::TimeDelta last_current_time_;
base::RepeatingTimer watch_time_timer_;
SimpleWatchTimer watch_timer_;

// A reference to the owner of |this|.
Client* client_;
Expand Down
11 changes: 8 additions & 3 deletions media/base/renderer_factory_selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ void RendererFactorySelector::SetBaseFactoryType(FactoryType type) {
base_factory_type_ = type;
}

RendererFactory* RendererFactorySelector::GetCurrentFactory() {
RendererFactorySelector::FactoryType
RendererFactorySelector::GetCurrentFactoryType() {
DCHECK(base_factory_type_);
FactoryType next_factory_type = base_factory_type_.value();

Expand All @@ -38,10 +39,14 @@ RendererFactory* RendererFactorySelector::GetCurrentFactory() {
if (query_is_flinging_active_cb_ && query_is_flinging_active_cb_.Run())
next_factory_type = FactoryType::FLINGING;

DVLOG(1) << __func__ << " Selecting factory type: " << next_factory_type;
return next_factory_type;
}

RendererFactory* current_factory = factories_[next_factory_type].get();
RendererFactory* RendererFactorySelector::GetCurrentFactory() {
FactoryType next_factory_type = GetCurrentFactoryType();

DVLOG(1) << __func__ << " Selecting factory type: " << next_factory_type;
RendererFactory* current_factory = factories_[next_factory_type].get();
DCHECK(current_factory);

return current_factory;
Expand Down
16 changes: 11 additions & 5 deletions media/base/renderer_factory_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ class MEDIA_EXPORT RendererFactorySelector {
using QueryIsRemotingActiveCB = base::Callback<bool()>;
using QueryIsFlingingActiveCB = base::Callback<bool()>;

// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum FactoryType {
DEFAULT, // DefaultRendererFactory.
MOJO, // MojoRendererFactory.
MEDIA_PLAYER, // MediaPlayerRendererClientFactory.
COURIER, // CourierRendererFactory.
FLINGING, // FlingingRendererClientFactory
DEFAULT = 0, // DefaultRendererFactory.
MOJO = 1, // MojoRendererFactory.
MEDIA_PLAYER = 2, // MediaPlayerRendererClientFactory.
COURIER = 3, // CourierRendererFactory.
FLINGING = 4, // FlingingRendererClientFactory
FACTORY_TYPE_MAX = FLINGING,
};

Expand All @@ -43,6 +45,10 @@ class MEDIA_EXPORT RendererFactorySelector {
// be used by default.
void SetBaseFactoryType(FactoryType type);

// Returns the type of the factory that GetCurrentFactory() would return.
// NOTE: SetBaseFactoryType() must be called before calling this method.
FactoryType GetCurrentFactoryType();

// Updates |current_factory_| if necessary, and returns its value.
// NOTE: SetBaseFactoryType() must be called before calling this method.
RendererFactory* GetCurrentFactory();
Expand Down
61 changes: 61 additions & 0 deletions media/base/simple_watch_timer.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "media/base/simple_watch_timer.h"

#include "base/location.h"

namespace media {

namespace {

constexpr base::TimeDelta kQueryInterval =
base::TimeDelta::FromMilliseconds(750);

} // namespace

SimpleWatchTimer::SimpleWatchTimer(TickCB tick_cb,
GetCurrentTimeCB get_current_time_cb)
: tick_cb_(std::move(tick_cb)),
get_current_time_cb_(std::move(get_current_time_cb)) {
DCHECK(!tick_cb_.is_null());
DCHECK(!get_current_time_cb_.is_null());
}

SimpleWatchTimer::~SimpleWatchTimer() {}

void SimpleWatchTimer::Start() {
if (timer_.IsRunning())
return;

last_current_time_ = get_current_time_cb_.Run();
timer_.Start(FROM_HERE, kQueryInterval, this, &SimpleWatchTimer::Tick);
}

void SimpleWatchTimer::Stop() {
if (!timer_.IsRunning())
return;

timer_.Stop();
Tick();
}

void SimpleWatchTimer::Tick() {
base::TimeDelta current_time = get_current_time_cb_.Run();
base::TimeDelta duration = current_time - last_current_time_;
last_current_time_ = current_time;

// Accumulate watch time if the duration is reasonable.
if (duration > base::TimeDelta() && duration < kQueryInterval * 2) {
unreported_ms_ += duration.InMilliseconds();
}

// Tick if the accumulated time is about a second.
if (unreported_ms_ >= 500) {
unreported_ms_ -= 1000;
tick_cb_.Run();
}
}

} // namespace media
56 changes: 56 additions & 0 deletions media/base/simple_watch_timer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef MEDIA_BASE_SIMPLE_WATCH_TIMER_H_
#define MEDIA_BASE_SIMPLE_WATCH_TIMER_H_

#include "base/callback_forward.h"
#include "base/macros.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "media/base/media_export.h"

namespace media {

// SimpleWatchTimer aids in recording UMA counts that accumulate media watch
// time in seconds. It will fire its callback about once per second during
// active playback.
//
// Active playback is a duration after Start() and before Stop() in
// which current time progresses. Large jumps in current time are not considered
// to be progress; they are assumed to be seeks or media errors.
//
// Start() and Stop() may be called repeatedly. It is recommended to call Stop()
// before destructing a SimpleWatchTimer so that |tick_cb| can be fired at an
// opportune time.
//
// Note: SimpleWatchTimer does not understand playbackRate and will discard
// durations with high rates.
class MEDIA_EXPORT SimpleWatchTimer {
public:
using TickCB = base::RepeatingClosure;
using GetCurrentTimeCB = base::RepeatingCallback<base::TimeDelta()>;

SimpleWatchTimer(TickCB tick_cb, GetCurrentTimeCB get_current_time_cb);
~SimpleWatchTimer();

void Start();
void Stop();

private:
void Tick();

TickCB tick_cb_;
GetCurrentTimeCB get_current_time_cb_;

int unreported_ms_ = 0;
base::TimeDelta last_current_time_;
base::RepeatingTimer timer_;

DISALLOW_COPY_AND_ASSIGN(SimpleWatchTimer);
};

} // namespace media

#endif // MEDIA_BASE_SIMPLE_WATCH_TIMER_H_
24 changes: 23 additions & 1 deletion media/blink/webmediaplayer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ namespace media {

namespace {

const char kWatchTimeHistogram[] = "Media.WebMediaPlayerImpl.WatchTime";

void RecordSimpleWatchTimeUMA(RendererFactorySelector::FactoryType type) {
UMA_HISTOGRAM_ENUMERATION(kWatchTimeHistogram, type,
RendererFactorySelector::FACTORY_TYPE_MAX + 1);
}

void SetSinkIdOnMediaThread(
scoped_refptr<blink::WebAudioSourceProviderImpl> sink,
const std::string& device_id,
Expand Down Expand Up @@ -314,7 +321,13 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
is_background_video_playback_enabled_(
params->IsBackgroundVideoPlaybackEnabled()),
is_background_video_track_optimization_supported_(
params->IsBackgroundVideoTrackOptimizationSupported()) {
params->IsBackgroundVideoTrackOptimizationSupported()),
reported_renderer_type_(RendererFactorySelector::DEFAULT),
simple_watch_timer_(
base::BindRepeating(&WebMediaPlayerImpl::OnSimpleWatchTimerTick,
base::Unretained(this)),
base::BindRepeating(&WebMediaPlayerImpl::GetCurrentTimeInternal,
base::Unretained(this))) {
DVLOG(1) << __func__;
DCHECK(adjust_allocated_memory_cb_);
DCHECK(renderer_factory_selector_);
Expand Down Expand Up @@ -444,6 +457,7 @@ WebMediaPlayerImpl::~WebMediaPlayerImpl() {
if (!surface_layer_for_video_enabled_ && video_layer_)
video_layer_->StopUsingProvider();

simple_watch_timer_.Stop();
media_log_->AddEvent(
media_log_->CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_DESTROYED));

Expand Down Expand Up @@ -770,6 +784,7 @@ void WebMediaPlayerImpl::Play() {
if (video_decode_stats_reporter_)
video_decode_stats_reporter_->OnPlaying();

simple_watch_timer_.Start();
media_metrics_provider_->SetHasPlayed();
media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::PLAY));

Expand Down Expand Up @@ -808,6 +823,7 @@ void WebMediaPlayerImpl::Pause() {
if (video_decode_stats_reporter_)
video_decode_stats_reporter_->OnPaused();

simple_watch_timer_.Stop();
media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::PAUSE));

// Paused changed so we should update media position state.
Expand Down Expand Up @@ -1762,6 +1778,7 @@ void WebMediaPlayerImpl::OnError(PipelineStatus status) {

MaybeSetContainerName();
ReportPipelineError(load_type_, status, media_log_.get());
simple_watch_timer_.Stop();
media_log_->AddEvent(media_log_->CreatePipelineErrorEvent(status));
media_metrics_provider_->OnError(status);
if (watch_time_reporter_)
Expand Down Expand Up @@ -2611,6 +2628,7 @@ std::unique_ptr<Renderer> WebMediaPlayerImpl::CreateRenderer() {
request_overlay_info_cb = BindToCurrentLoop(
base::Bind(&WebMediaPlayerImpl::OnOverlayInfoRequested, weak_this_));
#endif
reported_renderer_type_ = renderer_factory_selector_->GetCurrentFactoryType();
return renderer_factory_selector_->GetCurrentFactory()->CreateRenderer(
media_task_runner_, worker_task_runner_, audio_source_provider_.get(),
compositor_.get(), request_overlay_info_cb, client_->TargetColorSpace());
Expand Down Expand Up @@ -3592,4 +3610,8 @@ void WebMediaPlayerImpl::MaybeUpdateBufferSizesForPlayback() {
UpdateMediaPositionState();
}

void WebMediaPlayerImpl::OnSimpleWatchTimerTick() {
RecordSimpleWatchTimeUMA(reported_renderer_type_);
}

} // namespace media

0 comments on commit 6edfd78

Please sign in to comment.