Fix MediaSource renderer to limit memory consumption.

Fix MediaSource renderer to limit memory consumption.

MediaSource renderer wasn't marking keyframes properly, and as result
SourceBuffer would keep all data it receives forever. Also added
explicit remove() call to cleanup the buffers before each keyframe.

Beside that changed default duration to a correct value instead of 1s
It currently doesn't cause any problems, but it may in the future if 
MediaSource implementation is changed to handle overlapping frames
differently.

BUG=321825
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276574

[Jamie, 2014-06-11 21:41:43]

lgtm with nits.

https://codereview.chromium.org/329663002/diff/20001/remoting/client/plugin/m...
File remoting/client/plugin/media_source_video_renderer.cc (right):

https://codereview.chromium.org/329663002/diff/20001/remoting/client/plugin/m...
remoting/client/plugin/media_source_video_renderer.cc:58:
segment_->set_max_cluster_size(0);
I don't understand what these lines do. Are they covered by the CL description?

https://codereview.chromium.org/329663002/diff/20001/remoting/client/plugin/m...
remoting/client/plugin/media_source_video_renderer.cc:221: // First bit
indicates I-frames.
s/I/key/?

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/client_p...
File remoting/webapp/client_plugin.js (right):

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/client_p...
remoting/webapp/client_plugin.js:331: var keyframe = !!message.data['keyframe'];
Does this pass jscompile? I'm surprised you don't need to include a type.

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/media_so...
File remoting/webapp/media_source_renderer.js (right):

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/media_so...
remoting/webapp/media_source_renderer.js:25: * processed  'null' indicates that
the SourceBuffer can be reset because
Nit: missing period after "processed".

Also, "null" does not refer to this.buffers_ itself, but to a null element
within it. I suggest phrasing it something like "A null element indicates..."

[Sergey Ulanov, 2014-06-11 23:41:12]

https://codereview.chromium.org/329663002/diff/20001/remoting/client/plugin/m...
File remoting/client/plugin/media_source_video_renderer.cc (right):

https://codereview.chromium.org/329663002/diff/20001/remoting/client/plugin/m...
remoting/client/plugin/media_source_video_renderer.cc:58:
segment_->set_max_cluster_size(0);
On 2014/06/11 21:41:42, Jamie wrote:
> I don't understand what these lines do. Are they covered by the CL
description?

That was my attempt to workaround crbug.com/382807, but it didn't work (the
muxer still generates new cluster for each keyframe). Removed now. The bug will
be fixed in chrome by this CL: https://codereview.chromium.org/328653002/

https://codereview.chromium.org/329663002/diff/20001/remoting/client/plugin/m...
remoting/client/plugin/media_source_video_renderer.cc:221: // First bit
indicates I-frames.
On 2014/06/11 21:41:42, Jamie wrote:
> s/I/key/?

Done.

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/client_p...
File remoting/webapp/client_plugin.js (right):

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/client_p...
remoting/webapp/client_plugin.js:331: var keyframe = !!message.data['keyframe'];
On 2014/06/11 21:41:42, Jamie wrote:
> Does this pass jscompile? I'm surprised you don't need to include a type.

Yes it does.

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/media_so...
File remoting/webapp/media_source_renderer.js (right):

https://codereview.chromium.org/329663002/diff/20001/remoting/webapp/media_so...
remoting/webapp/media_source_renderer.js:25: * processed  'null' indicates that
the SourceBuffer can be reset because
On 2014/06/11 21:41:43, Jamie wrote:
> Nit: missing period after "processed".
> 
> Also, "null" does not refer to this.buffers_ itself, but to a null element
> within it. I suggest phrasing it something like "A null element indicates..."

Done.

[Sergey Ulanov, 2014-06-11 23:41:16]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-06-11 23:43:08]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyu@chromium.org/329663002/60001

[commit-bot: I haz the power, 2014-06-12 01:09:56]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15784)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18916)

[commit-bot: I haz the power, 2014-06-12 01:37:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-12 01:37:23]

Try jobs failed on following builders:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18980)

[Sergey Ulanov, 2014-06-12 03:43:41]

The CQ bit was checked by sergeyu@chromium.org

[commit-bot: I haz the power, 2014-06-12 03:46:41]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sergeyu@chromium.org/329663002/60001

[commit-bot: I haz the power, 2014-06-12 07:58:35]

Change committed as 276574