Allow setting memory limits on media::DemuxerStream

media/base/demuxer_stream.h

Index: media/base/demuxer_stream.h
diff --git a/media/base/demuxer_stream.h b/media/base/demuxer_stream.h
index 596ab67e83692aba287f11e6c1488a623ffee8e6..c2620c0238555acef9d449eb37995a478717ee64 100644
--- a/media/base/demuxer_stream.h
+++ b/media/base/demuxer_stream.h
@@ -91,6 +91,12 @@ class MEDIA_EXPORT DemuxerStream {
 
   virtual VideoRotation video_rotation() = 0;
 
+  // Get the data buffer size limit, in bytes.

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

Similar to my comments in https://codereview.chromium.org/1182183004/, I don't
think "buffer" is correct.

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

See the comment in that CL. It's a "buffer" that has the same meaning as in "MSE
SourceBuffer", which is different from e.g. media::DecoderBuffer. AFAIK both
uses of this word are pretty common.

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

Those are two examples of qualified uses of "buffer"; you have selected one of
them as the meaning of an unqualified use of "buffer." SourceBuffer is actually
the less common of the two since it's just one mode of the media pipeline.
"buffer" is used  in the other sense in many more cases in media/.

I replied in that CL well.

+  virtual size_t GetMemoryLimit() const = 0;

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

Is this ever called? I don't see it being used here. SourceBufferStream is the
only case that needs it, and it is NOT a DemuxerStream. Maybe it is being called
by the new code in https://codereview.chromium.org/1185453004/, but I'm not
convinced that design is desirable.

Should this really be on a demuxer _stream_ rather than the Demuxer? What
behavior would you expect if there were multiple audio streams?

Why does DemuxerStream need to implement this? Having to override this with
NOTIMPLEMENTED in so many places is a sign of a potential design issue. Perhaps
there is another way that would make more sense. Maybe MediaClient.

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

It's currently called in the unit tests (see the changes in
media/filters/chunk_demuxer_unittest.cc in this CL). I'm also planning to call
this from inside CmaRenderer in Chromecast code for adjust memory limits
according to the amount of memory available on the current platform.
Coincidentally that's the reason why I'd prefer this API to be on DemuxerStream,
instead of media::Demuxer. When a renderer is initialized it is given only
media::DemuxerStreamProvider (see media::Renderer::Initialize). So renderers can
easily access DemuxerStreams, but not the full media::Demuxer interface. On
Chromecast we provide a custom renderer, but the use the rest of the media
pipeline as provided by the standard Chromium implementation and we want to be
able to use these APIs.

Re NOTIMPLEMENTED - take a closer look, most of those NOTIMPLs are in mock/fake
objects being used to unit tests. The only two NOTIMPLs that are in actual
implementations are in FFmpegDemuxerStream and in MojoDemuxerStreamAdapter.
These will need to be implemented eventually (and I have some ideas how to do
this for FFmpeg, less clear on how to do this for Mojo).

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

Is that the amount of memory _currently_ available or the physical device
characteristics? IOW, will the limits change during runtime or just be set
per-device? If the former, will they depending on how many DemuxerStreams are
active? If the latter, asking MediaClient for an override might be a much
simpler solution.

+
+  // Set the data buffer size limit, in bytes.
+  virtual void SetMemoryLimit(size_t memory_limit) = 0;
+
  protected:
   // Only allow concrete implementations to get deleted.
   virtual ~DemuxerStream();