Allow setting memory limits on media::DemuxerStream

media/filters/source_buffer_stream.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/source_buffer_stream.h"
 
 #include <algorithm>
 #include <map>
 #include <sstream>
 
@@ skipping 574 matching lines @@
   }
 }
 
 void SourceBufferStream::SetConfigIds(const BufferQueue& buffers) {
   for (BufferQueue::const_iterator itr = buffers.begin();
        itr != buffers.end(); ++itr) {
     (*itr)->SetConfigId(append_config_index_);
   }
 }
 
 void SourceBufferStream::GarbageCollectIfNeeded() {

[ddorwin, 2015/06/17 21:02:16]

This implementation detail (limits and GC tied to a specific stream) is what
leads to the current methods for sizes. Is that dictated by something like the
spec or just the current implementation? If the latter, is it necessarily the
best or permanent solution?

[servolk, 2015/06/17 22:13:48]

Per MSE spec
(https://w3c.github.io/media-source/#sourcebuffer-coded-frame-eviction) GC
should be done on SourceBuffer level.
I think the question shouldn't be "is it the best or permanent solution". The
question should be "is it better than what we currently have". And the answer to
the latter is "yes, definitely". There are many issues with the current
implementation of GC in MSE (see crbug.com/440173, crbug.com/474806,
crbug.com/421694, etc) and I'm trying to make some steps to improve that.
One step is allowing better control over MSE sizes, so that we can increase
buffer limits for platforms that can afford that and that will help with
triggering GC-related errors less often to our end-users. Another step is MSE
algorithm improvements (e.g. CL https://codereview.chromium.org/1008463002/).
And I hope that through a series of steps like that we will arrive at some
better solution than the current "permanent" solution.

[ddorwin, 2015/06/20 01:50:26]

Right, and a SourceBuffer can have multiple streams (tracks), right? If so, that
would seem to mean eviction at this level is incorrect. That makes sense - why
would you evict different ranges of audio and video? (I'm not familiar with the
MSE details and could be wrong - I defer to wolenetz on the details of the MSE
spec and implementation.)

If I am right, limits should not be tied to streams for MSE and thus should not
be tied to DemuxerStreams.

   // Compute size of |ranges_|.
-  int ranges_size = 0;
+  size_t ranges_size = 0;
   for (RangeList::iterator itr = ranges_.begin(); itr != ranges_.end(); ++itr)
     ranges_size += (*itr)->size_in_bytes();
 
   // Return if we're under or at the memory limit.
-  if (ranges_size <= memory_limit_)
+  if (ranges_size <= GetMemoryLimit())
     return;
 
-  int bytes_to_free = ranges_size - memory_limit_;
+  size_t bytes_to_free = ranges_size - GetMemoryLimit();
 
   DVLOG(2) << __FUNCTION__ << " " << GetStreamTypeName() << ": Before GC"
            << " ranges_size=" << ranges_size
            << " ranges_=" << RangesToString(ranges_)
-           << " memory_limit_=" << memory_limit_;
+           << " GetMemoryLimit()=" << GetMemoryLimit();
 
   // Begin deleting after the last appended buffer.
-  int bytes_freed = FreeBuffersAfterLastAppended(bytes_to_free);
+  size_t bytes_freed = FreeBuffersAfterLastAppended(bytes_to_free);
 
   // Begin deleting from the front.
-  if (bytes_to_free - bytes_freed > 0)
+  if (bytes_freed < bytes_to_free)
     bytes_freed += FreeBuffers(bytes_to_free - bytes_freed, false);
 
   // Begin deleting from the back.
-  if (bytes_to_free - bytes_freed > 0)
+  if (bytes_freed < bytes_to_free)
     bytes_freed += FreeBuffers(bytes_to_free - bytes_freed, true);
 
   DVLOG(2) << __FUNCTION__ << " " << GetStreamTypeName() << ": After GC"
            << " bytes_freed=" << bytes_freed
            << " ranges_=" << RangesToString(ranges_);
 }
 
 int SourceBufferStream::FreeBuffersAfterLastAppended(int total_bytes_to_free) {
   DecodeTimestamp next_buffer_timestamp = GetNextBufferTimestamp();
   if (last_appended_buffer_timestamp_ == kNoDecodeTimestamp() ||
@@ skipping 957 matching lines @@
     return false;
 
   DCHECK_NE(have_splice_buffers, have_preroll_buffer);
   splice_buffers_index_ = 0;
   pending_buffer_.swap(*out_buffer);
   pending_buffers_complete_ = false;
   return true;
 }
 
 }  // namespace media