From b2ffa50ab2602680d1cb8ae7ed0eda208dda378e Mon Sep 17 00:00:00 2001 From: "sergeyu@chromium.org" Date: Thu, 12 Jun 2014 07:58:28 +0000 Subject: 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 Review URL: https://codereview.chromium.org/329663002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@276574 0039d316-1c4b-4281-b951-d872f2087c98 --- remoting/client/plugin/chromoting_instance.cc | 5 ++-- remoting/client/plugin/chromoting_instance.h | 3 ++- .../client/plugin/media_source_video_renderer.cc | 18 ++++++++----- .../client/plugin/media_source_video_renderer.h | 3 ++- remoting/webapp/client_plugin.js | 4 ++- remoting/webapp/js_proto/dom_proto.js | 9 +++++++ remoting/webapp/media_source_renderer.js | 31 +++++++++++++++++----- 7 files changed, 55 insertions(+), 18 deletions(-) (limited to 'remoting') diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc index 2790229..b655546 100644 --- a/remoting/client/plugin/chromoting_instance.cc +++ b/remoting/client/plugin/chromoting_instance.cc @@ -1196,14 +1196,15 @@ void ChromotingInstance::OnMediaSourceReset(const std::string& format) { PostLegacyJsonMessage("mediaSourceReset", data.Pass()); } -void ChromotingInstance::OnMediaSourceData(uint8_t* buffer, - size_t buffer_size) { +void ChromotingInstance::OnMediaSourceData(uint8_t* buffer, size_t buffer_size, + bool keyframe) { pp::VarArrayBuffer array_buffer(buffer_size); void* data_ptr = array_buffer.Map(); memcpy(data_ptr, buffer, buffer_size); array_buffer.Unmap(); pp::VarDictionary data_dictionary; data_dictionary.Set(pp::Var("buffer"), array_buffer); + data_dictionary.Set(pp::Var("keyframe"), keyframe); PostChromotingMessage("mediaSourceData", data_dictionary); } diff --git a/remoting/client/plugin/chromoting_instance.h b/remoting/client/plugin/chromoting_instance.h index 3f6b0c6..0d4b17f 100644 --- a/remoting/client/plugin/chromoting_instance.h +++ b/remoting/client/plugin/chromoting_instance.h @@ -256,7 +256,8 @@ class ChromotingInstance : const webrtc::DesktopVector& dpi) OVERRIDE; virtual void OnMediaSourceShape(const webrtc::DesktopRegion& shape) OVERRIDE; virtual void OnMediaSourceReset(const std::string& format) OVERRIDE; - virtual void OnMediaSourceData(uint8_t* buffer, size_t buffer_size) OVERRIDE; + virtual void OnMediaSourceData(uint8_t* buffer, size_t buffer_size, + bool keyframe) OVERRIDE; bool initialized_; diff --git a/remoting/client/plugin/media_source_video_renderer.cc b/remoting/client/plugin/media_source_video_renderer.cc index 817df11..9d79c98 100644 --- a/remoting/client/plugin/media_source_video_renderer.cc +++ b/remoting/client/plugin/media_source_video_renderer.cc @@ -35,7 +35,8 @@ class MediaSourceVideoRenderer::VideoWriter : public mkvmuxer::IMkvWriter { virtual void ElementStartNotify(mkvmuxer::uint64 element_id, mkvmuxer::int64 position) OVERRIDE; - scoped_ptr OnVideoFrame(const std::string& video_data); + scoped_ptr OnVideoFrame(const std::string& video_data, + bool keyframe); private: webrtc::DesktopSize frame_size_; @@ -86,7 +87,7 @@ MediaSourceVideoRenderer::VideoWriter::VideoWriter( video_track->set_crop_bottom(crop_bottom); video_track->set_frame_rate(base::Time::kNanosecondsPerSecond / kFrameIntervalNs); - video_track->set_default_duration(base::Time::kNanosecondsPerSecond); + video_track->set_default_duration(kFrameIntervalNs); mkvmuxer::SegmentInfo* const info = segment_->GetSegmentInfo(); info->set_writing_app("ChromotingViewer"); info->set_muxing_app("ChromotingViewer"); @@ -124,13 +125,13 @@ void MediaSourceVideoRenderer::VideoWriter::ElementStartNotify( scoped_ptr MediaSourceVideoRenderer::VideoWriter::OnVideoFrame( - const std::string& video_data) { + const std::string& video_data, + bool keyframe) { DCHECK(!output_data_); output_data_.reset(new DataBuffer()); - bool first_frame = (timecode_ == 0); segment_->AddFrame(reinterpret_cast(video_data.data()), - video_data.size(), 1, timecode_, first_frame); + video_data.size(), 1, timecode_, keyframe); timecode_ += kFrameIntervalNs; return output_data_.Pass(); } @@ -215,9 +216,12 @@ void MediaSourceVideoRenderer::ProcessVideoPacket( delegate_->OnMediaSourceShape(desktop_shape_); } + // First bit is set to 0 for key frames. + bool keyframe = (packet->data()[0] & 1) == 0; + scoped_ptr buffer = - writer_->OnVideoFrame(packet->data()); - delegate_->OnMediaSourceData(&(*(buffer->begin())), buffer->size()); + writer_->OnVideoFrame(packet->data(), keyframe); + delegate_->OnMediaSourceData(&(*(buffer->begin())), buffer->size(), keyframe); } } // namespace remoting diff --git a/remoting/client/plugin/media_source_video_renderer.h b/remoting/client/plugin/media_source_video_renderer.h index 25d7d07..98e0b3e 100644 --- a/remoting/client/plugin/media_source_video_renderer.h +++ b/remoting/client/plugin/media_source_video_renderer.h @@ -38,7 +38,8 @@ class MediaSourceVideoRenderer : public VideoRenderer { virtual void OnMediaSourceReset(const std::string& format) = 0; // Called when new data becomes available. - virtual void OnMediaSourceData(uint8_t* buffer, size_t buffer_size) = 0; + virtual void OnMediaSourceData(uint8_t* buffer, size_t buffer_size, + bool keyframe) = 0; }; explicit MediaSourceVideoRenderer(Delegate* data_forwarder); diff --git a/remoting/webapp/client_plugin.js b/remoting/webapp/client_plugin.js index 7319e6b..9b04aec 100644 --- a/remoting/webapp/client_plugin.js +++ b/remoting/webapp/client_plugin.js @@ -327,8 +327,10 @@ remoting.ClientPlugin.prototype.handleMessageMethod_ = function(message) { console.error('Unexpected mediaSourceData.'); return; } + // keyframe flag may be absent from the message. + var keyframe = !!message.data['keyframe']; this.mediaSourceRenderer_.onIncomingData( - (/** @type {ArrayBuffer} */ message.data['buffer'])); + (/** @type {ArrayBuffer} */ message.data['buffer']), keyframe); } }; diff --git a/remoting/webapp/js_proto/dom_proto.js b/remoting/webapp/js_proto/dom_proto.js index 1e392cb..1877b50 100644 --- a/remoting/webapp/js_proto/dom_proto.js +++ b/remoting/webapp/js_proto/dom_proto.js @@ -151,12 +151,21 @@ var SourceBuffer = function() {} /** @type {boolean} */ SourceBuffer.prototype.updating; +/** @type {TimeRanges} */ +SourceBuffer.prototype.buffered; + /** * @param {ArrayBuffer} buffer */ SourceBuffer.prototype.appendBuffer = function(buffer) {} /** + * @param {number} start + * @param {number} end + */ +SourceBuffer.prototype.remove = function(start, end) {} + +/** * @constructor * @extends {EventTargetStub} */ diff --git a/remoting/webapp/media_source_renderer.js b/remoting/webapp/media_source_renderer.js index a79173b..1bbb13e 100644 --- a/remoting/webapp/media_source_renderer.js +++ b/remoting/webapp/media_source_renderer.js @@ -22,7 +22,8 @@ remoting.MediaSourceRenderer = function(videoTag) { this.sourceBuffer_ = null; /** @type {!Array.} Queue of pending buffers that haven't been - * processed . */ + * processed. A null element indicates that the SourceBuffer can be reset + * because the following buffer contains a keyframe. */ this.buffers_ = []; } @@ -68,18 +69,36 @@ remoting.MediaSourceRenderer.prototype.onSourceOpen_ = function(format) { remoting.MediaSourceRenderer.prototype.processPendingData_ = function() { if (this.sourceBuffer_) { while (this.buffers_.length > 0 && !this.sourceBuffer_.updating) { - // TODO(sergeyu): Figure out the way to determine when a frame is rendered - // and use it to report performance statistics. - this.sourceBuffer_.appendBuffer( - /** @type {ArrayBuffer} */(this.buffers_.shift())); + var buffer = /** @type {ArrayBuffer} */ this.buffers_.shift(); + if (buffer == null) { + // Remove all data from the SourceBuffer. By default Chrome buffers up + // 150MB of data in SourceBuffer. We never need to seek the stream, so + // it doesn't make sense to keep any of that data. + if (this.sourceBuffer_.buffered.length > 0) { + this.sourceBuffer_.remove( + this.sourceBuffer_.buffered.start(0), + this.sourceBuffer_.buffered.end( + this.sourceBuffer_.buffered.length - 1)); + } + } else { + // TODO(sergeyu): Figure out the way to determine when a frame is + // rendered and use it to report performance statistics. + this.sourceBuffer_.appendBuffer(buffer); + } } } } /** * @param {ArrayBuffer} data + * @param {boolean} keyframe */ -remoting.MediaSourceRenderer.prototype.onIncomingData = function(data) { +remoting.MediaSourceRenderer.prototype.onIncomingData = + function(data, keyframe) { + if (keyframe) { + // Queue SourceBuffer reset request. + this.buffers_.push(null); + } this.buffers_.push(data); this.processPendingData_(); } -- cgit v1.1