Update media duration if data is appended after the previous duration

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/callback_helpers.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ skipping 505 matching lines @@
 }
 
 ChunkDemuxer::ChunkDemuxer(ChunkDemuxerClient* client)
     : state_(WAITING_FOR_INIT),
       host_(NULL),
       client_(client),
       start_time_(kNoTimestamp()) {
   DCHECK(client);
 }
 
-void ChunkDemuxer::Initialize(DemuxerHost* host,
-                              const PipelineStatusCB& cb) {
+void ChunkDemuxer::Initialize(DemuxerHost* host, const PipelineStatusCB& cb) {
   DVLOG(1) << "Init()";
   {
     base::AutoLock auto_lock(lock_);
     DCHECK_EQ(state_, WAITING_FOR_INIT);
     host_ = host;
 
     ChangeState_Locked(INITIALIZING);
     init_cb_ = cb;
   }
 
@@ skipping 243 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_);
     }
 
-    if (audio_ && !video_) {
-      ranges = audio_->GetBufferedRanges();
-    } else if (!audio_ && video_) {
-      ranges = video_->GetBufferedRanges();
-    } else {
-      ranges = ComputeIntersection();
-    }
+    ranges = GetBufferedRanges();
   }
 
   for (size_t i = 0; i < ranges.size(); ++i)
     host_->AddBufferedTimeRange(ranges.start(i), ranges.end(i));
 
   if (!cb.is_null())
     cb.Run(PIPELINE_OK);
 
   return true;
 }
@@ skipping 153 matching lines @@
   DVLOG(1) << "OnStreamParserInitDone(" << success << ", "
            << duration.InSecondsF() << ")";
   lock_.AssertAcquired();
   DCHECK_EQ(state_, INITIALIZING);
   if (!success || (!audio_ && !video_)) {
     ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
     return;
   }
 
   if (duration > duration_)
-    duration_ = duration;
+    UpdateDuration(duration);
 
   // Wait until all streams have initialized.
   if ((!source_id_audio_.empty() && !audio_) ||
       (!source_id_video_.empty() && !video_))
     return;
 
-  host_->SetDuration(duration_);
-
   ChangeState_Locked(WAITING_FOR_START_TIME);
 }
 
 bool ChunkDemuxer::OnNewConfigs(bool has_audio, bool has_video,
                                 const AudioDecoderConfig& audio_config,
                                 const VideoDecoderConfig& video_config) {
   DVLOG(1) << "OnNewConfigs(" << has_audio << ", " << has_video
            << ", " << audio_config.IsValidConfig()
            << ", " << video_config.IsValidConfig() << ")";
   CHECK(audio_config.IsValidConfig() || video_config.IsValidConfig());
@@ skipping 38 matching lines @@
   lock_.AssertAcquired();
   DCHECK_NE(state_, SHUTDOWN);
 
   if (!audio_)
     return false;
 
   CHECK(IsValidId(source_id_audio_));
   AdjustBufferTimestamps(
       buffers, source_info_map_[source_id_audio_].timestamp_offset);
 
+  IncreaseDurationIfNecessary(buffers);
+
   return audio_->Append(buffers);
 }
 
 bool ChunkDemuxer::OnVideoBuffers(const StreamParser::BufferQueue& buffers) {
   lock_.AssertAcquired();
   DCHECK_NE(state_, SHUTDOWN);
 
   if (!video_)
     return false;
 
   CHECK(IsValidId(source_id_video_));
   AdjustBufferTimestamps(
       buffers, source_info_map_[source_id_video_].timestamp_offset);
 
+  IncreaseDurationIfNecessary(buffers);
+
   return video_->Append(buffers);
 }
 
 bool ChunkDemuxer::OnNeedKey(scoped_array<uint8> init_data,
                              int init_data_size) {
   client_->DemuxerNeedKey(init_data.Pass(), init_data_size);
   return true;
 }
 
 void ChunkDemuxer::OnNewMediaSegment(const std::string& source_id,
@@ skipping 24 matching lines @@
 
   if (audio_) {
     audio_->SetStartTime(start_time_);
     audio_->Seek(start_time_);
   }
   if (video_) {
     video_->SetStartTime(start_time_);
     video_->Seek(start_time_);
   }
 
+  if (start_time_ > duration_)
+    UpdateDuration(start_time_);

[acolwell GONE FROM CHROMIUM, 2012/07/27 00:03:23]

nit: Could this be up where start_time_ is initially set?
Should we have a comment here indicating that this should be changed to
(start_time - start_time_) once http://crbug.com/137275 is addressed?

[vrk (LEFT CHROMIUM), 2012/07/28 01:42:05]

Done.

+
   // The demuxer is now initialized after the |start_timestamp_| was set.
   ChangeState_Locked(INITIALIZED);
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 void ChunkDemuxer::OnEndOfMediaSegment(const std::string& source_id) {
   DVLOG(2) << "OnEndOfMediaSegment(" << source_id << ")";
   CHECK(IsValidId(source_id));
   source_info_map_[source_id].can_update_offset = true;
 }
@@ skipping 10 matching lines @@
         (*itr)->GetDecodeTimestamp() + timestamp_offset);
     (*itr)->SetTimestamp((*itr)->GetTimestamp() + timestamp_offset);
   }
 }
 
 bool ChunkDemuxer::IsValidId(const std::string& source_id) const {
   return source_info_map_.count(source_id) > 0u &&
       stream_parser_map_.count(source_id) > 0u;
 }
 
+void ChunkDemuxer::UpdateDuration(base::TimeDelta new_duration) {
+  duration_ = new_duration;

[acolwell GONE FROM CHROMIUM, 2012/07/27 00:03:23]

DCHECK(duration_ != new_duration) to make sure code only notifies on changes?

[vrk (LEFT CHROMIUM), 2012/07/28 01:42:05]

Done.

+  host_->SetDuration(new_duration);

[acolwell GONE FROM CHROMIUM, 2012/07/27 00:03:23]

Does this cause double duration notifications on muxed content appended to the
end? If so I wonder if we should defer notifying the host until the end of
AppendData() & EndOfStream()?

[vrk (LEFT CHROMIUM), 2012/07/28 01:42:05]

Yes this happens!
Talked offline; it's a little icky to do this elegantly so I'll keep as-is for
now!

+}
+
+void ChunkDemuxer::IncreaseDurationIfNecessary(
+    const StreamParser::BufferQueue& buffers) {
+  DCHECK(!buffers.empty());
+  if (buffers.back()->GetTimestamp() <= duration_)
+    return;
+
+  Ranges<TimeDelta> ranges = GetBufferedRanges();
+  base::TimeDelta last_timestamp_buffered = ranges.end(ranges.size() - 1);
+  DCHECK(last_timestamp_buffered > duration_);

[acolwell GONE FROM CHROMIUM, 2012/07/27 00:03:23]

I don't think this is right. The buffers aren't appended yet so they won't be
reflected by GetBufferedRanges(). Also calling GetBufferedRanges() seems a
little heavy-weight here especially since the caller knows whether the buffers
are being appended to audio_ or video_. You could just pass a ChunkDemuxerStream
pointer to this method and call GetBufferedRanges() on that.

[vrk (LEFT CHROMIUM), 2012/07/28 01:42:05]

AHH you are right! I must have fiddled with this before testing, this breaks
pretty immediately when tried out :) 

Fixed, and the duration change tests I added also fail with this un-fixed.
Done, but FYI this changes the behavior a little bit. If in a muxed stream, the
audio data ends at 25s and the video ends at 30s  (and the original duration
was, say, 20s), the code here would say the duration is 25s (which matches
sourceBuffered()), while your suggested changes makes the duration 30s. I think
30s makes more sense, though it would mean that the duration would always exceed
sourceBuffered() if the audio and video don't line up perfectly at the end. Do
you have thoughts on this?

+  UpdateDuration(last_timestamp_buffered);
+}
+
+Ranges<TimeDelta> ChunkDemuxer::GetBufferedRanges() const {
+  lock_.AssertAcquired();
+
+  if (audio_ && !video_)
+    return audio_->GetBufferedRanges();
+  else if (!audio_ && video_)
+    return video_->GetBufferedRanges();
+
+  return ComputeIntersection();
+}
+
 }  // namespace media