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..4ded0122b555b73ad33b5c2c1a948df78fdffbc4 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,35 @@ void SourceBufferStream::SetConfigIds(const BufferQueue& buffers) {
   }
 }
 
+void SourceBufferStream::OnMemoryPressure(
+    DecodeTimestamp media_time,
+    base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level,
+    bool force_gc) {
+  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

[DaleCurtis, 2017/01/26 23:07:37]

Comment talks about critical but if force is enabled it does it immediately.
Comment is also worded in a speculative sense when the exact operation can be
specified instead :)

You should paragraph break at "Strictly speaking..." too and replace that text
with "Per spec, MSE garbage collection must only occur during SourceBuffer
append()" etc.

[servolk, 2017/01/27 00:52:13]

Updated the comment and moved it closer to where we decide the value of
|force_gc| in WMPI::OnMemoryPressure.

+  // 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 (force_gc) {
+    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(kReduceMSEBuffersOnMemoryPressure)) {

[DaleCurtis, 2017/01/26 23:08:42]

Also drop {} from single line if statements (here and elsewhere).

[servolk, 2017/01/27 00:52:13]

Done.

+    DCHECK(!end_of_stream_);
+  }
   // Compute size of |ranges_|.
   size_t ranges_size = GetBufferedSize();
 
@@ -713,11 +737,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:
+        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 +767,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 +877,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(