Add support for partial append window end trimming.

media/filters/frame_processor.cc

 // Copyright 2014 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/frame_processor.h"
 
 #include <cstdlib>
 
 #include "base/stl_util.h"
 #include "media/base/buffers.h"
@@ skipping 314 matching lines @@
 
   return result;
 }
 
 bool FrameProcessor::HandlePartialAppendWindowTrimming(
     base::TimeDelta append_window_start,
     base::TimeDelta append_window_end,
     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.
+  // Ignore any buffers entirely after or inside of the append window.
+  if (buffer->timestamp() > append_window_end ||
+      (buffer->timestamp() > append_window_start &&
+       frame_end_timestamp < append_window_end)) {
     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 &&
       frame_end_timestamp <= append_window_start) {
     audio_preroll_buffer_ = buffer;
     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_)
     return false;
 
+  bool trimmed_buffer = 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();
+  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();
 
-  // 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 (audio_preroll_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()) {
-      buffer->SetPrerollBuffer(audio_preroll_buffer_);
-    } else {
-      // TODO(dalecurtis): Add a MEDIA_LOG() for when this is dropped unused.
+    // 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 (audio_preroll_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()) {
+        buffer->SetPrerollBuffer(audio_preroll_buffer_);
+      } else {
+        // TODO(dalecurtis): Add a MEDIA_LOG() for when this is dropped unused.
+      }
+      audio_preroll_buffer_ = NULL;
     }
-    audio_preroll_buffer_ = NULL;
+
+    // Decrease the duration appropriately.  We only need to shorten the buffer
+    // if it overlaps |append_window_start|.
+    if (buffer->timestamp() < append_window_start) {
+      buffer->set_discard_padding(std::make_pair(
+          append_window_start - buffer->timestamp(), base::TimeDelta()));
+      buffer->set_duration(frame_end_timestamp - append_window_start);
+    }
+
+    // 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);
+    trimmed_buffer = true;
   }
 
-  // Decrease the duration appropriately.  We only need to shorten the buffer if
-  // it overlaps |append_window_start|.
-  if (buffer->timestamp() < append_window_start) {
-    buffer->set_discard_padding(std::make_pair(
-        append_window_start - buffer->timestamp(), base::TimeDelta()));
-    buffer->set_duration(frame_end_timestamp - append_window_start);
+  // See if a partial discard can be done around |append_window_end|.
+  if (buffer->timestamp() <= append_window_end &&

[acolwell GONE FROM CHROMIUM, 2014/07/23 00:15:42]

s/<=/</ ? How do you get into the == situation? It seems like those buffers
shouldn't even get here. What am I forgetting here?

[DaleCurtis, 2014/07/23 01:50:41]

You're right, <= should have been skipped in the if on l.340.  Fixed.

+      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();
+    buffer->set_discard_padding(
+        std::make_pair(buffer->discard_padding().first,
+                       frame_end_timestamp - append_window_end));
+    buffer->set_duration(append_window_end - buffer->timestamp());
+    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;
+  return trimmed_buffer;
 }
 
 bool FrameProcessor::ProcessFrame(
     const scoped_refptr<StreamParserBuffer>& frame,
     base::TimeDelta append_window_start,
     base::TimeDelta append_window_end,
     base::TimeDelta* timestamp_offset,
     bool* new_media_segment) {
   // Implements the loop within step 1 of the coded frame processing algorithm
   // for a single input frame per April 1, 2014 MSE spec editor's draft:
@@ skipping 146 matching lines @@
 
         // 7.6. Jump to the Loop Top step above to restart processing of the
         //      current coded frame.
         DVLOG(3) << __FUNCTION__ << ": Discontinuity: reprocessing frame";
         continue;
       }
     }
 
     // 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
     //      the need random access point flag to true, drop the coded frame, and
     //      jump to the top of the loop to start processing the next coded
     //      frame.
     // Note: We keep the result of partial discard of a buffer that overlaps
     //      |append_window_start| and does not end after |append_window_end|.
     // 11. If frame end timestamp is greater than appendWindowEnd, then set the
     //     need random access point flag to true, drop the coded frame, and jump
     //     to the top of the loop to start processing the next coded frame.
     frame->set_timestamp(presentation_timestamp);
     frame->SetDecodeTimestamp(decode_timestamp);
     if (track_buffer->stream()->supports_partial_append_window_trimming() &&
         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->timestamp() != presentation_timestamp && !sequence_mode_)
         *new_media_segment = true;
 
       // |frame| has been partially trimmed or had preroll added.  Though
       // |frame|'s duration may have changed, do not update |frame_duration|
       // here, so |track_buffer|'s last frame duration update uses original

[acolwell GONE FROM CHROMIUM, 2014/07/23 00:15:42]

Is this still true since you are setting a new frame_end_timestamp now?

[DaleCurtis, 2014/07/23 01:50:41]

I'm not sure, wolenetz?

[wolenetz, 2014/07/25 22:33:03]

I believe this is still correct. |frame_duration| should not be changed here; it
is used, below, to set track_buffer's last frame duration, and in later
iterations of this method, that "last frame duration" is used to see if there is
a large enough gap since "last decode timestamp" to trigger discontinuity logic.
Append window trimming correctly reduces the trimmed buffer's duration, but does
not reduce the eventual track buffer's "last frame duration". This seems to me
to be a quality of implementation issue, where spurious discontinuity detections
were problematic, with "fix" being to use the original |frame_duration| if the
frame survives and is appended to the track buffer.

       // 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 ||
         frame_end_timestamp > append_window_end) {
       track_buffer->set_needs_random_access_point(true);
       DVLOG(3) << "Dropping frame that is outside append window.";
       return true;
     }
 
     // Note: This step is relocated, versus April 1 spec, to allow append window
@@ skipping 74 matching lines @@
     DCHECK(group_end_timestamp_ >= base::TimeDelta());
 
     return true;
   }
 
   NOTREACHED();
   return false;
 }
 
 }  // namespace media