Mpeg2TS - estimate duration for video frames.

media/formats/mp2t/es_parser_h264.cc

Index: media/formats/mp2t/es_parser_h264.cc
diff --git a/media/formats/mp2t/es_parser_h264.cc b/media/formats/mp2t/es_parser_h264.cc
index 691678ce81ebd49fb9b3a099c306de3412f11e99..a617b88fcb5ad259b416114960443e0753ef1c75 100644
--- a/media/formats/mp2t/es_parser_h264.cc
+++ b/media/formats/mp2t/es_parser_h264.cc
@@ -23,6 +23,12 @@ namespace mp2t {
 // 3 bytes for the start code + 1 byte for the NALU type.
 const int kMinAUDSize = 4;
 
+// Default video frame duration.
+const base::TimeDelta kDefaultFrameDuration(

[acolwell GONE FROM CHROMIUM, 2014/07/03 00:50:58]

nit: Please add a comment saying why you picked 25fps.

+    base::TimeDelta::FromMilliseconds(40));
+
+const size_t kMaxPtsHistoryLength = 16;

[acolwell GONE FROM CHROMIUM, 2014/07/03 00:50:58]

nit: I'm assuming this is derived based on the max number of B-frames that can
appear after non-B-frames right? Please add a comment indicating where this
number was derived from. 

+
 EsParserH264::EsParserH264(
     const NewVideoConfigCB& new_video_config_cb,
     const EmitBufferCB& emit_buffer_cb)
@@ -85,6 +91,7 @@ void EsParserH264::Reset() {
   next_access_unit_pos_ = 0;
   timing_desc_list_.clear();
   last_video_decoder_config_ = VideoDecoderConfig();
+  frame_pts_history_.clear();
 }
 
 bool EsParserH264::FindAUD(int64* stream_pos) {
@@ -237,6 +244,32 @@ bool EsParserH264::EmitFrame(int64 access_unit_pos, int access_unit_size,
   if (current_timing_desc.pts == kNoTimestamp())
     return false;
 
+  // In Mpeg2 TS, there is no notion of frame duration.
+  // Only try to get an estimate of the frame duration.
+  // TODO(damienv): when the H264 stream has SEI timing information,
+  // we could get more accurate information about the frame duration.
+  if (!frame_pts_history_.empty() &&
+      frame_pts_history_.back() > current_timing_desc.pts) {
+    // To estimate the frame duration, B frames are assigned the same PTS
+    // as the last non-B frame.
+    frame_pts_history_.push_back(frame_pts_history_.back());
+  } else {
+    frame_pts_history_.push_back(current_timing_desc.pts);
+  }
+  if (frame_pts_history_.size() > kMaxPtsHistoryLength)
+    frame_pts_history_.pop_front();
+
+  base::TimeDelta frame_duration = kDefaultFrameDuration;
+  int pts_count = frame_pts_history_.size();
+  if (pts_count > 2) {
+    base::TimeDelta avg_frame_duration =
+        (frame_pts_history_.back() - frame_pts_history_.front()) /
+        (pts_count - 1);
+    frame_duration =
+        ((kMaxPtsHistoryLength - pts_count) * kDefaultFrameDuration +

[acolwell GONE FROM CHROMIUM, 2014/07/03 00:50:58]

I'm assuming that you are trying to smooth out the value for the first couple
frames here right? Is that because you don't know how many B-frames there might
be and you don't know if hierarchical B-frames are being used? Perhaps a
comment?

It seems like you shouldn't have to estimate if you delay emitting frames until
you get to a non-B. I worry that incorrect estimates may cause problems
downstream if the pts + duration overlaps another frame.

[damienv1, 2014/07/07 17:26:34]

- In theory, to get the frame duration accurately, I would need to replicate
exactly the H264 frame bumping process here (which would involve a latency for
buffer emission, and that latency would be "optimal").
However, I don't think going that deep inside the H264 parsing is the right way
either (at least, it is not inline with the spirit of a demuxer which should not
have to parse ES streams in a first place).

- Delaying the frame buffer emission with a constant latency (i.e. not going
through the H264 frame bumping process) could be another way (also not as
generic). 

Incorrect frame duration estimates could impact the upper layer only if the
upper layer understands the decoding/rendering process (which shouldn't be the
intent of MSE).

I was not really in favor of introducing a latency in that part of the code
(when you have a muxed audio/video stream, the impact is the same as the video
was coming later into the stream: i.e. could make the stream become badly muxed
stream from a buffer emission perspective).

Let me think about it though.

+         pts_count * avg_frame_duration) / kMaxPtsHistoryLength;
+  }
+
   // Update the video decoder configuration if needed.
   const H264PPS* pps = h264_parser_->GetPPS(pps_id);
   if (!pps) {
@@ -273,6 +306,7 @@ bool EsParserH264::EmitFrame(int64 access_unit_pos, int access_unit_size,
           0);
   stream_parser_buffer->SetDecodeTimestamp(current_timing_desc.dts);
   stream_parser_buffer->set_timestamp(current_timing_desc.pts);
+  stream_parser_buffer->set_duration(frame_duration);
   emit_buffer_cb_.Run(stream_parser_buffer);
   return true;
 }