Add gapless playback support for AAC playback.

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 7f83a226b75cb9ad3165a81c115c72d1371e3510..f1e61421063c6d1f53c7ca5aa12b8f0e2b6747f2 100644
--- a/media/filters/frame_processor.cc
+++ b/media/filters/frame_processor.cc
@@ -75,11 +75,12 @@ bool FrameProcessor::ProcessFrames(
   return true;
 }
 
-bool FrameProcessor::ProcessFrame(scoped_refptr<StreamParserBuffer> frame,
-                                  base::TimeDelta append_window_start,
-                                  base::TimeDelta append_window_end,
-                                  base::TimeDelta* timestamp_offset,
-                                  bool* new_media_segment) {
+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:
   // https://dvcs.w3.org/hg/html-media/raw-file/d471a4412040/media-source/
@@ -251,8 +252,8 @@ bool FrameProcessor::ProcessFrame(scoped_refptr<StreamParserBuffer> frame,
 
     // 9. Let frame end timestamp equal the sum of presentation timestamp and
     //    frame duration.
-    base::TimeDelta frame_end_timestamp = presentation_timestamp +
-        frame_duration;
+    const 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
@@ -263,45 +264,34 @@ bool FrameProcessor::ProcessFrame(scoped_refptr<StreamParserBuffer> frame,
     // 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)) {
+      // |frame| has been partially trimmed or had preroll added.

[wolenetz, 2014/05/30 19:58:45]

I agree that the false path would set the flags for new media segment/random
access point needed, but in the case where no |audio_preroll_buffer_| was
previously held aside, and |frame| overlaps |append_window_start| and therefore
has been partially trimmed, we should still signal SBS of a new media segment
(if not in sequence mode). We shouldn't need to worry about setting
needs_random_access_point in this case since HandlePartialAppendWindowTrimming
DCHECKS the partially trimmed frame is a key frame. Net: Please add the
following here, perhaps with a clarifying comment.
if (!sequence_mode_)
  *new_media_segment = true;

[wolenetz, 2014/05/30 20:45:12]

From our chat, it's also possible that append window partial discard even in
sequence mode could introduce a gap such that SBS should be told a new media
segment is starting.
Please add: (here and also in LegacyFrameProcessor)
if (frame->timestamp() != presentation_timestamp) {
  *new_media_segment = true;
  track_buffer->set_needs_random_access_point(true);
}

[DaleCurtis, 2014/05/30 20:54:30]

Done.

+      decode_timestamp = frame->GetDecodeTimestamp();
+      presentation_timestamp = frame->timestamp();
+      frame_duration = frame->duration();
+
+      // The end timestamp of the frame should be unchanged.
+      DCHECK(frame_end_timestamp == presentation_timestamp + frame_duration);
+    }
+
     if (presentation_timestamp < append_window_start ||
         frame_end_timestamp > append_window_end) {
-      // See if a partial discard can be done around |append_window_start|.
-      // TODO(wolenetz): Refactor this into a base helper across legacy and
-      // new frame processors?
-      if (track_buffer->stream()->supports_partial_append_window_trimming() &&
-          presentation_timestamp < append_window_start &&
-          frame_end_timestamp > append_window_start &&
-          frame_end_timestamp <= append_window_end) {
-        DCHECK(frame->IsKeyframe());
-        DVLOG(1) << "Truncating buffer which overlaps append window start."
-                 << " presentation_timestamp "
-                 << presentation_timestamp.InSecondsF()
-                 << " append_window_start " << append_window_start.InSecondsF();
-
-        // Adjust the timestamp of this frame forward to |append_window_start|,
-        // while decreasing the duration appropriately.
-        frame->set_discard_padding(std::make_pair(
-            append_window_start - presentation_timestamp, base::TimeDelta()));
-        presentation_timestamp = append_window_start;  // |frame| updated below.
-        decode_timestamp = append_window_start;  // |frame| updated below.
-        frame_duration = frame_end_timestamp - presentation_timestamp;
-        frame->set_duration(frame_duration);
-
-        // TODO(dalecurtis): This could also be done with |append_window_end|,
-        // but is not necessary since splice frames covert the overlap there.
-      } else {
-        track_buffer->set_needs_random_access_point(true);
-        DVLOG(3) << "Dropping frame that is outside append window.";
-
-        if (!sequence_mode_) {
-          // This also triggers a discontinuity so we need to treat the next
-          // frames appended within the append window as if they were the
-          // beginning of a new segment.
-          *new_media_segment = true;
-        }
-
-        return true;
+      track_buffer->set_needs_random_access_point(true);
+      DVLOG(3) << "Dropping frame that is outside append window.";
+
+      if (!sequence_mode_) {

[wolenetz, 2014/05/30 20:45:12]

From our chat, this condition needs to be dropped. Even in sequence mode, a gap
could be introduced.

If MSE spec editors truly desire sequence mode append window filtering to filter
timestampOffset-adjusted-appended-buffers such that any buffers surviving the
filter get further relocated (and timestampOffset further adjusted) such that
they start without gap after previous group_end_timestamp, then I believe the
spec would need updating.

acolwell@, please comment on this.

[DaleCurtis, 2014/05/30 20:54:30]

I defer to you guys to sort this out outside of this patch.

[wolenetz, 2014/05/30 22:31:42]

SGTM. On further inspection, and per our offline chat, leaving this as-is
conforms to the intent of sequence mode. I retract my earlier comment here and
am working on a separate CL that clarifies new frame processor behavior w.r.t.
small append window discards.

[wolenetz, 2014/05/31 01:22:04]

I have a separate CL to test/clarify small append window discards:
https://codereview.chromium.org/310483005/

+        // This also triggers a discontinuity so we need to treat the next
+        // frames appended within the append window as if they were the
+        // beginning of a new segment.
+        *new_media_segment = true;
       }
+
+      return true;
     }
 
     // 12. If the need random access point flag on track buffer equals true,
@@ -331,8 +321,6 @@ bool FrameProcessor::ProcessFrame(scoped_refptr<StreamParserBuffer> frame,
     DVLOG(3) << __FUNCTION__ << ": Sending processed frame to stream, "
              << "PTS=" << presentation_timestamp.InSecondsF()
              << ", DTS=" << decode_timestamp.InSecondsF();
-    frame->set_timestamp(presentation_timestamp);
-    frame->SetDecodeTimestamp(decode_timestamp);
 
     // Steps 13-18:
     // TODO(wolenetz): Collect and emit more than one buffer at a time, if