Fix MSE GC, make it less aggressive, more spec-compliant

media/filters/chunk_demuxer.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/chunk_demuxer.h"
 
 #include <algorithm>
 #include <limits>
 #include <list>
 
@@ skipping 115 matching lines @@
 
   // Aborts the current append sequence and resets the parser.
   void Abort(TimeDelta append_window_start,
              TimeDelta append_window_end,
              TimeDelta* timestamp_offset);
 
   // Calls Remove(|start|, |end|, |duration|) on all
   // ChunkDemuxerStreams managed by this object.
   void Remove(TimeDelta start, TimeDelta end, TimeDelta duration);
 
+  // Attempts to perform garbage collection in SourceBuffers associated with
+  // this ChunkDemuxer. Returns true if garbage collection was successful.
+  // |media_time| parameter should indicate the current playback position,

[wolenetz, 2015/06/04 00:38:56]

nit: s/position, if/position. If/

[servolk, 2015/06/04 03:03:01]

Done.

+  // if it's a valid time value, the garbage collection algorithm will make sure
+  // to preserve unconsumed data buffers in the range between last read position
+  // and the current playback position.
+  bool EvictFrames(DecodeTimestamp media_time);
+
   // Returns true if currently parsing a media segment, or false otherwise.
   bool parsing_media_segment() const { return parsing_media_segment_; }
 
   // Sets |frame_processor_|'s sequence mode to |sequence_mode|.
   void SetSequenceMode(bool sequence_mode);
 
   // Signals the coded frame processor to update its group start timestamp to be
   // |timestamp_offset| if it is in sequence append mode.
   void SetGroupStartTimestampIfInSequenceMode(base::TimeDelta timestamp_offset);
 
@@ skipping 221 matching lines @@
 
   if (video_)
     video_->Remove(start, end, duration);
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->Remove(start, end, duration);
   }
 }
 
+bool SourceState::EvictFrames(DecodeTimestamp media_time) {
+  bool success = true;
+
+  if(audio_)

[wolenetz, 2015/06/04 00:38:56]

nit: spacing

[servolk, 2015/06/04 03:03:01]

Done.

+    success = audio_->EvictFrames(media_time) && success;
+
+  if (video_)
+    success = video_->EvictFrames(media_time) && success;
+
+  for (TextStreamMap::iterator itr = text_stream_map_.begin();
+       itr != text_stream_map_.end(); ++itr) {
+    success = itr->second->EvictFrames(media_time) && success;
+  }
+
+  return success;
+}
+
 Ranges<TimeDelta> SourceState::GetBufferedRanges(TimeDelta duration,
                                                  bool ended) const {
   // TODO(acolwell): When we start allowing disabled tracks we'll need to update
   // this code to only add ranges from active tracks.
   RangesList ranges_list;
   if (audio_)
     ranges_list.push_back(audio_->GetBufferedRanges(duration));
 
   if (video_)
     ranges_list.push_back(video_->GetBufferedRanges(duration));
@@ skipping 489 matching lines @@
 
   return true;
 }
 
 void ChunkDemuxerStream::Remove(TimeDelta start, TimeDelta end,
                                 TimeDelta duration) {
   base::AutoLock auto_lock(lock_);
   stream_->Remove(start, end, duration);
 }
 
+bool ChunkDemuxerStream::EvictFrames(DecodeTimestamp media_time) {
+  base::AutoLock auto_lock(lock_);
+  return stream_->GarbageCollectIfNeeded(media_time);
+}
+
 void ChunkDemuxerStream::OnSetDuration(TimeDelta duration) {
   base::AutoLock auto_lock(lock_);
   stream_->OnSetDuration(duration);
 }
 
 Ranges<TimeDelta> ChunkDemuxerStream::GetBufferedRanges(
     TimeDelta duration) const {
   base::AutoLock auto_lock(lock_);
 
   if (type_ == TEXT) {
@@ skipping 203 matching lines @@
       break;
   }
 
   base::ResetAndReturn(&read_cb_).Run(status, buffer);
 }
 
 ChunkDemuxer::ChunkDemuxer(
     const base::Closure& open_cb,
     const EncryptedMediaInitDataCB& encrypted_media_init_data_cb,
     const LogCB& log_cb,
+    const GetMediaTimeCB& get_media_time_cb,
     const scoped_refptr<MediaLog>& media_log,
     bool splice_frames_enabled)
     : state_(WAITING_FOR_INIT),
       cancel_next_seek_(false),
       host_(NULL),
       open_cb_(open_cb),
       encrypted_media_init_data_cb_(encrypted_media_init_data_cb),
       enable_text_(false),
       log_cb_(log_cb),
+      get_media_time_cb_(get_media_time_cb),
       media_log_(media_log),
       duration_(kNoTimestamp()),
       user_specified_duration_(-1),
       liveness_(DemuxerStream::LIVENESS_UNKNOWN),
       splice_frames_enabled_(splice_frames_enabled) {
   DCHECK(!open_cb_.is_null());
   DCHECK(!encrypted_media_init_data_cb_.is_null());
 }
 
 void ChunkDemuxer::Initialize(
@@ skipping 182 matching lines @@
 Ranges<TimeDelta> ChunkDemuxer::GetBufferedRanges(const std::string& id) const {
   base::AutoLock auto_lock(lock_);
   DCHECK(!id.empty());
 
   SourceStateMap::const_iterator itr = source_state_map_.find(id);
 
   DCHECK(itr != source_state_map_.end());
   return itr->second->GetBufferedRanges(duration_, state_ == ENDED);
 }
 
+bool ChunkDemuxer::EvictFrames() {
+  DecodeTimestamp current_media_time;
+  if (!get_media_time_cb_.is_null()) {
+    current_media_time =
+        DecodeTimestamp::FromPresentationTime(get_media_time_cb_.Run());

[wolenetz, 2015/06/04 00:38:57]

I'm not convinced the conversion from PTS to DTS is good for out-of-order
decodes and the scenario this CL is trying to fix. Are you also trying to fix
this scenario with out-of-order decode streams (like h264)?

[servolk, 2015/06/04 03:03:01]

The reason we are converting to DTS here is because buffered ranges are defined
in terms of DTS (SourceBufferRange::GetStartTimestamp/GetEndTimestamp return DTS
and most other processing in SBS seems to be in terms of DTS), but media
renderer returns current playback position as PTS. In order to figure out which
buffered range PTS belongs to, we need to compare it with range start/end DTS
values, so we need a DTS. I understand that PTS to DTS conversion doesn't make
actual adjustment for out-of-order decode streams, but I think in this case
that's fine, since we only need to figure out which buffered range playback
position belongs to. And as far as I understand single GOP always belongs
completely to some SourceBufferRange (i.e. a single GOP is never split between
two different SourceBufferRange objects). So I believe this should work
correctly both for regular in-order streams and out-of-order H.264 (or HEVC)
streams.

[wolenetz, 2015/06/04 20:06:50]

Ok. Thanks for considering this detail. As FYI, the incorrect usage of DTS
instead of PTS in older parts of Chromium MSE impl such as SourceBufferStream is
a known issue and tracked at least bug 373039 (plus bug 398130 and bug 402502).
Hence my original question since I didn't want this CL to regress behavior
further.

[servolk, 2015/06/04 20:22:46]

Acknowledged.

+  } else {
+    current_media_time = kNoDecodeTimestamp();
+  }
+
+  bool success = true;
+  for (SourceStateMap::iterator itr = source_state_map_.begin();
+       itr != source_state_map_.end(); ++itr) {
+    success = itr->second->EvictFrames(current_media_time) && success;
+  }
+  return success;
+}
+
 void ChunkDemuxer::AppendData(
     const std::string& id,
     const uint8* data,
     size_t length,
     TimeDelta append_window_start,
     TimeDelta append_window_end,
     TimeDelta* timestamp_offset,
     const InitSegmentReceivedCB& init_segment_received_cb) {
   DVLOG(1) << "AppendData(" << id << ", " << length << ")";
 
@@ skipping 510 matching lines @@
 }
 
 void ChunkDemuxer::ShutdownAllStreams() {
   for (SourceStateMap::iterator itr = source_state_map_.begin();
        itr != source_state_map_.end(); ++itr) {
     itr->second->Shutdown();
   }
 }
 
 }  // namespace media