Allow setting memory limits on media::DemuxerStream

media/base/fake_text_track_stream.cc

 // Copyright 2013 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/base/fake_text_track_stream.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
@@ skipping 26 matching lines @@
 DemuxerStream::Type FakeTextTrackStream::type() const {
   return DemuxerStream::TEXT;
 }
 
 bool FakeTextTrackStream::SupportsConfigChanges() { return false; }
 
 VideoRotation FakeTextTrackStream::video_rotation() {
   return VIDEO_ROTATION_0;
 }
 
+int FakeTextTrackStream::GetMemoryLimit() const {
+  NOTIMPLEMENTED();

[chcunningham, 2015/06/11 18:56:55]

We're having to do this same override boilerplate in a few places. I'm newish to
the team, so this may be the preferred style, but I notice we have some
"default" implementations in demuxer_stream.cc. WDYT about making this a default
there and removing the duplicate overrides? (Matt, please comment on whether
this suggestion is stylistically correct).

[servolk, 2015/06/12 00:50:48]

Yeah, that's a good point. I was pondering this myself. I went with pure virtual
function initially to make it easier for me to catch all the places that might
need to be updated (and so I've added a proper implementation of
Get/SetMemoryLimit for DecryptingDemuxerStream and left a TODO in Mojo code),
but yeah, there are a lot of media::DemuxerStream implementations and some of
them (e.g. all the test/fake/mock implementation) don't care about this. So we
could probably go with a default definition in demuxer_stream.cc for now, we'll
just have to remember to implement this for any actual DemuxerStream
implementations as necessary.

[wolenetz, 2015/06/12 21:31:56]

I was pondering similarly.
Backing up a bit, ISTM the goal is that eventually all real DemuxerStreams
should have memory buffer limit control that is meaningful.
To discover any new DemuxerStream implementations that might crop-up over time,
ISTM that pure-virtual, non-default in DemuxerStream, with explicit need for
overrides (with TODOs for those that explicitly require implementation) in each
DemuxerStream implementation, while somewhat boilerplate, will ensure the build
won't work for some variant that might use a specialized DemuxerStream that
hasn't been discovered in this CL (prevents dynamic fragility). Test mocks will
need the boilerplate, but IMO, that's a necessary complexity that is warranted
by catching unexpected lack of implementations that might crop up.

+  return 0;
+}
+
+void FakeTextTrackStream::SetMemoryLimit(int memory_limit) {
+  NOTIMPLEMENTED();
+}
+
 void FakeTextTrackStream::SatisfyPendingRead(
     const base::TimeDelta& start,
     const base::TimeDelta& duration,
     const std::string& id,
     const std::string& content,
     const std::string& settings) {
   DCHECK(!read_cb_.is_null());
 
   const uint8* const data_buf = reinterpret_cast<const uint8*>(content.data());
   const int data_len = static_cast<int>(content.size());
@@ skipping 28 matching lines @@
   base::ResetAndReturn(&read_cb_).Run(kOk, DecoderBuffer::CreateEOSBuffer());
 }
 
 void FakeTextTrackStream::Stop() {
   stopping_ = true;
   if (!read_cb_.is_null())
     AbortPendingRead();
 }
 
 }  // namespace media