Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1100)

Unified Diff: media/blink/watch_time_reporter.cc

Issue 2780533004: Start recording background video watch time. (Closed)
Patch Set: Add moar tests. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: media/blink/watch_time_reporter.cc
diff --git a/media/blink/watch_time_reporter.cc b/media/blink/watch_time_reporter.cc
index 06e97646739b1c5b4db09cca46756447e74a3394..9aec6f2514de7dbe4f217a7391c560c5a9b11687 100644
--- a/media/blink/watch_time_reporter.cc
+++ b/media/blink/watch_time_reporter.cc
@@ -30,6 +30,25 @@ WatchTimeReporter::WatchTimeReporter(bool has_audio,
scoped_refptr<MediaLog> media_log,
const gfx::Size& initial_video_size,
const GetMediaTimeCB& get_media_time_cb)
+ : WatchTimeReporter(has_audio,
+ has_video,
+ is_mse,
+ is_encrypted,
+ is_embedded_media_experience_enabled,
+ std::move(media_log),
+ initial_video_size,
+ get_media_time_cb,
+ false) {}
+
+WatchTimeReporter::WatchTimeReporter(bool has_audio,
+ bool has_video,
+ bool is_mse,
+ bool is_encrypted,
+ bool is_embedded_media_experience_enabled,
+ scoped_refptr<MediaLog> media_log,
+ const gfx::Size& initial_video_size,
+ const GetMediaTimeCB& get_media_time_cb,
+ bool is_background)
: has_audio_(has_audio),
has_video_(has_video),
is_mse_(is_mse),
@@ -38,15 +57,34 @@ WatchTimeReporter::WatchTimeReporter(bool has_audio,
is_embedded_media_experience_enabled),
media_log_(std::move(media_log)),
initial_video_size_(initial_video_size),
- get_media_time_cb_(get_media_time_cb) {
+ get_media_time_cb_(get_media_time_cb),
+ is_background_(is_background) {
DCHECK(!get_media_time_cb_.is_null());
DCHECK(has_audio_ || has_video_);
if (base::PowerMonitor* pm = base::PowerMonitor::Get())
pm->AddObserver(this);
+
+ if (is_background_) {
+ DCHECK(has_audio_);
+ DCHECK(!has_video_);
+ return;
+ }
+
+ // Background watch time is reported by creating an audio only watch time
+ // reporter which receives play when hidden and pause when shown. This avoids
+ // unnecessary complexity inside the UpdateWatchTime() for handling this case.
+ if (has_video_ && has_audio_ && ShouldReportWatchTime()) {
+ background_reporter_.reset(
+ new WatchTimeReporter(has_audio_, false, is_mse_, is_encrypted_,
+ is_embedded_media_experience_enabled_, media_log_,
+ initial_video_size_, get_media_time_cb_, true));
+ }
}
WatchTimeReporter::~WatchTimeReporter() {
+ background_reporter_.reset();
+
// If the timer is still running, finalize immediately, this is our last
// chance to capture metrics.
if (reporting_timer_.IsRunning())
@@ -57,16 +95,25 @@ WatchTimeReporter::~WatchTimeReporter() {
}
void WatchTimeReporter::OnPlaying() {
+ if (background_reporter_ && !is_visible_)
+ background_reporter_->OnPlaying();
+
is_playing_ = true;
MaybeStartReportingTimer(get_media_time_cb_.Run());
}
void WatchTimeReporter::OnPaused() {
+ if (background_reporter_)
+ background_reporter_->OnPaused();
+
is_playing_ = false;
MaybeFinalizeWatchTime(FinalizeTime::ON_NEXT_UPDATE);
}
void WatchTimeReporter::OnSeeking() {
+ if (background_reporter_)
+ background_reporter_->OnSeeking();
+
if (!reporting_timer_.IsRunning())
return;
@@ -80,6 +127,9 @@ void WatchTimeReporter::OnSeeking() {
}
void WatchTimeReporter::OnVolumeChange(double volume) {
+ if (background_reporter_)
+ background_reporter_->OnVolumeChange(volume);
+
const double old_volume = volume_;
volume_ = volume;
@@ -91,6 +141,9 @@ void WatchTimeReporter::OnVolumeChange(double volume) {
}
void WatchTimeReporter::OnShown() {
+ if (background_reporter_)
+ background_reporter_->OnPaused();
+
if (!has_video_)
return;
@@ -99,6 +152,9 @@ void WatchTimeReporter::OnShown() {
}
void WatchTimeReporter::OnHidden() {
+ if (background_reporter_ && is_playing_)
+ background_reporter_->OnPlaying();
+
if (!has_video_)
return;
@@ -197,6 +253,8 @@ void WatchTimeReporter::UpdateWatchTime() {
is_finalizing ? end_timestamp_ : get_media_time_cb_.Run();
const base::TimeDelta elapsed = current_timestamp - start_timestamp_;
+ std::unique_ptr<MediaLogEvent> log_event;
+
// Only report watch time after some minimum amount has elapsed. Don't update
// watch time if media time hasn't changed since the last run; this may occur
// if a seek is taking some time to complete or the playback is stalled for
@@ -204,38 +262,29 @@ void WatchTimeReporter::UpdateWatchTime() {
if (elapsed >= kMinimumElapsedWatchTime &&
last_media_timestamp_ != current_timestamp) {
last_media_timestamp_ = current_timestamp;
-
- std::unique_ptr<MediaLogEvent> log_event =
- media_log_->CreateEvent(MediaLogEvent::Type::WATCH_TIME_UPDATE);
-
- log_event->params.SetDoubleWithoutPathExpansion(
- has_video_ ? MediaLog::kWatchTimeAudioVideoAll
- : MediaLog::kWatchTimeAudioAll,
- elapsed.InSecondsF());
- if (is_mse_) {
- log_event->params.SetDoubleWithoutPathExpansion(
- has_video_ ? MediaLog::kWatchTimeAudioVideoMse
- : MediaLog::kWatchTimeAudioMse,
- elapsed.InSecondsF());
- } else {
- log_event->params.SetDoubleWithoutPathExpansion(
- has_video_ ? MediaLog::kWatchTimeAudioVideoSrc
- : MediaLog::kWatchTimeAudioSrc,
- elapsed.InSecondsF());
- }
- if (is_encrypted_) {
- log_event->params.SetDoubleWithoutPathExpansion(
- has_video_ ? MediaLog::kWatchTimeAudioVideoEme
- : MediaLog::kWatchTimeAudioEme,
- elapsed.InSecondsF());
- }
-
- if (is_embedded_media_experience_enabled_) {
- log_event->params.SetDoubleWithoutPathExpansion(
- has_video_ ? MediaLog::kWatchTimeAudioVideoEmbeddedExperience
- : MediaLog::kWatchTimeAudioEmbeddedExperience,
- elapsed.InSecondsF());
- }
+ log_event = media_log_->CreateEvent(MediaLogEvent::Type::WATCH_TIME_UPDATE);
+
+#define RECORD_WATCH_TIME(key, value) \
+ do { \
+ log_event->params.SetDoubleWithoutPathExpansion( \
+ has_video_ \
+ ? MediaLog::kWatchTimeAudioVideo##key \
+ : (is_background_ ? MediaLog::kWatchTimeAudioVideoBackground##key \
+ : MediaLog::kWatchTimeAudio##key), \
+ value.InSecondsF()); \
+ } while (0)
+
+ RECORD_WATCH_TIME(All, elapsed);
+ if (is_mse_)
+ RECORD_WATCH_TIME(Mse, elapsed);
+ else
+ RECORD_WATCH_TIME(Src, elapsed);
+
+ if (is_encrypted_)
+ RECORD_WATCH_TIME(Eme, elapsed);
+
+ if (is_embedded_media_experience_enabled_)
+ RECORD_WATCH_TIME(EmbeddedExperience, elapsed);
// Record watch time using the last known value for |is_on_battery_power_|;
// if there's a |pending_power_change_| use that to accurately finalize the
@@ -248,27 +297,26 @@ void WatchTimeReporter::UpdateWatchTime() {
// Again, only update watch time if enough time has elapsed; we need to
// recheck the elapsed time here since the power source can change anytime.
if (elapsed_power >= kMinimumElapsedWatchTime) {
- if (is_on_battery_power_) {
- log_event->params.SetDoubleWithoutPathExpansion(
- has_video_ ? MediaLog::kWatchTimeAudioVideoBattery
- : MediaLog::kWatchTimeAudioBattery,
- elapsed_power.InSecondsF());
- } else {
- log_event->params.SetDoubleWithoutPathExpansion(
- has_video_ ? MediaLog::kWatchTimeAudioVideoAc
- : MediaLog::kWatchTimeAudioAc,
- elapsed_power.InSecondsF());
- }
+ if (is_on_battery_power_)
+ RECORD_WATCH_TIME(Battery, elapsed_power);
+ else
+ RECORD_WATCH_TIME(Ac, elapsed_power);
}
- if (is_finalizing)
- log_event->params.SetBoolean(MediaLog::kWatchTimeFinalize, true);
- else if (is_power_change_pending)
- log_event->params.SetBoolean(MediaLog::kWatchTimeFinalizePower, true);
+#undef RECORD_WATCH_TIME
+ }
- DVLOG(2) << "Sending watch time update.";
+ // If we didn't report a new watch time, but need to finalize, create event.
+ if (!log_event && (is_finalizing || is_power_change_pending))
+ log_event = media_log_->CreateEvent(MediaLogEvent::Type::WATCH_TIME_UPDATE);
+
+ if (is_finalizing)
+ log_event->params.SetBoolean(MediaLog::kWatchTimeFinalize, true);
+ else if (is_power_change_pending)
+ log_event->params.SetBoolean(MediaLog::kWatchTimeFinalizePower, true);
+
+ if (log_event)
media_log_->AddEvent(std::move(log_event));
- }
if (is_power_change_pending) {
// Invert battery power status here instead of using the value returned by

Powered by Google App Engine
This is Rietveld 408576698