Commit fc23b592 authored by chcunningham's avatar chcunningham Committed by Commit Bot

Expire old stats from the VideoDecodeStatsDB

Metrics show that the DB makes its best predictions for a given key
after accumulating ~2500 frames. With this change rows in the DB will
begin to down-weight the existing frames after reaching the 2500 frame
max. New appends of more than 2500 frames will completely flush out old
stats. This roughly approximates a sliding window of the last 2500
frames without adding complexity to the DB of a real sliding window.

This change also expires old statistics that have not received an
update in the last 30 days. This ensures that clients who have a bad
outlier decode performance aren't punished forever - otherwise sites
may never give them another chance at that resolution / frame-rate.

Change-Id: I82f6920fbe3f594dad36d35b12c55d32e65de5a6
Bug: 818889
Test: New unit tests
Reviewed-on: https://chromium-review.googlesource.com/1113503
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: default avatarJoshua Bell <jsbell@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594427}
parent d6c58bdd
......@@ -526,7 +526,7 @@ IN_PROC_BROWSER_TEST_F(BrowsingDataRemoverBrowserTest, VideoDecodePerfHistory) {
media::mojom::PredictionTargets prediction_targets;
prediction_targets.frames_decoded = kFramesDecoded;
prediction_targets.frames_dropped = kFramesDropped;
prediction_targets.frames_decoded_power_efficient = kFramesPowerEfficient;
prediction_targets.frames_power_efficient = kFramesPowerEfficient;
{
base::RunLoop run_loop;
......
......@@ -352,8 +352,8 @@ void VideoDecodeStatsReporter::UpdateStats() {
DVLOG(2) << __func__ << " Recording -- dropped:" << targets->frames_dropped
<< "/" << targets->frames_decoded
<< " power efficient:" << targets->frames_decoded_power_efficient
<< "/" << targets->frames_decoded;
<< " power efficient:" << targets->frames_power_efficient << "/"
<< targets->frames_decoded;
recorder_ptr_->UpdateRecord(std::move(targets));
}
......
......@@ -84,13 +84,13 @@ class RecordInterceptor : public mojom::VideoDecodeStatsRecorder {
void UpdateRecord(mojom::PredictionTargetsPtr targets) override {
MockUpdateRecord(targets->frames_decoded, targets->frames_dropped,
targets->frames_decoded_power_efficient);
targets->frames_power_efficient);
}
MOCK_METHOD3(MockUpdateRecord,
void(uint32_t frames_decoded,
uint32_t frames_dropped,
uint32_t frames_decoded_power_efficient));
uint32_t frames_power_efficient));
MOCK_METHOD0(FinalizeRecord, void());
};
......
......@@ -13,9 +13,17 @@ package media;
message DecodeStatsProto {
// Required. Count of decoded video frames.
optional uint64 frames_decoded = 1;
// Required. Count of dropped video frames. Should not exceed
// |frames_decoded|.
optional uint64 frames_dropped = 2;
// Required. Count of power efficiently decoded frames.
optional uint64 frames_decoded_power_efficient = 3 [default = 0];
}
optional uint64 frames_power_efficient = 3;
// Required. Time of last data write from base::Time::ToJsTime(). Data will be
// discarded when the date indicates its very old. This avoids a circumstance
// where a few bad outlier playbacks permanently define a machines
// capabilities.
optional double last_write_date = 7;
}
\ No newline at end of file
......@@ -39,37 +39,37 @@ std::string VideoDecodeStatsDB::VideoDescKey::ToLogString() const {
VideoDecodeStatsDB::DecodeStatsEntry::DecodeStatsEntry(
uint64_t frames_decoded,
uint64_t frames_dropped,
uint64_t frames_decoded_power_efficient)
uint64_t frames_power_efficient)
: frames_decoded(frames_decoded),
frames_dropped(frames_dropped),
frames_decoded_power_efficient(frames_decoded_power_efficient) {
frames_power_efficient(frames_power_efficient) {
DCHECK_GE(frames_decoded, 0u);
DCHECK_GE(frames_dropped, 0u);
DCHECK_GE(frames_decoded_power_efficient, 0u);
DCHECK_GE(frames_power_efficient, 0u);
}
VideoDecodeStatsDB::DecodeStatsEntry::DecodeStatsEntry(
const DecodeStatsEntry& entry)
: frames_decoded(entry.frames_decoded),
frames_dropped(entry.frames_dropped),
frames_decoded_power_efficient(entry.frames_decoded_power_efficient) {}
frames_power_efficient(entry.frames_power_efficient) {}
std::string VideoDecodeStatsDB::DecodeStatsEntry::ToLogString() const {
return base::StringPrintf(
"DecodeStatsEntry {frames decoded:%" PRIu64 ", dropped:%" PRIu64
", power efficient decoded:%" PRIu64 "}",
frames_decoded, frames_dropped, frames_decoded_power_efficient);
", power efficient:%" PRIu64 "}",
frames_decoded, frames_dropped, frames_power_efficient);
}
VideoDecodeStatsDB::DecodeStatsEntry& VideoDecodeStatsDB::DecodeStatsEntry::
operator+=(const DecodeStatsEntry& right) {
DCHECK_GE(right.frames_decoded, 0u);
DCHECK_GE(right.frames_dropped, 0u);
DCHECK_GE(right.frames_decoded_power_efficient, 0u);
DCHECK_GE(right.frames_power_efficient, 0u);
frames_decoded += right.frames_decoded;
frames_dropped += right.frames_dropped;
frames_decoded_power_efficient += right.frames_decoded_power_efficient;
frames_power_efficient += right.frames_power_efficient;
return *this;
}
......@@ -87,7 +87,7 @@ bool operator==(const VideoDecodeStatsDB::DecodeStatsEntry& x,
const VideoDecodeStatsDB::DecodeStatsEntry& y) {
return x.frames_decoded == y.frames_decoded &&
x.frames_dropped == y.frames_dropped &&
x.frames_decoded_power_efficient == y.frames_decoded_power_efficient;
x.frames_power_efficient == y.frames_power_efficient;
}
bool operator!=(const VideoDecodeStatsDB::DecodeStatsEntry& x,
......
......@@ -51,7 +51,7 @@ class MEDIA_EXPORT VideoDecodeStatsDB {
struct MEDIA_EXPORT DecodeStatsEntry {
DecodeStatsEntry(uint64_t frames_decoded,
uint64_t frames_dropped,
uint64_t frames_decoded_power_efficient);
uint64_t frames_power_efficient);
DecodeStatsEntry(const DecodeStatsEntry& entry);
// Add stats from |right| to |this| entry.
......@@ -63,7 +63,7 @@ class MEDIA_EXPORT VideoDecodeStatsDB {
// Note: operator == and != are defined outside this class.
uint64_t frames_decoded;
uint64_t frames_dropped;
uint64_t frames_decoded_power_efficient;
uint64_t frames_power_efficient;
};
virtual ~VideoDecodeStatsDB();
......
......@@ -13,6 +13,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/sequence_checker.h"
#include "base/task/post_task.h"
#include "base/time/default_clock.h"
#include "components/leveldb_proto/proto_database_impl.h"
#include "media/capabilities/video_decode_stats.pb.h"
......@@ -43,10 +44,19 @@ std::unique_ptr<VideoDecodeStatsDBImpl> VideoDecodeStatsDBImpl::Create(
new VideoDecodeStatsDBImpl(std::move(proto_db), db_dir));
}
constexpr char VideoDecodeStatsDBImpl::kDefaultWriteTime[];
VideoDecodeStatsDBImpl::VideoDecodeStatsDBImpl(
std::unique_ptr<leveldb_proto::ProtoDatabase<DecodeStatsProto>> db,
const base::FilePath& db_dir)
: db_(std::move(db)), db_dir_(db_dir), weak_ptr_factory_(this) {
: db_(std::move(db)),
db_dir_(db_dir),
wall_clock_(base::DefaultClock::GetInstance()),
weak_ptr_factory_(this) {
bool time_parsed =
base::Time::FromString(kDefaultWriteTime, &default_write_time_);
DCHECK(time_parsed);
DCHECK(db_);
DCHECK(!db_dir_.empty());
}
......@@ -118,12 +128,25 @@ void VideoDecodeStatsDBImpl::GetDecodeStats(const VideoDescKey& key,
weak_ptr_factory_.GetWeakPtr(), std::move(get_stats_cb)));
}
bool VideoDecodeStatsDBImpl::AreStatsExpired(
const DecodeStatsProto* const stats_proto) {
double last_write_date = stats_proto->last_write_date();
if (last_write_date == 0) {
// Set a default time if the write date is zero (no write since proto was
// updated to include the time stamp).
last_write_date = default_write_time_.ToJsTime();
}
return wall_clock_->Now() - base::Time::FromJsTime(last_write_date) >
base::TimeDelta::FromDays(VideoDecodeStatsDBImpl::kMaxDaysToKeepStats);
}
void VideoDecodeStatsDBImpl::WriteUpdatedEntry(
const VideoDescKey& key,
const DecodeStatsEntry& entry,
const DecodeStatsEntry& new_entry,
AppendDecodeStatsCB append_done_cb,
bool read_success,
std::unique_ptr<DecodeStatsProto> prev_stats_proto) {
std::unique_ptr<DecodeStatsProto> stats_proto) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(IsInitialized());
......@@ -138,37 +161,63 @@ void VideoDecodeStatsDBImpl::WriteUpdatedEntry(
return;
}
if (!prev_stats_proto) {
prev_stats_proto.reset(new DecodeStatsProto());
prev_stats_proto->set_frames_decoded(0);
prev_stats_proto->set_frames_dropped(0);
prev_stats_proto->set_frames_decoded_power_efficient(0);
if (!stats_proto || AreStatsExpired(stats_proto.get())) {
// Default instance will have all zeros for numeric types.
stats_proto.reset(new DecodeStatsProto());
}
uint64_t sum_frames_decoded =
prev_stats_proto->frames_decoded() + entry.frames_decoded;
uint64_t sum_frames_dropped =
prev_stats_proto->frames_dropped() + entry.frames_dropped;
uint64_t sum_frames_decoded_power_efficient =
prev_stats_proto->frames_decoded_power_efficient() +
entry.frames_decoded_power_efficient;
prev_stats_proto->set_frames_decoded(sum_frames_decoded);
prev_stats_proto->set_frames_dropped(sum_frames_dropped);
prev_stats_proto->set_frames_decoded_power_efficient(
sum_frames_decoded_power_efficient);
DVLOG(3) << __func__ << " Updating " << key.ToLogString() << " with "
<< entry.ToLogString() << " aggregate:"
<< DecodeStatsEntry(sum_frames_decoded, sum_frames_dropped,
sum_frames_decoded_power_efficient)
.ToLogString();
std::unique_ptr<ProtoDecodeStatsEntry::KeyEntryVector> entries =
std::make_unique<ProtoDecodeStatsEntry::KeyEntryVector>();
uint64_t old_frames_decoded = stats_proto->frames_decoded();
uint64_t old_frames_dropped = stats_proto->frames_dropped();
uint64_t old_frames_power_efficient = stats_proto->frames_power_efficient();
if (old_frames_decoded + new_entry.frames_decoded > kMaxFramesPerBuffer) {
// The |new_entry| is pushing out some or all of the old data. Achieve this
// by weighting the dropped and power efficiency stats by the ratio of the
// the buffer that new entry fills.
double fill_ratio = std::min(
static_cast<double>(new_entry.frames_decoded) / kMaxFramesPerBuffer,
1.0);
double old_dropped_ratio =
static_cast<double>(old_frames_dropped) / old_frames_decoded;
double old_efficient_ratio =
static_cast<double>(old_frames_power_efficient) / old_frames_decoded;
double new_entry_dropped_ratio =
static_cast<double>(new_entry.frames_dropped) /
new_entry.frames_decoded;
double new_entry_efficient_ratio =
static_cast<double>(new_entry.frames_power_efficient) /
new_entry.frames_decoded;
double agg_dropped_ratio = fill_ratio * new_entry_dropped_ratio +
(1 - fill_ratio) * old_dropped_ratio;
double agg_efficient_ratio = fill_ratio * new_entry_efficient_ratio +
(1 - fill_ratio) * old_efficient_ratio;
stats_proto->set_frames_decoded(kMaxFramesPerBuffer);
stats_proto->set_frames_dropped(
std::round(agg_dropped_ratio * kMaxFramesPerBuffer));
stats_proto->set_frames_power_efficient(
std::round(agg_efficient_ratio * kMaxFramesPerBuffer));
} else {
// Adding |new_entry| does not exceed |kMaxFramesPerfBuffer|. Simply sum the
// stats.
stats_proto->set_frames_decoded(new_entry.frames_decoded +
old_frames_decoded);
stats_proto->set_frames_dropped(new_entry.frames_dropped +
old_frames_dropped);
stats_proto->set_frames_power_efficient(new_entry.frames_power_efficient +
old_frames_power_efficient);
}
entries->emplace_back(key.Serialize(), *prev_stats_proto);
// Update the time stamp for the current write.
stats_proto->set_last_write_date(wall_clock_->Now().ToJsTime());
// Push the update to the DB.
using DBType = leveldb_proto::ProtoDatabase<DecodeStatsProto>;
std::unique_ptr<DBType::KeyEntryVector> entries =
std::make_unique<DBType::KeyEntryVector>();
entries->emplace_back(key.Serialize(), *stats_proto);
db_->UpdateEntries(std::move(entries),
std::make_unique<leveldb_proto::KeyVector>(),
base::BindOnce(&VideoDecodeStatsDBImpl::OnEntryUpdated,
......@@ -192,11 +241,13 @@ void VideoDecodeStatsDBImpl::OnGotDecodeStats(
UMA_HISTOGRAM_BOOLEAN("Media.VideoDecodeStatsDB.OpSuccess.Read", success);
std::unique_ptr<DecodeStatsEntry> entry;
if (stats_proto) {
if (stats_proto && !AreStatsExpired(stats_proto.get())) {
DCHECK(success);
entry = std::make_unique<DecodeStatsEntry>(
stats_proto->frames_decoded(), stats_proto->frames_dropped(),
stats_proto->frames_decoded_power_efficient());
stats_proto->frames_power_efficient());
}
DVLOG(3) << __func__ << " read " << (success ? "succeeded" : "FAILED!")
......
......@@ -17,6 +17,7 @@
namespace base {
class FilePath;
class Clock;
} // namespace base
namespace media {
......@@ -28,6 +29,20 @@ class DecodeStatsProto;
// construction. API callbacks will also occur on this sequence.
class MEDIA_EXPORT VideoDecodeStatsDBImpl : public VideoDecodeStatsDB {
public:
enum : int {
// Number chosen after manual review of metrics (accuracy, precision,
// recall).
// TODO(chcunningham): Run experiments with different values. Metrics
// suggest different platforms, even different stream properties (e.g.
// framerate) could benefit from customized thresholds. Machine learning
// would probably be best.
kMaxFramesPerBuffer = 2500,
// Number of days after which stats will be discarded if not updated. This
// avoids users getting stuck with a bad capability prediction that may have
// been due to one-off circumstances.
kMaxDaysToKeepStats = 30,
};
// Create an instance! |db_dir| specifies where to store LevelDB files to
// disk. LevelDB generates a handful of files, so its recommended to provide a
// dedicated directory to keep them isolated.
......@@ -53,6 +68,12 @@ class MEDIA_EXPORT VideoDecodeStatsDBImpl : public VideoDecodeStatsDB {
std::unique_ptr<leveldb_proto::ProtoDatabase<DecodeStatsProto>> db,
const base::FilePath& dir);
// Default |last_write_time| for DB entries that lack a time stamp due to
// using an earlier version of DecodeStatsProto. Date chosen so old stats from
// previous version will expire (unless new stats arrive) roughly 2 months
// after the proto update hits the chrome Stable channel (M71).
static constexpr char kDefaultWriteTime[] = "01-FEB-2019 12:00pm";
// Called when the database has been initialized. Will immediately call
// |init_cb| to forward |success|.
void OnInit(InitializeCB init_cb, bool success);
......@@ -66,7 +87,7 @@ class MEDIA_EXPORT VideoDecodeStatsDBImpl : public VideoDecodeStatsDB {
const DecodeStatsEntry& entry,
AppendDecodeStatsCB append_done_cb,
bool read_success,
std::unique_ptr<DecodeStatsProto> prev_stats_proto);
std::unique_ptr<DecodeStatsProto> stats_proto);
// Called when the database has been modified after a call to
// |WriteUpdatedEntry|. Will run |append_done_cb| when done.
......@@ -75,10 +96,9 @@ class MEDIA_EXPORT VideoDecodeStatsDBImpl : public VideoDecodeStatsDB {
// Called when GetDecodeStats() operation was performed. |get_stats_cb|
// will be run with |success| and a |DecodeStatsEntry| created from
// |stats_proto| or nullptr if no entry was found for the requested key.
void OnGotDecodeStats(
GetDecodeStatsCB get_stats_cb,
bool success,
std::unique_ptr<DecodeStatsProto> capabilities_info_proto);
void OnGotDecodeStats(GetDecodeStatsCB get_stats_cb,
bool success,
std::unique_ptr<DecodeStatsProto> stats_proto);
// Internal callback for first step of ClearStats(). Will clear all stats If
// |keys| fetched successfully.
......@@ -90,6 +110,14 @@ class MEDIA_EXPORT VideoDecodeStatsDBImpl : public VideoDecodeStatsDB {
// ClearStats(). Method simply logs |success| and runs |clear_done_cb|.
void OnStatsCleared(base::OnceClosure clear_done_cb, bool success);
// Return true if:
// "now" - stats_proto.last_write_date > kMaxDaysToKeepStats
bool AreStatsExpired(const DecodeStatsProto* const stats_proto);
void set_wall_clock_for_test(const base::Clock* tick_clock) {
wall_clock_ = tick_clock;
}
// Indicates whether initialization is completed. Does not indicate whether it
// was successful. Will be reset upon calling DestroyStats(). Failed
// initialization is signaled by setting |db_| to null.
......@@ -102,6 +130,12 @@ class MEDIA_EXPORT VideoDecodeStatsDBImpl : public VideoDecodeStatsDB {
// Directory where levelDB should store database files.
base::FilePath db_dir_;
// For getting wall-clock time. Tests may override via SetClockForTest().
const base::Clock* wall_clock_ = nullptr;
// Stores parsed value of |kDefaultWriteTime|.
base::Time default_write_time_;
// Ensures all access to class members come on the same sequence. API calls
// and callbacks should occur on the same sequence used during construction.
// LevelDB operations happen on a separate task runner, but all LevelDB
......
......@@ -308,6 +308,6 @@ struct PredictionFeatures {
struct PredictionTargets {
uint32 frames_decoded = 0;
uint32 frames_dropped = 0;
uint32 frames_decoded_power_efficient = 0;
uint32 frames_power_efficient = 0;
};
......@@ -124,7 +124,7 @@ void VideoDecodePerfHistory::AssessStats(
double percent_dropped =
static_cast<double>(stats->frames_dropped) / stats->frames_decoded;
double percent_power_efficient =
static_cast<double>(stats->frames_decoded_power_efficient) /
static_cast<double>(stats->frames_power_efficient) /
stats->frames_decoded;
*is_power_efficient =
......@@ -153,7 +153,7 @@ void VideoDecodePerfHistory::OnGotStatsForRequest(
percent_dropped =
static_cast<double>(stats->frames_dropped) / stats->frames_decoded;
percent_power_efficient =
static_cast<double>(stats->frames_decoded_power_efficient) /
static_cast<double>(stats->frames_power_efficient) /
stats->frames_decoded;
}
......@@ -213,7 +213,7 @@ void VideoDecodePerfHistory::SavePerfRecord(ukm::SourceId source_id,
features.profile, features.video_size, features.frames_per_sec);
VideoDecodeStatsDB::DecodeStatsEntry new_stats(
targets.frames_decoded, targets.frames_dropped,
targets.frames_decoded_power_efficient);
targets.frames_power_efficient);
// Get past perf info and report UKM metrics before saving this record.
db_->GetDecodeStats(
......@@ -301,7 +301,7 @@ void VideoDecodePerfHistory::ReportUkmMetrics(
builder.SetPerf_PastVideoFramesDecoded(past_stats->frames_decoded);
builder.SetPerf_PastVideoFramesDropped(past_stats->frames_dropped);
builder.SetPerf_PastVideoFramesPowerEfficient(
past_stats->frames_decoded_power_efficient);
past_stats->frames_power_efficient);
} else {
builder.SetPerf_PastVideoFramesDecoded(0);
builder.SetPerf_PastVideoFramesDropped(0);
......@@ -315,8 +315,7 @@ void VideoDecodePerfHistory::ReportUkmMetrics(
builder.SetPerf_RecordIsPowerEfficient(new_is_efficient);
builder.SetPerf_VideoFramesDecoded(new_stats.frames_decoded);
builder.SetPerf_VideoFramesDropped(new_stats.frames_dropped);
builder.SetPerf_VideoFramesPowerEfficient(
new_stats.frames_decoded_power_efficient);
builder.SetPerf_VideoFramesPowerEfficient(new_stats.frames_power_efficient);
builder.Record(ukm_recorder);
}
......
......@@ -69,8 +69,8 @@ class FakeVideoDecodeStatsDB : public VideoDecodeStatsDB {
entries_.at(key_str) = DecodeStatsEntry(
known_entry.frames_decoded + new_entry.frames_decoded,
known_entry.frames_dropped + new_entry.frames_dropped,
known_entry.frames_decoded_power_efficient +
new_entry.frames_decoded_power_efficient);
known_entry.frames_power_efficient +
new_entry.frames_power_efficient);
}
std::move(append_done_cb).Run(true);
......@@ -154,14 +154,13 @@ class VideoDecodePerfHistoryTest : public testing::Test {
return features;
}
mojom::PredictionTargets MakeTargets(
uint32_t frames_decoded,
uint32_t frames_dropped,
uint32_t frames_decoded_power_efficient) {
mojom::PredictionTargets MakeTargets(uint32_t frames_decoded,
uint32_t frames_dropped,
uint32_t frames_power_efficient) {
mojom::PredictionTargets targets;
targets.frames_decoded = frames_decoded;
targets.frames_dropped = frames_dropped;
targets.frames_decoded_power_efficient = frames_decoded_power_efficient;
targets.frames_power_efficient = frames_power_efficient;
return targets;
}
......
......@@ -53,12 +53,11 @@ void VideoDecodeStatsRecorder::UpdateRecord(
// Dropped can never exceed decoded.
DCHECK_LE(targets->frames_dropped, targets->frames_decoded);
// Power efficient frames can never exceed decoded frames.
DCHECK_LE(targets->frames_decoded_power_efficient, targets->frames_decoded);
DCHECK_LE(targets->frames_power_efficient, targets->frames_decoded);
// Should never go backwards.
DCHECK_GE(targets->frames_decoded, targets_.frames_decoded);
DCHECK_GE(targets->frames_dropped, targets_.frames_dropped);
DCHECK_GE(targets->frames_decoded_power_efficient,
targets_.frames_decoded_power_efficient);
DCHECK_GE(targets->frames_power_efficient, targets_.frames_power_efficient);
targets_ = *targets;
}
......@@ -74,8 +73,7 @@ void VideoDecodeStatsRecorder::FinalizeRecord() {
<< " fps:" << features_.frames_per_sec
<< " decoded:" << targets_.frames_decoded
<< " dropped:" << targets_.frames_dropped
<< " power efficient decoded:"
<< targets_.frames_decoded_power_efficient;
<< " power efficient decoded:" << targets_.frames_power_efficient;
// Final argument is an empty save-done-callback. No action to take if save
// fails (DB already records UMAs on failure). Callback mainly used by tests.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment