Reland: MSE: Relax the 'media segment must begin with keyframe' requirement

media/filters/source_buffer_range.h

Index: media/filters/source_buffer_range.h
diff --git a/media/filters/source_buffer_range.h b/media/filters/source_buffer_range.h
index c29ed4f29accdad120da717d26faa87b051d32f6..6989763067b2e742e6fdd8d503a304fb8c1ce949 100644
--- a/media/filters/source_buffer_range.h
+++ b/media/filters/source_buffer_range.h
@@ -37,17 +37,23 @@ class SourceBufferRange {
     ALLOW_GAPS
   };
 
-  // Buffers with the same timestamp are only allowed under certain conditions.
-  // More precisely, it is allowed in all situations except when the previous
-  // frame is not a key frame and the current is a key frame.
-  // Examples of situations where DTS of two consecutive frames can be equal:
+  // Sequential buffers with the same decode timestamp make sense under certain
+  // conditions, typically when the first buffer is a keyframe. Due to some
+  // atypical media append behaviors where a new keyframe might have the same
+  // timestamp as a previous non-keyframe, the playback of the sequence might
+  // involve some throwaway decode work. This method supports detecting this
+  // situation so that callers can log warnings (it returns true in this case
+  // only).
+  // For all other cases, including more typical same-DTS sequences, this method
+  // returns false. Examples of typical situations where DTS of two consecutive
+  // frames can be equal:
   // - Video: VP8 Alt-Ref frames.
   // - Video: IPBPBP...: DTS for I frame and for P frame can be equal.
   // - Text track cues that start at same time.
   // Returns true if |prev_is_keyframe| and |current_is_keyframe| indicate a
-  // same timestamp situation that is allowed. False is returned otherwise.
-  static bool AllowSameTimestamp(bool prev_is_keyframe,
-                                 bool current_is_keyframe);
+  // same timestamp situation that is atypical. False is returned otherwise.
+  static bool IsUncommonSameTimestampSequence(bool prev_is_keyframe,

[wolenetz, 2016/02/05 23:27:24]

Note that I've inverted the logic and the name here. This is handled at the
callers correctly.

+                                              bool current_is_keyframe);
 
   // Creates a source buffer range with |new_buffers|. |new_buffers| cannot be
   // empty and the front of |new_buffers| must be a keyframe.
@@ -198,8 +204,8 @@ class SourceBufferRange {
   bool EndOverlaps(const SourceBufferRange& range) const;
 
   // Returns true if |timestamp| is the timestamp of the next buffer in
-  // sequence after |buffers_.back()|, false otherwise.
-  bool IsNextInSequence(DecodeTimestamp timestamp, bool is_key_frame) const;
+  // sequence after |buffers_.back()|, false othewise.

[wolenetz, 2016/02/05 23:27:24]

nit-to-self: fix this typo.

[wolenetz, 2016/02/12 01:23:47]

Done.

+  bool IsNextInSequence(DecodeTimestamp timestamp) const;
 
   // Adds all buffers which overlap [start, end) to the end of |buffers|.  If
   // no buffers exist in the range returns false, true otherwise.