MSE: Allow fully discarding frames with resulting timestamps below 0

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..706830cc5c434f969ac1eeb49b891d7d335cb11c 100644
--- a/media/filters/frame_processor.cc
+++ b/media/filters/frame_processor.cc
@@ -236,19 +236,6 @@ bool FrameProcessor::ProcessFrame(scoped_refptr<StreamParserBuffer> frame,
       }
     }
 
-    // 8. If the presentation timestamp or decode timestamp is less than the
-    // presentation start time, then run the end of stream algorithm with the
-    // error parameter set to "decode", and abort these steps.
-    if (presentation_timestamp < base::TimeDelta() ||
-        decode_timestamp < base::TimeDelta()) {
-      DVLOG(2) << __FUNCTION__
-               << ": frame PTS=" << presentation_timestamp.InSecondsF()
-               << " or DTS=" << decode_timestamp.InSecondsF()
-               << " negative after applying timestampOffset and handling any "
-               << " discontinuity";
-      return false;
-    }
-
     // 9. Let frame end timestamp equal the sum of presentation timestamp and
     //    frame duration.
     base::TimeDelta frame_end_timestamp = presentation_timestamp +
@@ -304,6 +291,24 @@ bool FrameProcessor::ProcessFrame(scoped_refptr<StreamParserBuffer> frame,
       }
     }
 
+    // Note: This step is relocated, versus April 1 spec, to after append window
+    // filtering to allow more than 1 full frame preroll to support gapless

[acolwell GONE FROM CHROMIUM, 2014/05/24 05:27:36]

nit: I don't think this comment needs to be AAC specific. I think it is
sufficient to just put this in terms of timestampOffset shifting a media segment
in such a way that some of the coded frames happen to be before 0.

[wolenetz, 2014/05/27 19:35:14]

Done.

+    // AAC beginning at time 0. This relocation verifies only the frames that
+    // survive append window filtering, rather than checking before append
+    // window filtering.
+    // 8. If the presentation timestamp or decode timestamp is less than the
+    // presentation start time, then run the end of stream algorithm with the
+    // error parameter set to "decode", and abort these steps.
+    if (presentation_timestamp < base::TimeDelta() ||

[acolwell GONE FROM CHROMIUM, 2014/05/24 05:27:36]

Yes. I believe the spec should probably be updated to move this step here. The
constraints on setting append_window_start essentially enforces
presentation_timestamp >= 0 at this point so you could probably just change the
presentation_timestamp condition to a DCHECK. The only question remains is
whether we should allow negative DTS through here. For now, I think the answer
should be no. 

Matt, I realize that the "sequence" mode with B-frames issue may still be
something we have to deal with, but for now I think the code should still
enforce DTS >= 0 until we have a better understanding of what the potential
issues are.

[wolenetz, 2014/05/27 19:35:14]

For PTS, added DCHECK.
For DTS < 0: I've added a note and updated the DVLOG. I did not add a TODO or a
bug; we're not sure yet about MP4 sequence mode layout test failure cause
(parser, test media, or spec).

+        decode_timestamp < base::TimeDelta()) {
+      DVLOG(2) << __FUNCTION__
+               << ": frame PTS=" << presentation_timestamp.InSecondsF()
+               << " or DTS=" << decode_timestamp.InSecondsF()
+               << " negative after applying timestampOffset and handling any "
+               << " discontinuity";
+      return false;
+    }
+
     // 12. If the need random access point flag on track buffer equals true,
     //     then run the following steps:
     if (track_buffer->needs_random_access_point()) {
@@ -357,9 +362,9 @@ bool FrameProcessor::ProcessFrame(scoped_refptr<StreamParserBuffer> frame,
 
     // 22. If frame end timestamp is greater than group end timestamp, then set
     //     group end timestamp equal to frame end timestamp.
-    DCHECK(group_end_timestamp_ >= base::TimeDelta());
     if (frame_end_timestamp > group_end_timestamp_)
       group_end_timestamp_ = frame_end_timestamp;
+    DCHECK(group_end_timestamp_ >= base::TimeDelta());
 
     return true;
   }