Commit 0cb57978 authored by Chris Cunningham's avatar Chris Cunningham Committed by Commit Bot

MediaCapabilities: Fix DB corruption (divide by zero)

debug::Alias did the trick. We were dividing by zero whenever a record
with significant duration was saved to an empty bucket.

Pretty awful bug. UMA shows this is causing 2% of DB reads to be
discarded (assuming this is the sole remaining source of corruption).

Bug: 982009
Change-Id: Ie13643a79652163bfaf343259af44141f2a53fc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854796
Auto-Submit: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: default avatarFrank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704996}
parent 834c0430
...@@ -261,6 +261,10 @@ void VideoDecodeStatsDBImpl::WriteUpdatedEntry( ...@@ -261,6 +261,10 @@ void VideoDecodeStatsDBImpl::WriteUpdatedEntry(
new_entry_efficient_ratio = new_entry_efficient_ratio =
static_cast<double>(new_entry.frames_power_efficient) / static_cast<double>(new_entry.frames_power_efficient) /
new_entry.frames_decoded; new_entry.frames_decoded;
} else {
// Callers shouldn't ask DB to save empty records. See
// VideoDecodeStatsRecorder.
NOTREACHED() << __func__ << " saving empty stats record";
} }
if (old_frames_decoded + new_entry.frames_decoded > kMaxFramesPerBuffer) { if (old_frames_decoded + new_entry.frames_decoded > kMaxFramesPerBuffer) {
...@@ -271,10 +275,14 @@ void VideoDecodeStatsDBImpl::WriteUpdatedEntry( ...@@ -271,10 +275,14 @@ void VideoDecodeStatsDBImpl::WriteUpdatedEntry(
static_cast<double>(new_entry.frames_decoded) / kMaxFramesPerBuffer, static_cast<double>(new_entry.frames_decoded) / kMaxFramesPerBuffer,
1.0); 1.0);
double old_dropped_ratio = double old_dropped_ratio = 0;
static_cast<double>(old_frames_dropped) / old_frames_decoded; double old_efficient_ratio = 0;
double old_efficient_ratio = if (old_frames_decoded) {
static_cast<double>(old_frames_power_efficient) / old_frames_decoded; old_dropped_ratio =
static_cast<double>(old_frames_dropped) / old_frames_decoded;
old_efficient_ratio =
static_cast<double>(old_frames_power_efficient) / old_frames_decoded;
}
double agg_dropped_ratio = fill_ratio * new_entry_dropped_ratio + double agg_dropped_ratio = fill_ratio * new_entry_dropped_ratio +
(1 - fill_ratio) * old_dropped_ratio; (1 - fill_ratio) * old_dropped_ratio;
...@@ -337,6 +345,11 @@ void VideoDecodeStatsDBImpl::WriteUpdatedEntry( ...@@ -337,6 +345,11 @@ void VideoDecodeStatsDBImpl::WriteUpdatedEntry(
// Update the time stamp for the current write. // Update the time stamp for the current write.
stats_proto->set_last_write_date(wall_clock_->Now().ToJsTime()); stats_proto->set_last_write_date(wall_clock_->Now().ToJsTime());
// Make sure we never write bogus stats into the DB! While its possible the DB
// may experience some corruption (disk), we should have detected that above
// and discarded any bad data prior to this upcoming save.
DCHECK(AreStatsUsable(stats_proto.get()));
// Push the update to the DB. // Push the update to the DB.
using DBType = leveldb_proto::ProtoDatabase<DecodeStatsProto>; using DBType = leveldb_proto::ProtoDatabase<DecodeStatsProto>;
std::unique_ptr<DBType::KeyEntryVector> entries = std::unique_ptr<DBType::KeyEntryVector> entries =
......
...@@ -446,6 +446,43 @@ TEST_F(VideoDecodeStatsDBImplTest, FillBufferInMixedIncrements) { ...@@ -446,6 +446,43 @@ TEST_F(VideoDecodeStatsDBImplTest, FillBufferInMixedIncrements) {
std::round(GetMaxFramesPerBuffer() * kEfficientRateC))); std::round(GetMaxFramesPerBuffer() * kEfficientRateC)));
} }
// Overfilling an empty buffer triggers the codepath to compute weighted dropped
// and power efficient ratios under a circumstance where the existing counts are
// all zero. This test ensures that we don't do any dividing by zero with that
// empty data.
TEST_F(VideoDecodeStatsDBImplTest, OverfillEmptyBuffer) {
InitializeDB();
// Setup DB entry that overflows the buffer max (by 1) with 10% of frames
// dropped and 50% of frames power efficient.
const int kNumFramesOverfill = GetMaxFramesPerBuffer() + 1;
DecodeStatsEntry entryA(kNumFramesOverfill,
std::round(0.1 * kNumFramesOverfill),
std::round(0.5 * kNumFramesOverfill));
// Append entry to completely fill the buffer and verify read.
AppendStats(kStatsKeyVp9, entryA);
// Read-back stats should have same ratios, but scaled such that
// frames_decoded = GetMaxFramesPerBuffer().
DecodeStatsEntry readBackEntryA(GetMaxFramesPerBuffer(),
std::round(0.1 * GetMaxFramesPerBuffer()),
std::round(0.5 * GetMaxFramesPerBuffer()));
VerifyReadStats(kStatsKeyVp9, readBackEntryA);
// Append another entry that again overfills with different dropped and power
// efficient ratios. Verify that read-back only reflects latest entry.
DecodeStatsEntry entryB(kNumFramesOverfill,
std::round(0.2 * kNumFramesOverfill),
std::round(0.6 * kNumFramesOverfill));
AppendStats(kStatsKeyVp9, entryB);
// Read-back stats should have same ratios, but scaled such that
// frames_decoded = GetMaxFramesPerBuffer().
DecodeStatsEntry readBackEntryB(GetMaxFramesPerBuffer(),
std::round(0.2 * GetMaxFramesPerBuffer()),
std::round(0.6 * GetMaxFramesPerBuffer()));
VerifyReadStats(kStatsKeyVp9, readBackEntryB);
}
TEST_F(VideoDecodeStatsDBImplTest, NoWriteDateReadAndExpire) { TEST_F(VideoDecodeStatsDBImplTest, NoWriteDateReadAndExpire) {
InitializeDB(); InitializeDB();
......
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