Return EOS when selected_range_ is NULL in SourceBufferStream

media/filters/source_buffer_stream.cc

Index: media/filters/source_buffer_stream.cc
diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 4271ab9601bb44393a6f79f0db3fd09daf7c00fd..10f83892a554897af7ab4a590e1f44c6baf0404b 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -928,7 +928,7 @@ void SourceBufferStream::Seek(base::TimeDelta timestamp) {
 }
 
 bool SourceBufferStream::IsSeekPending() const {
-  return !(end_of_stream_ && IsEndSelected()) && seek_pending_;
+  return !IsEndOfStreamReached() && seek_pending_;

[wolenetz, 2015/05/12 20:36:24]

nit: please swap the order of the evaluation here to enable rapid short-circuit
evaluation in the common case of seek_pending_ == false.

[landell, 2015/05/13 09:55:56]

Done.

 }
 
 void SourceBufferStream::OnSetDuration(base::TimeDelta duration) {
@@ -1099,7 +1099,7 @@ SourceBufferStream::Status SourceBufferStream::GetNextBufferInternal(
   }
 

[wolenetz, 2015/05/12 20:36:23]

nit: probably good for protection against later regression to add
DCHECK(track_buffer_.empty()) here now.

[landell, 2015/05/13 09:55:56]

Done.

   if (!selected_range_ || !selected_range_->HasNextBuffer()) {
-    if (end_of_stream_ && IsEndSelected())
+    if (IsEndOfStreamReached())
       return kEndOfStream;
     DVLOG(3) << __FUNCTION__ << " " << GetStreamTypeName()
              << ": returning kNeedBuffer "
@@ -1205,7 +1205,10 @@ void SourceBufferStream::UnmarkEndOfStream() {
   end_of_stream_ = false;
 }
 
-bool SourceBufferStream::IsEndSelected() const {
+bool SourceBufferStream::IsEndOfStreamReached() const {
+  if (!end_of_stream_)
+    return false;
+
   if (ranges_.empty())

[wolenetz, 2015/05/12 20:36:23]

nit: I think it might be possible in an edge case (or in later regression) to
have something in track_buffer_, but for ranges_ to be empty. In this case, we
haven't reached end of stream since a non-empty track_buffer_ has more buffers
to be served by GetNextBuffer() still.

Suggestion: change line 1209 to be:
  if (!end_of_stream_ || !track_buffer_.empty())
and remove the track_buffer_.empty() condition from line 1221.

[landell, 2015/05/13 09:55:56]

Done.

     return true;
 
@@ -1215,6 +1218,9 @@ bool SourceBufferStream::IsEndSelected() const {
     return seek_buffer_timestamp_ >= last_range_end_time;
   }
 
+  if (!selected_range_ && track_buffer_.empty())

[wolenetz, 2015/05/12 20:36:24]

nit: see my comment, above.

[landell, 2015/05/13 09:55:56]

Done.

+    return true;
+
   return selected_range_ == ranges_.back();
 }