Fix ChunkDemuxer so it properly outputs buffered ranges.

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 "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/string_util.h"
@@ skipping 55 matching lines @@
   return new mp4::MP4StreamParser();
 }
 
 static const SupportedTypeInfo kSupportedTypeInfo[] = {
   { "video/webm", &BuildWebMParser, kVideoWebMCodecs },
   { "audio/webm", &BuildWebMParser, kAudioWebMCodecs },
   { "video/mp4", &BuildMP4Parser, kVideoMP4Codecs },
   { "audio/mp4", &BuildMP4Parser, kAudioMP4Codecs },
 };
 
+
+// The fake total size we use for converting times to bytes
+// for AddBufferedByteRange() calls.
+enum { kFakeTotalBytes = 1000000 };

[Ami GONE FROM CHROMIUM, 2012/06/19 17:40:37]

TODO to drop this in favor of teaching Pipeline to accept buffered times instead
of buffered bytes?

[acolwell GONE FROM CHROMIUM, 2012/06/19 19:50:15]

Done.

+
 // Checks to see if the specified |type| and |codecs| list are supported.
 // Returns true if |type| and all codecs listed in |codecs| are supported.
 //         |factory_function| contains a function that can build a StreamParser
 //                            for this type.
 //         |has_audio| is true if an audio codec was specified.
 //         |has_video| is true if a video codec was specified.
 // Returns false otherwise. The values of |factory_function|, |has_audio|,
 //         and |has_video| are undefined.
 static bool IsSupported(const std::string& type,
                         std::vector<std::string>& codecs,
@@ skipping 54 matching lines @@
 class ChunkDemuxerStream : public DemuxerStream {
  public:
   typedef std::deque<scoped_refptr<StreamParserBuffer> > BufferQueue;
   typedef std::deque<ReadCB> ReadCBQueue;
   typedef std::deque<base::Closure> ClosureQueue;
 
   explicit ChunkDemuxerStream(const AudioDecoderConfig& audio_config);
   explicit ChunkDemuxerStream(const VideoDecoderConfig& video_config);
 
   void StartWaitingForSeek();
   void Seek(base::TimeDelta time);

[Ami GONE FROM CHROMIUM, 2012/06/19 17:40:37]

At 21 occurrences in this file, I'd say base::TimeDelta has earned a using
statement (to be able to drop base:: from the use sites).  Your choice.

[acolwell GONE FROM CHROMIUM, 2012/06/19 19:50:15]

Done.

   bool IsSeekPending() const;
   void Flush();
 
   // Add buffers to this stream.  Buffers are stored in SourceBufferStreams,
   // which handle ordering and overlap resolution.
   // Returns true if buffers were successfully added.
   bool Append(const StreamParser::BufferQueue& buffers);
 
   // Returns a list of the buffered time ranges.
-  SourceBufferStream::TimespanList GetBufferedTime() const;
+  Ranges<base::TimeDelta> GetBufferedTime() const;
 
   // Signal to the stream that buffers handed in through subsequent calls to
   // Append() belong to a media segment that starts at |start_timestamp|.
   void OnNewMediaSegment(base::TimeDelta start_timestamp);
 
   // Called when mid-stream config updates occur.
   // Returns true if the new config is accepted.
   // Returns false if the new config should trigger an error.
   bool UpdateAudioConfig(const AudioDecoderConfig& config);
   bool UpdateVideoConfig(const VideoDecoderConfig& config);
@@ skipping 112 matching lines @@
     }
     CreateReadDoneClosures_Locked(&closures);
   }
 
   for (ClosureQueue::iterator it = closures.begin(); it != closures.end(); ++it)
     it->Run();
 
   return true;
 }
 
-SourceBufferStream::TimespanList ChunkDemuxerStream::GetBufferedTime() const {
+Ranges<base::TimeDelta> ChunkDemuxerStream::GetBufferedTime() const {
   base::AutoLock auto_lock(lock_);
   return stream_->GetBufferedTime();
 }
 
 bool ChunkDemuxerStream::UpdateAudioConfig(const AudioDecoderConfig& config) {
   DCHECK(config.IsValidConfig());
   DCHECK_EQ(type_, AUDIO);
 
   const AudioDecoderConfig& current_config =
       stream_->GetCurrentAudioDecoderConfig();
@@ skipping 152 matching lines @@
     if (!stream_->GetNextBuffer(&buffer))
       return;
     closures->push_back(base::Bind(read_cbs_.front(), buffer));
     read_cbs_.pop_front();
   }
 }
 
 ChunkDemuxer::ChunkDemuxer(ChunkDemuxerClient* client)
     : state_(WAITING_FOR_INIT),
       host_(NULL),
-      client_(client),
-      buffered_bytes_(0) {
+      client_(client) {
   DCHECK(client);
 }
 
 void ChunkDemuxer::Initialize(DemuxerHost* host,
                               const PipelineStatusCB& cb) {
   DVLOG(1) << "Init()";
   {
     base::AutoLock auto_lock(lock_);
     DCHECK_EQ(state_, WAITING_FOR_INIT);
     host_ = host;
@@ skipping 142 matching lines @@
   delete stream_parser_map_[id];
   stream_parser_map_.erase(id);
 
   if (source_id_audio_ == id && audio_)
     audio_->Shutdown();
 
   if (source_id_video_ == id && video_)
     video_->Shutdown();
 }
 
-bool ChunkDemuxer::GetBufferedRanges(const std::string& id,
-                                     Ranges* ranges_out) const {
+Ranges<base::TimeDelta> ChunkDemuxer::GetBufferedRanges(
+    const std::string& id) const {
   DCHECK(!id.empty());
   DCHECK_GT(stream_parser_map_.count(id), 0u);
   DCHECK(id == source_id_audio_ || id == source_id_video_);
-  DCHECK(ranges_out);
 
   base::AutoLock auto_lock(lock_);
 
   if (id == source_id_audio_ && id != source_id_video_) {
     // Only include ranges that have been buffered in |audio_|
-    return CopyIntoRanges(audio_->GetBufferedTime(), ranges_out);
+    return audio_->GetBufferedTime();
   }
 
   if (id != source_id_audio_ && id == source_id_video_) {
     // Only include ranges that have been buffered in |video_|
-    return CopyIntoRanges(video_->GetBufferedTime(), ranges_out);
+    return video_->GetBufferedTime();
   }
 
+  return ComputeIntersection();
+}
+
+Ranges<base::TimeDelta> ChunkDemuxer::ComputeIntersection() const {
+  lock_.AssertAcquired();
+
+  if (!audio_ || !video_)
+    return Ranges<base::TimeDelta>();
+
   // Include ranges that have been buffered in both |audio_| and |video_|.
-  SourceBufferStream::TimespanList audio_ranges = audio_->GetBufferedTime();
-  SourceBufferStream::TimespanList video_ranges = video_->GetBufferedTime();
-  SourceBufferStream::TimespanList::const_iterator video_ranges_itr =
-      video_ranges.begin();
-  SourceBufferStream::TimespanList::const_iterator audio_ranges_itr =
-      audio_ranges.begin();
-  bool success = false;
+  Ranges<base::TimeDelta> audio_ranges = audio_->GetBufferedTime();
+  Ranges<base::TimeDelta> video_ranges = video_->GetBufferedTime();
+  Ranges<base::TimeDelta> result = audio_ranges.IntersectionWith(video_ranges);
 
-  while (audio_ranges_itr != audio_ranges.end() &&
-         video_ranges_itr != video_ranges.end()) {
-    // If this is the last range and EndOfStream() was called (i.e. all data
-    // has been appended), choose the max end point of the ranges.
-    bool last_range_after_ended =
-        state_ == ENDED &&
-        (audio_ranges_itr + 1) == audio_ranges.end() &&
-        (video_ranges_itr + 1) == video_ranges.end();
+  if (state_ == ENDED) {
+    // If appending has ended, extend the intersection to include all of the
+    // data from the last ranges for each stream. This allows the buffered
+    // information to match the actual time range that will get played out if
+    // the streams have slightly different lengths. We only do this if the
+    // final ranges overlap because this indicates there is actually a location
+    // you can seek to, near the end, where we have data for both streams.
+    base::TimeDelta audio_start = audio_ranges.start(audio_ranges.size() - 1);
+    base::TimeDelta audio_end = audio_ranges.end(audio_ranges.size() - 1);
+    base::TimeDelta video_start = video_ranges.start(video_ranges.size() - 1);
+    base::TimeDelta video_end = video_ranges.end(video_ranges.size() - 1);
 
-    // Audio range start time is within the video range.
-    if ((*audio_ranges_itr).first >= (*video_ranges_itr).first &&
-        (*audio_ranges_itr).first <= (*video_ranges_itr).second) {
-      AddIntersectionRange(*audio_ranges_itr, *video_ranges_itr,
-                           last_range_after_ended, ranges_out);
-      audio_ranges_itr++;
-      success = true;
-      continue;
+    if ((audio_start <= video_start && video_start <= audio_end) ||
+        (video_start <= audio_start && audio_start <= video_end)) {
+      result.Add(std::max(audio_start, video_start),

[Ami GONE FROM CHROMIUM, 2012/06/19 17:40:37]

should this be a "min" instead of a max?  Otherwise l.667-668 is a lie - not all
the data for from the last ranges is added.

But if the comment reflects what you really want to happen, why not replace
l.673-681 with simply adding the last range from each stream to result and let
range coallescing do its thing?

[acolwell GONE FROM CHROMIUM, 2012/06/19 19:50:15]

I don't want min here. I only want the intersection plus whatever extra data
exists past the end of the intersection. 
The intersection represents a region where I can actually play both streams, but
in the end of stream case the data beyond the intersection will also get played
if the streams happen to be different lengths. If I used min here buffered would
lie about what ranges are actually playable because It would reflect starting
positions that only have data for one of the streams.

+                 std::max(audio_end, video_end));
     }
-
-    // Video range start time is within the audio range.
-    if ((*video_ranges_itr).first >= (*audio_ranges_itr).first &&
-        (*video_ranges_itr).first <= (*audio_ranges_itr).second) {
-      AddIntersectionRange(*video_ranges_itr, *audio_ranges_itr,
-                           last_range_after_ended, ranges_out);
-      video_ranges_itr++;
-      success = true;
-      continue;
-    }
-
-    // No overlap was found.  Increment the earliest one and keep looking.
-    if ((*audio_ranges_itr).first < (*video_ranges_itr).first)
-      audio_ranges_itr++;
-    else
-      video_ranges_itr++;
   }
 
-  return success;
-}
-
-bool ChunkDemuxer::CopyIntoRanges(
-    const SourceBufferStream::TimespanList& timespans,
-    Ranges* ranges_out) const {
-  for (SourceBufferStream::TimespanList::const_iterator itr = timespans.begin();
-       itr != timespans.end(); ++itr) {
-    ranges_out->push_back(*itr);
-  }
-  return !timespans.empty();
-}
-
-void ChunkDemuxer::AddIntersectionRange(
-    SourceBufferStream::Timespan timespan_a,
-    SourceBufferStream::Timespan timespan_b,
-    bool last_range_after_ended,
-    Ranges* ranges_out) const {
-  base::TimeDelta start = timespan_a.first;
-
-  // If this is the last range after EndOfStream() was called, choose the later
-  // end point of the ranges, otherwise choose the earlier.
-  base::TimeDelta end;
-  if (last_range_after_ended)
-    end = std::max(timespan_a.second, timespan_b.second);
-  else
-    end = std::min(timespan_a.second, timespan_b.second);
-
-  ranges_out->push_back(std::make_pair(start, end));
+  return result;
 }
 
 bool ChunkDemuxer::AppendData(const std::string& id,
                               const uint8* data,
                               size_t length) {
   DVLOG(1) << "AppendData(" << id << ", " << length << ")";
 
   DCHECK(!id.empty());
   DCHECK(data);
   DCHECK_GT(length, 0u);
 
-  int64 buffered_bytes = 0;
+  Ranges<base::TimeDelta> ranges;
 
   PipelineStatusCB cb;
   {
     base::AutoLock auto_lock(lock_);
 
     // Capture if the SourceBuffer has a pending seek before we start parsing.
     bool old_seek_pending = IsSeekPending_Locked();
 
     switch (state_) {
       case INITIALIZING:
@@ skipping 20 matching lines @@
         DVLOG(1) << "AppendData(): called in unexpected state " << state_;
         return false;
     }
 
     // Check to see if data was appended at the pending seek point. This
     // indicates we have parsed enough data to complete the seek.
     if (old_seek_pending && !IsSeekPending_Locked() && !seek_cb_.is_null()) {
       std::swap(cb, seek_cb_);
     }
 
-    buffered_bytes_ += length;
-    buffered_bytes = buffered_bytes_;
+    if (duration_ > base::TimeDelta() && duration_ != kInfiniteDuration()) {
+      if (audio_ && !video_) {
+        ranges = audio_->GetBufferedTime();
+      } else if (!audio_ && video_) {
+        ranges = video_->GetBufferedTime();
+      } else {
+        ranges = ComputeIntersection();
+      }
+    }
   }
 
-  // Notify the host of 'network activity' because we got data, using a bogus
-  // range.
-  host_->AddBufferedByteRange(0, buffered_bytes);
+  for (size_t i = 0; i < ranges.size(); ++i) {
+    DCHECK(duration_ > base::TimeDelta());

[Ami GONE FROM CHROMIUM, 2012/06/19 17:40:37]

silly to do this inside the for-loop; move up as:
DCHECK(ranges.empty() || duration_ > base::TimeDelta());
?

[acolwell GONE FROM CHROMIUM, 2012/06/19 19:50:15]

Done. Used !ranges.size() since empty() doesn't exist.

+
+    // Notify the host of 'network activity' because we got data.
+    int64 start =
+        kFakeTotalBytes * ranges.start(i).InSecondsF() / duration_.InSecondsF();
+    int64 end =
+        kFakeTotalBytes * ranges.end(i).InSecondsF() / duration_.InSecondsF();
+    host_->AddBufferedByteRange(start, end);
+  }
 
   if (!cb.is_null())
     cb.Run(PIPELINE_OK);
 
   return true;
 }
 
 void ChunkDemuxer::Abort(const std::string& id) {
   DVLOG(1) << "Abort(" << id << ")";
   DCHECK(!id.empty());
@@ skipping 148 matching lines @@
   }
 
   if (duration > duration_)
     duration_ = duration;
 
   // Wait until all streams have initialized.
   if ((!source_id_audio_.empty() && !audio_) ||
       (!source_id_video_.empty() && !video_))
     return;
 
+  if (duration_ > base::TimeDelta() && duration_ != kInfiniteDuration())
+    host_->SetTotalBytes(kFakeTotalBytes);
   host_->SetDuration(duration_);
 
   ChangeState_Locked(INITIALIZED);
   PipelineStatusCB cb;
   std::swap(cb, init_cb_);
   cb.Run(PIPELINE_OK);
 }
 
 bool ChunkDemuxer::OnNewConfigs(bool has_audio, bool has_video,
                                 const AudioDecoderConfig& audio_config,
@@ skipping 68 matching lines @@
   // TODO(vrk): There should be a special case for the first appends where all
   // streams (for both demuxed and muxed case) begin at the earliest stream
   // timestamp. (crbug.com/132815)
   if (audio_ && source_id == source_id_audio_)
     audio_->OnNewMediaSegment(start_timestamp);
   if (video_ && source_id == source_id_video_)
     video_->OnNewMediaSegment(start_timestamp);
 }
 
 }  // namespace media