Index: media/blink/webmediaplayer_impl.cc |
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc |
index 4cc6abe818db3059ca58ade50b6d58ec021ff159..bd3380cdab0f59dd81f538d1bb13b702395531be 100644 |
--- a/media/blink/webmediaplayer_impl.cc |
+++ b/media/blink/webmediaplayer_impl.cc |
@@ -18,6 +18,7 @@ |
#include "base/debug/alias.h" |
#include "base/debug/crash_logging.h" |
#include "base/location.h" |
+#include "base/memory/ptr_util.h" |
#include "base/metrics/histogram_macros.h" |
#include "base/single_thread_task_runner.h" |
#include "base/strings/string_number_conversions.h" |
@@ -1080,7 +1081,7 @@ void WebMediaPlayerImpl::OnPipelineSuspended() { |
data_source_->OnBufferingHaveEnough(true); |
} |
- ReportMemoryUsage(); |
+ ReportMemoryUsage(MaybeMemPressureLvl()); |
if (pending_suspend_resume_cycle_) { |
pending_suspend_resume_cycle_ = false; |
@@ -1092,6 +1093,17 @@ void WebMediaPlayerImpl::OnDemuxerOpened() { |
DCHECK(main_task_runner_->BelongsToCurrentThread()); |
client_->mediaSourceOpened( |
new WebMediaSourceImpl(chunk_demuxer_, media_log_)); |
+ // Using base::Unretained(this) is safe here, because MemoryPressureListener |
+ // object is owned by this (WMPI::mem_pressure_listener_). |
watk
2016/12/13 23:34:31
"because the".
Or if you want to reword it: "base
servolk
2016/12/14 00:07:23
Done.
|
+ mem_pressure_listener_ = |
+ base::WrapUnique(new base::MemoryPressureListener(base::Bind( |
watk
2016/12/13 23:34:31
Prefer MakeUnique(). (Unless there's a reason you
servolk
2016/12/14 00:07:23
Done.
|
+ &WebMediaPlayerImpl::OnMemoryPressure, base::Unretained(this)))); |
+} |
+ |
+void WebMediaPlayerImpl::OnMemoryPressure( |
+ base::MemoryPressureListener::MemoryPressureLevel lvl) { |
+ DCHECK(main_task_runner_->BelongsToCurrentThread()); |
+ ReportMemoryUsage(lvl); |
} |
void WebMediaPlayerImpl::OnError(PipelineStatus status) { |
@@ -1199,7 +1211,7 @@ void WebMediaPlayerImpl::OnBufferingStateChange(BufferingState state) { |
// Once we have enough, start reporting the total memory usage. We'll also |
// report once playback starts. |
- ReportMemoryUsage(); |
+ ReportMemoryUsage(MaybeMemPressureLvl()); |
// Report the amount of time it took to leave the underflow state. Don't |
// bother to report this for MSE playbacks since it's out of our control. |
@@ -1764,12 +1776,12 @@ void WebMediaPlayerImpl::SetMemoryReportingState( |
} |
if (is_memory_reporting_enabled) { |
- memory_usage_reporting_timer_.Start(FROM_HERE, |
- base::TimeDelta::FromSeconds(2), this, |
- &WebMediaPlayerImpl::ReportMemoryUsage); |
+ memory_usage_reporting_timer_.Start( |
+ FROM_HERE, base::TimeDelta::FromSeconds(2), this, |
+ &WebMediaPlayerImpl::ReportMemoryUsageOnTimer); |
} else { |
memory_usage_reporting_timer_.Stop(); |
- ReportMemoryUsage(); |
+ ReportMemoryUsage(MaybeMemPressureLvl()); |
} |
} |
@@ -1910,7 +1922,12 @@ WebMediaPlayerImpl::UpdatePlayState_ComputePlayState(bool is_remote, |
return result; |
} |
-void WebMediaPlayerImpl::ReportMemoryUsage() { |
+void WebMediaPlayerImpl::ReportMemoryUsageOnTimer() { |
watk
2016/12/13 23:34:31
Can you bind the nullopt into the callback and del
servolk
2016/12/14 00:07:23
This is passed into timer.Start (see https://cs.ch
servolk
2016/12/14 00:11:23
Ah, wait, despite the weird Start method at https:
servolk
2016/12/14 00:15:37
Done.
|
+ ReportMemoryUsage(MaybeMemPressureLvl()); |
+} |
+ |
+void WebMediaPlayerImpl::ReportMemoryUsage( |
+ MaybeMemPressureLvl maybe_mem_pressure_lvl) { |
DCHECK(main_task_runner_->BelongsToCurrentThread()); |
// About base::Unretained() usage below: We destroy |demuxer_| on the main |
@@ -1922,32 +1939,70 @@ void WebMediaPlayerImpl::ReportMemoryUsage() { |
base::PostTaskAndReplyWithResult( |
media_task_runner_.get(), FROM_HERE, |
base::Bind(&Demuxer::GetMemoryUsage, base::Unretained(demuxer_.get())), |
- base::Bind(&WebMediaPlayerImpl::FinishMemoryUsageReport, AsWeakPtr())); |
+ base::Bind(&WebMediaPlayerImpl::FinishMemoryUsageReport, AsWeakPtr(), |
+ maybe_mem_pressure_lvl)); |
} else { |
- FinishMemoryUsageReport(0); |
+ FinishMemoryUsageReport(maybe_mem_pressure_lvl, 0); |
} |
} |
-void WebMediaPlayerImpl::FinishMemoryUsageReport(int64_t demuxer_memory_usage) { |
+std::string MaybeMemPressureToLogString( |
+ const base::Optional<base::MemoryPressureListener::MemoryPressureLevel>& |
watk
2016/12/13 23:34:31
Use the type alias?
servolk
2016/12/14 00:07:23
I could, but the type alias is currently private i
watk
2016/12/14 00:34:05
Ah, didn't notice this was a non-member function.
servolk
2016/12/14 02:34:33
Acknowledged.
|
+ maybe_mem_pressure_lvl) { |
+ if (maybe_mem_pressure_lvl) { |
+ switch (*maybe_mem_pressure_lvl) { |
+ case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE: |
+ return "none"; |
+ case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE: |
+ return "moderate"; |
+ case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL: |
+ return "critical"; |
+ } |
+ } |
+ return ""; |
+} |
+ |
+void WebMediaPlayerImpl::FinishMemoryUsageReport( |
+ MaybeMemPressureLvl maybe_mem_pressure_lvl, |
+ int64_t demuxer_memory_usage) { |
DCHECK(main_task_runner_->BelongsToCurrentThread()); |
const PipelineStatistics stats = pipeline_.GetStatistics(); |
+ const int64_t data_source_mem_usage = |
watk
2016/12/13 23:34:31
"memory_usage" for consistency with other variable
servolk
2016/12/14 00:07:23
Done.
|
+ (data_source_ ? data_source_->GetMemoryUsage() : 0); |
const int64_t current_memory_usage = |
stats.audio_memory_usage + stats.video_memory_usage + |
- (data_source_ ? data_source_->GetMemoryUsage() : 0) + |
- demuxer_memory_usage; |
+ data_source_mem_usage + demuxer_memory_usage; |
+ |
+ std::string mem_pressure_log = |
+ MaybeMemPressureToLogString(maybe_mem_pressure_lvl); |
+ if (mem_pressure_log != "") |
+ mem_pressure_log = " (Mem pressure: " + mem_pressure_log + ")"; |
watk
2016/12/13 23:34:31
Do this inside MaybeMemPressureToLogString and inl
servolk
2016/12/14 00:07:23
This would make MaybeMemPressureToLogString less r
|
// Note, this isn't entirely accurate, there may be VideoFrames held by the |
// compositor or other resources that we're unaware of. |
- |
- DVLOG(2) << "Memory Usage -- Audio: " << stats.audio_memory_usage |
- << ", Video: " << stats.video_memory_usage << ", DataSource: " |
- << (data_source_ ? data_source_->GetMemoryUsage() : 0) |
+ DVLOG(2) << "Memory Usage " << mem_pressure_log |
+ << " -- Audio: " << stats.audio_memory_usage |
+ << ", Video: " << stats.video_memory_usage |
+ << ", DataSource: " << data_source_mem_usage |
<< ", Demuxer: " << demuxer_memory_usage; |
const int64_t delta = current_memory_usage - last_reported_memory_usage_; |
last_reported_memory_usage_ = current_memory_usage; |
adjust_allocated_memory_cb_.Run(delta); |
+ |
+ if (maybe_mem_pressure_lvl && |
DaleCurtis
2016/12/12 23:49:48
Hmm, why? Why not just always report the average m
servolk
2016/12/12 23:56:12
ReportMemoryUsage and FinishMemoryUsageReport are
DaleCurtis
2016/12/13 00:04:07
Reporting UMA is mostly just updating a static val
|
+ *maybe_mem_pressure_lvl >= |
+ base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE) { |
+ UMA_HISTOGRAM_MEMORY_KB("Media.WebMediaPlayerImpl.Memory.AudioKb", |
+ stats.audio_memory_usage / 1024); |
+ UMA_HISTOGRAM_MEMORY_KB("Media.WebMediaPlayerImpl.Memory.VideoKb", |
+ stats.video_memory_usage / 1024); |
+ UMA_HISTOGRAM_MEMORY_KB("Media.WebMediaPlayerImpl.Memory.DataSourceKb", |
+ stats.video_memory_usage / 1024); |
+ UMA_HISTOGRAM_MEMORY_KB("Media.WebMediaPlayerImpl.Memory.DemuxerKb", |
+ demuxer_memory_usage / 1024); |
+ } |
} |
void WebMediaPlayerImpl::ScheduleIdlePauseTimer() { |