MSE: Allow fully discarding frames with resulting timestamps below 0

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 "base/stl_util.h"
 #include "media/base/buffers.h"
 #include "media/base/stream_parser_buffer.h"
 
@@ skipping 218 matching lines @@
         // 7.2. - 7.5.:
         Reset();
 
         // 7.6. Jump to the Loop Top step above to restart processing of the
         //      current coded frame.
         DVLOG(3) << __FUNCTION__ << ": Discontinuity: reprocessing frame";
         continue;
       }
     }
 
-    // 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 +
         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
@@ skipping 35 matching lines @@
           // 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;
       }
     }
 
+    // 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()) {
       // 12.1. If the coded frame is not a random access point, then drop the
       //       coded frame and jump to the top of the loop to start processing
       //       the next coded frame.
       if (!frame->IsKeyframe()) {
         DVLOG(3) << __FUNCTION__
                  << ": Dropping frame that is not a random access point";
         return true;
@@ skipping 33 matching lines @@
 
     // 21. If highest presentation timestamp for track buffer is unset or frame
     //     end timestamp is greater than highest presentation timestamp, then
     //     set highest presentation timestamp for track buffer to frame end
     //     timestamp.
     track_buffer->SetHighestPresentationTimestampIfIncreased(
         frame_end_timestamp);
 
     // 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;
   }
 
   NOTREACHED();
   return false;
 }
 
 void FrameProcessor::SetAllTrackBuffersNeedRandomAccessPoint() {
   for (TrackBufferMap::iterator itr = track_buffers_.begin();
        itr != track_buffers_.end(); ++itr) {
     itr->second->set_needs_random_access_point(true);
   }
 }
 
 }  // namespace media