Experiment with more aggressive MSE GC on memory pressure

media/filters/source_buffer_stream_unittest.cc

Index: media/filters/source_buffer_stream_unittest.cc
diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
index c539839e1eb718a9cba1551d81ca330b2bff163b..6fa43c48aefdc9139a729dc83e515248a748821a 100644
--- a/media/filters/source_buffer_stream_unittest.cc
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -16,9 +16,11 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/test/scoped_feature_list.h"
 #include "media/base/data_buffer.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/media_log.h"
+#include "media/base/media_switches.h"
 #include "media/base/media_util.h"
 #include "media/base/mock_media_log.h"
 #include "media/base/test_helpers.h"
@@ -4752,6 +4754,74 @@ TEST_F(SourceBufferStreamTest, GetHighestPresentationTimestamp) {
   EXPECT_EQ(base::TimeDelta(), stream_->GetHighestPresentationTimestamp());
 }
 
+TEST_F(SourceBufferStreamTest, GarbageCollectionUnderMemoryPressure) {
+  SetMemoryLimit(20);
+  NewCodedFrameGroupAppend(0, 20, &kDataA);

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

Can you use the string version of this API instead. I find the old version
pretty hard to read.

https://cs.chromium.org/chromium/src/media/filters/source_buffer_stream_unitt...

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

Well, one big advantage of using this form of the API is that it allows us to be
much more concise. If we used the string API it would be something like:
NewCodedFrameGroupAppend("0K 10 20 30 40 50K 60 70 80 90 100K 110 120 130 140
150K 160 170 180 190 200K");

I personally prefer the shorter form and find it more readable in this case,
since we don't need to draw that much attention to actual frame timestamps. Plus
other GC-related tests also use mostly the short form API, so for consistency
I've used it here too.
But I can change the string version, if you have a strong preference to the
longer form.

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

It doesn't have to be this huge. you can make your gops smaller and have less of
them and still verify everything you're doing in this test. 

This API (and the associated CheckExpectedRangesByTimestamp()) is more readable
and better at self documenting. It clearly lays out where the GOPs start and
stop (I otherwise have to look up the impl of NewCodedFrameGroupAppend and then
lookup the value for default key frame interval). This helps readers verify that
the expected ranges after garbage collection preserved the correct GOP.

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

Done.

+  CheckExpectedRanges("{ [0,19) }");
+
+  // This feature is disabled by default, so by default memory pressure
+  // notification takes no effect and the memory limits and won't remove
+  // anything from buffered ranges, since we are under the limit of 20 bytes.
+  stream_->OnMemoryPressure(
+      DecodeTimestamp::FromMilliseconds(0),
+      base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE);
+  EXPECT_TRUE(GarbageCollectWithPlaybackAtBuffer(5, 0));
+  CheckExpectedRanges("{ [0,19) }");
+
+  // Now enable the feature.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(kReduceMSEBuffersOnMemoryPressure);
+
+  // Verify that effective MSE memory limit is reduced under memory pressure.
+  stream_->OnMemoryPressure(
+      DecodeTimestamp::FromMilliseconds(0),
+      base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE);
+
+  // Effective memory limit is now 10 buffers, but we still will not collect any
+  // data between the current playback position 5 and last append position 19.
+  EXPECT_TRUE(GarbageCollectWithPlaybackAtBuffer(5, 0));
+  CheckExpectedRanges("{ [5,19) }");
+
+  // As playback proceeds further to buffer 12 we should be able to collect
+  // enough data to bring us back under memory limit of 10 buffers.
+  EXPECT_TRUE(GarbageCollectWithPlaybackAtBuffer(12, 0));
+  CheckExpectedRanges("{ [10,19) }");
+
+  // If memory pressure becomes critical, the garbage collection algorithm
+  // becomes even more aggressive and collects everything up to the current
+  // playback position.
+  stream_->OnMemoryPressure(
+      DecodeTimestamp::FromMilliseconds(0),
+      base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL);
+  EXPECT_TRUE(GarbageCollectWithPlaybackAtBuffer(15, 0));
+  CheckExpectedRanges("{ [15,19) }");
+
+  // But even under critical memory pressure the MSE memory limit imposed by the
+  // memory pressure is soft, i.e. we should be able to append more data
+  // successfully up to the hard limit of 20 bytes.
+  NewCodedFrameGroupAppend(20, 15, &kDataA);

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

Ditto

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

Done.

+  CheckExpectedRanges("{ [15,34) }");
+  EXPECT_TRUE(GarbageCollectWithPlaybackAtBuffer(15, 0));
+  CheckExpectedRanges("{ [15,34) }");
+}
+
+TEST_F(SourceBufferStreamTest, InstantGarbageCollectionUnderMemoryPressure) {
+  SetMemoryLimit(20);
+  NewCodedFrameGroupAppend(0, 20, &kDataA);

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

Ditto

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

Done.

+  CheckExpectedRanges("{ [0,19) }");
+
+  // Verify that garbage collection happens immediately on critical memory
+  // pressure notification, even without explicit GarbageCollect invocation,
+  // when the immediate GC is allowed.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeatures(
+      {kReduceMSEBuffersOnMemoryPressure, kMSEInstantGCOnMemoryPressure}, {});
+  stream_->OnMemoryPressure(
+      DecodeTimestamp::FromMilliseconds(500),
+      base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL);
+  CheckExpectedRanges("{ [15,19) }");
+}
+
 // TODO(vrk): Add unit tests where keyframes are unaligned between streams.
 // (crbug.com/133557)