Experiment with more aggressive MSE GC on memory pressure

media/filters/source_buffer_stream.cc

Index: media/filters/source_buffer_stream.cc
diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index fe3dbfea4fca61cd406a420363b5ea62b194e62b..86c73861b6887c57788e5f39211dbb5a9b2469f5 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/trace_event/trace_event.h"
+#include "media/base/media_switches.h"
 #include "media/base/timestamp_constants.h"
 #include "media/filters/source_buffer_platform.h"
 #include "media/filters/source_buffer_range.h"
@@ -692,12 +693,36 @@ void SourceBufferStream::SetConfigIds(const BufferQueue& buffers) {
   }
 }
 
+void SourceBufferStream::OnMemoryPressure(
+    DecodeTimestamp media_time,
+    base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) {
+  DVLOG(4) << __func__ << " level=" << memory_pressure_level;
+  memory_pressure_level_ = memory_pressure_level;
+
+  // The new value of |memory_pressure_level_| updated set above will take
+  // effect on the next SourceBuffer append operation. But if we are notified
+  // that memory pressure is critical, we might want to perform a GC right away,
+  // to release some memory immediately. Strictly speaking MSE apps expect
+  // buffered ranges only at well-defined points, like SourceBuffer append
+  // operation, and may not expect this. But it might still be better than being
+  // killed due to OOM, so might be worth the risk.
+  if (base::FeatureList::IsEnabled(kMSEInstantGCOnMemoryPressure) &&
+      memory_pressure_level ==
+          base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) {
+    GarbageCollectIfNeeded(media_time, 0);
+  }
+}
+
 bool SourceBufferStream::GarbageCollectIfNeeded(DecodeTimestamp media_time,
                                                 size_t newDataSize) {
   DCHECK(media_time != kNoDecodeTimestamp());
   // Garbage collection should only happen before/during appending new data,
-  // which should not happen in end-of-stream state.
-  DCHECK(!end_of_stream_);
+  // which should not happen in end-of-stream state. Unless we also allow GC to
+  // happen on memory pressure notifications, which might happen even in EOS
+  // state.
+  if (!base::FeatureList::IsEnabled(kMSEInstantGCOnMemoryPressure)) {
+    DCHECK(!end_of_stream_);
+  }
   // Compute size of |ranges_|.
   size_t ranges_size = GetBufferedSize();
 
@@ -713,11 +738,29 @@ bool SourceBufferStream::GarbageCollectIfNeeded(DecodeTimestamp media_time,
     return false;
   }
 
+  size_t effective_memory_limit = memory_limit_;
+  if (base::FeatureList::IsEnabled(kReduceMSEBuffersOnMemoryPressure)) {
+    switch (memory_pressure_level_) {
+      case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE:
+        effective_memory_limit = memory_limit_ / 2;
+        break;
+      case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL:
+        effective_memory_limit = 0;
+        break;
+      case base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_NONE:

[chcunningham, 2017/01/25 03:47:15]

Maybe no-op is fine, but what made you decide not to instead restore the memory
limit to its original value?

[servolk, 2017/01/25 18:06:47]

Tbh I'm not sure what you mean by 'restore the memory limit' - restore which
limit? We are not changing |memory_limit_| at all, and the
|effective_memory_limit| is a local variable, which is already initialized to
the |memory_limit_| value. 

[chcunningham, 2017/01/25 22:21:57]

My bad. I overlooked the local variable. Seems it is behaving as I wanted, i.e.
upon memory_pressur_none we go back to GC'ing with our original level of
aggressiveness. 

[servolk, 2017/01/26 18:10:14]

Acknowledged.

+        break;
+    }
+  }
+
   // Return if we're under or at the memory limit.
-  if (ranges_size + newDataSize <= memory_limit_)
+  if (ranges_size + newDataSize <= effective_memory_limit)
     return true;
 
-  size_t bytes_to_free = ranges_size + newDataSize - memory_limit_;
+  size_t bytes_over_hard_memory_limit = 0;
+  if (ranges_size + newDataSize > memory_limit_)
+    bytes_over_hard_memory_limit = ranges_size + newDataSize - memory_limit_;
+
+  size_t bytes_to_free = ranges_size + newDataSize - effective_memory_limit;
 
   DVLOG(2) << __func__ << " " << GetStreamTypeName()
            << ": Before GC media_time=" << media_time.InSecondsF()
@@ -725,6 +768,7 @@ bool SourceBufferStream::GarbageCollectIfNeeded(DecodeTimestamp media_time,
            << " seek_pending_=" << seek_pending_
            << " ranges_size=" << ranges_size << " newDataSize=" << newDataSize
            << " memory_limit_=" << memory_limit_
+           << " effective_memory_limit=" << effective_memory_limit
            << " last_appended_buffer_timestamp_="
            << last_appended_buffer_timestamp_.InSecondsF();
 
@@ -834,9 +878,10 @@ bool SourceBufferStream::GarbageCollectIfNeeded(DecodeTimestamp media_time,
   DVLOG(2) << __func__ << " " << GetStreamTypeName()
            << ": After GC bytes_to_free=" << bytes_to_free
            << " bytes_freed=" << bytes_freed
+           << " bytes_over_hard_memory_limit=" << bytes_over_hard_memory_limit
            << " ranges_=" << RangesToString(ranges_);
 
-  return bytes_freed >= bytes_to_free;
+  return bytes_freed >= bytes_over_hard_memory_limit;
 }
 
 size_t SourceBufferStream::FreeBuffersAfterLastAppended(