Add support for partial append window end trimming.

media/filters/frame_processor.cc

Index: media/filters/frame_processor.cc
diff --git a/media/filters/frame_processor.cc b/media/filters/frame_processor.cc
index a10ed5017ed50814a2269dd0fde06d8fa3189618..1b92c43ed4a6f4f2d87249157cdf97e99cd1b0d7 100644
--- a/media/filters/frame_processor.cc
+++ b/media/filters/frame_processor.cc
@@ -332,21 +332,11 @@ bool FrameProcessor::HandlePartialAppendWindowTrimming(
     const scoped_refptr<StreamParserBuffer>& buffer) {
   DCHECK(buffer->duration() > base::TimeDelta());
   DCHECK_EQ(DemuxerStream::AUDIO, buffer->type());
+  DCHECK(buffer->IsKeyframe());
 
   const base::TimeDelta frame_end_timestamp =
       buffer->timestamp() + buffer->duration();
 
-  // Ignore any buffers which start after |append_window_start| or end after
-  // |append_window_end|.  For simplicity, even those that start before
-  // |append_window_start|.
-  if (buffer->timestamp() > append_window_start ||
-      frame_end_timestamp > append_window_end) {
-    // TODO(dalecurtis): Partial append window trimming could also be done
-    // around |append_window_end|, but is not necessary since splice frames
-    // cover overlaps there.
-    return false;
-  }
-
   // If the buffer is entirely before |append_window_start|, save it as preroll
   // for the first buffer which overlaps |append_window_start|.
   if (buffer->timestamp() < append_window_start &&
@@ -355,28 +345,27 @@ bool FrameProcessor::HandlePartialAppendWindowTrimming(
     return false;
   }
 
-  // There's nothing to be done if we have no preroll and the buffer starts on
-  // the append window start.
-  if (buffer->timestamp() == append_window_start && !audio_preroll_buffer_)
+  // If the buffer is entirely after |append_window_end| there's nothing to do.
+  if (buffer->timestamp() > append_window_end)

[wolenetz, 2014/07/30 22:04:32]

s/>/>=. append_window_start cannot == append_window_end per spec. So buffer is
strictly outside append window even if it starts at the append window end time.

[DaleCurtis, 2014/07/30 22:52:00]

Done.

     return false;
 
-  // See if a partial discard can be done around |append_window_start|.
-  DCHECK(buffer->timestamp() <= append_window_start);
-  DCHECK(buffer->IsKeyframe());
-  DVLOG(1) << "Truncating buffer which overlaps append window start."
-           << " presentation_timestamp " << buffer->timestamp().InSecondsF()
-           << " append_window_start " << append_window_start.InSecondsF();
+  DCHECK(buffer->timestamp() >= append_window_start ||
+         frame_end_timestamp > append_window_start);
 
-  // If this isn't the first buffer discarded by the append window, try to use
-  // the last buffer discarded for preroll.  This ensures that the partially
-  // trimmed buffer can be correctly decoded.
+  // If we have a preroll buffer see if we can attach it to the first buffer
+  // overlapping or after |append_window_start|.
   if (audio_preroll_buffer_) {
-    // We only want to use the preroll buffer if it directly precedes (less than
-    // one sample apart) the current buffer.
+    // We only want to use the preroll buffer if it directly precedes (less
+    // than one sample apart) the current buffer.
     const int64 delta = std::abs((audio_preroll_buffer_->timestamp() +
                                   audio_preroll_buffer_->duration() -
                                   buffer->timestamp()).InMicroseconds());
     if (delta < sample_duration_.InMicroseconds()) {
+      DVLOG(1) << "Attaching audio preroll buffer ["
+               << audio_preroll_buffer_->timestamp().InSecondsF() << ", "
+               << (audio_preroll_buffer_->timestamp() +
+                   audio_preroll_buffer_->duration()).InSecondsF() << ") to "
+               << buffer->timestamp().InSecondsF();
       buffer->SetPrerollBuffer(audio_preroll_buffer_);

[wolenetz, 2014/07/30 22:04:32]

We need to return true later in this case (per method comment in .h). Suggest
s/trimmed_buffer/buffer_trimmed_or_preroll_added/. Suggest also adding a test
that exposes the scenario "preroll successfully added to buffer that is entirely
within append window", or did one exist already that failed for this PS?

[DaleCurtis, 2014/07/30 22:52:00]

Done. Doesn't matter though since the buffer would pass the existing append
window checks.

     } else {
       // TODO(dalecurtis): Add a MEDIA_LOG() for when this is dropped unused.
@@ -384,20 +373,46 @@ bool FrameProcessor::HandlePartialAppendWindowTrimming(
     audio_preroll_buffer_ = NULL;
   }
 
-  // Decrease the duration appropriately.  We only need to shorten the buffer if
-  // it overlaps |append_window_start|.
+  bool trimmed_buffer = false;
+
+  // See if a partial discard can be done around |append_window_start|.
   if (buffer->timestamp() < append_window_start) {
+    DVLOG(1) << "Truncating buffer which overlaps append window start."
+             << " presentation_timestamp " << buffer->timestamp().InSecondsF()
+             << " frame_end_timestamp " << frame_end_timestamp.InSecondsF()
+             << " append_window_start " << append_window_start.InSecondsF();
+
+    // Mark the overlapping portion of the buffer for discard.
     buffer->set_discard_padding(std::make_pair(
         append_window_start - buffer->timestamp(), base::TimeDelta()));
+
+    // Adjust the timestamp of this buffer forward to |append_window_start| and
+    // decrease the duration to compensate.
+    buffer->set_timestamp(append_window_start);
+    buffer->SetDecodeTimestamp(append_window_start);
     buffer->set_duration(frame_end_timestamp - append_window_start);
+    trimmed_buffer = true;
   }
 
-  // Adjust the timestamp of this buffer forward to |append_window_start|.  The
-  // timestamps are always set, even if |buffer|'s timestamp is already set to
-  // |append_window_start|, to ensure the preroll buffer is setup correctly.
-  buffer->set_timestamp(append_window_start);
-  buffer->SetDecodeTimestamp(append_window_start);
-  return true;
+  // See if a partial discard can be done around |append_window_end|.
+  if (buffer->timestamp() < append_window_end &&

[wolenetz, 2014/07/30 22:04:32]

nit: drop (or change to a DCHECK) the first clause here, once line 349 is
changed to >=.

[DaleCurtis, 2014/07/30 22:52:00]

Done.

+      frame_end_timestamp > append_window_end) {
+    DVLOG(1) << "Truncating buffer which overlaps append window end."
+             << " presentation_timestamp " << buffer->timestamp().InSecondsF()
+             << " frame_end_timestamp " << frame_end_timestamp.InSecondsF()
+             << " append_window_end " << append_window_end.InSecondsF();
+
+    // Mark the overlapping portion of the buffer for discard.
+    buffer->set_discard_padding(
+        std::make_pair(buffer->discard_padding().first,
+                       frame_end_timestamp - append_window_end));
+
+    // Decrease the duration of the buffer to remove the discarded portion.
+    buffer->set_duration(append_window_end - buffer->timestamp());
+    trimmed_buffer = true;
+  }
+
+  return trimmed_buffer;
 }
 
 bool FrameProcessor::ProcessFrame(
@@ -564,7 +579,7 @@ bool FrameProcessor::ProcessFrame(
 
     // 9. Let frame end timestamp equal the sum of presentation timestamp and
     //    frame duration.
-    const base::TimeDelta frame_end_timestamp =
+    base::TimeDelta frame_end_timestamp =
         presentation_timestamp + frame_duration;
 
     // 10.  If presentation timestamp is less than appendWindowStart, then set
@@ -582,8 +597,9 @@ bool FrameProcessor::ProcessFrame(
         HandlePartialAppendWindowTrimming(append_window_start,
                                           append_window_end,
                                           frame)) {
-      // If |frame| was shortened a discontinuity may exist, so treat the next
-      // frames appended as if they were the beginning of a new media segment.
+      // If |frame| was front-trimmed a discontinuity may exist, so treat the
+      // next frames appended as if they were the beginning of a new media
+      // segment.
       if (frame->timestamp() != presentation_timestamp && !sequence_mode_)
         *new_media_segment = true;
 
@@ -593,9 +609,7 @@ bool FrameProcessor::ProcessFrame(
       // frame duration and reduces spurious discontinuity detection.
       decode_timestamp = frame->GetDecodeTimestamp();
       presentation_timestamp = frame->timestamp();
-
-      // The end timestamp of the frame should be unchanged.
-      DCHECK(frame_end_timestamp == presentation_timestamp + frame->duration());
+      frame_end_timestamp = frame->timestamp() + frame->duration();
     }
 
     if (presentation_timestamp < append_window_start ||