Make the timestamp unroll offset consistent accross PES pids.

media/formats/mp2t/mp2t_stream_parser.cc

Index: media/formats/mp2t/mp2t_stream_parser.cc
diff --git a/media/formats/mp2t/mp2t_stream_parser.cc b/media/formats/mp2t/mp2t_stream_parser.cc
index 35c61d6bde0bf20d6e4caf59a4d47e769ca79a99..53828b513ffda522ec7c837d81f9df4e28de0100 100644
--- a/media/formats/mp2t/mp2t_stream_parser.cc
+++ b/media/formats/mp2t/mp2t_stream_parser.cc
@@ -217,6 +217,10 @@ void Mp2tStreamParser::Flush() {
   // Reset the selected PIDs.
   selected_audio_pid_ = -1;
   selected_video_pid_ = -1;
+
+  // Reset the timestamp unroller.
+  timestamp_unroller_.Reset();
+  time_offset_ = base::TimeDelta();
 }
 
 bool Mp2tStreamParser::Parse(const uint8* buf, int size) {
@@ -356,7 +360,7 @@ void Mp2tStreamParser::RegisterPes(int pmt_pid,
   // Create the PES state here.
   DVLOG(1) << "Create a new PES state";
   scoped_ptr<TsSection> pes_section_parser(
-      new TsSectionPes(es_parser.Pass()));
+      new TsSectionPes(es_parser.Pass(), &timestamp_unroller_));
   PidState::PidType pid_type =
       is_audio ? PidState::kPidAudioPes : PidState::kPidVideoPes;
   scoped_ptr<PidState> pes_pid_state(
@@ -519,10 +523,6 @@ void Mp2tStreamParser::OnEmitAudioBuffer(
       << stream_parser_buffer->timestamp().InMilliseconds()
       << " dur="
       << stream_parser_buffer->duration().InMilliseconds();
-  stream_parser_buffer->set_timestamp(
-      stream_parser_buffer->timestamp() - time_offset_);
-  stream_parser_buffer->SetDecodeTimestamp(
-      stream_parser_buffer->GetDecodeTimestamp() - time_offset_);
 
   // Ignore the incoming buffer if it is not associated with any config.
   if (buffer_queue_chain_.empty()) {
@@ -550,10 +550,6 @@ void Mp2tStreamParser::OnEmitVideoBuffer(
       << stream_parser_buffer->duration().InMilliseconds()
       << " IsKeyframe="
       << stream_parser_buffer->IsKeyframe();
-  stream_parser_buffer->set_timestamp(
-      stream_parser_buffer->timestamp() - time_offset_);
-  stream_parser_buffer->SetDecodeTimestamp(
-      stream_parser_buffer->GetDecodeTimestamp() - time_offset_);
 
   // Ignore the incoming buffer if it is not associated with any config.
   if (buffer_queue_chain_.empty()) {
@@ -588,15 +584,27 @@ bool Mp2tStreamParser::EmitRemainingBuffers() {
 
   // Buffer emission.
   while (!buffer_queue_chain_.empty()) {
+    BufferQueueWithConfig& queue_with_config = buffer_queue_chain_.front();
+
     // Start a segment if needed.
     if (!segment_started_) {
+      // We need at least some audio and video buffers to derive the global
+      // timestamp offset.
+      if (selected_audio_pid_ >= 0 && queue_with_config.audio_queue.empty())
+        return true;
+      if (selected_video_pid_ >= 0 && queue_with_config.video_queue.empty())
+        return true;
+      // Compensate if some timestamps are negative.

[wolenetz, 2014/09/05 21:18:49]

I'm a bit confused why this compensation is required. MSE coded frame processing
algorithm and byte stream specification for mpt2s do not currently make
allowance for this, IIUC. Rather, if PTS >= 0, and DTS < 0 parse (decode) error
should result; if PTS < (0 or whatever appendWindowStart is, which is >= 0), and
DTS < 0, then either the frame is dropped without error or is partially trimmed
or used as subsequent decoder preroll (and DTS + PTS are adjusted to be >=
appendWindowStart), without error.
Do these conditions break common mp2ts, and if so, do the MSE specs need
adjustment accordingly to match this compensation logic? Or is the following
verbiage in the MSE mp2ts byte stream format spec what this compensation is
implementing for realz: "When a discontinuity is detected,
MPEG2TS_timestampOffset must be adjusted to make the timestamps after the
discontinuity appear to come immediately after the timestamps before the
discontinuity"? (And if the latter, setTimestampOffset() and abort() need to
reset time_offset_ to base::TimeDelta, IIUC.)
Either way, I would prefer you split this compensation piece out to a separate
CL.

[damienv1, 2014/09/05 21:52:03]

Timestamps in MPEG-2 TS are defined modulo 2^33. So in itself, these timestamps
have no relationship at all to the media timeline that is used in Chrome.
And there is no notion of negative timestamps in MPEG-2 TS.

You need to provide a global offset that will be applied in the same way to
every single timestamp.
Here, I apply a global offset so that MSE get positive DTS.

Note: the MPEG2TS_timestampsOffset you are referring to is inside the
TimestampUnroller.

On 2014/09/05 21:18:49, wolenetz wrote:

+      if (timestamp_unroller_.GetMinUnrolledTimestamp() < 0)
+        time_offset_ = base::TimeDelta::FromMicroseconds(
+            (1000 * (INT64_C(1) << 33)) / 90);
+
       DVLOG(1) << "Starting a new segment";
       segment_started_ = true;
       new_segment_cb_.Run();
     }
 
     // Update the audio and video config if needed.
-    BufferQueueWithConfig& queue_with_config = buffer_queue_chain_.front();
     if (!queue_with_config.is_config_sent) {
       if (!config_cb_.Run(queue_with_config.audio_config,
                           queue_with_config.video_config,
@@ -609,6 +617,8 @@ bool Mp2tStreamParser::EmitRemainingBuffers() {
     TextBufferQueueMap empty_text_map;
     if (!queue_with_config.audio_queue.empty() ||
         !queue_with_config.video_queue.empty()) {
+      ApplyTimeOffset(&queue_with_config.audio_queue);
+      ApplyTimeOffset(&queue_with_config.video_queue);
       if (!new_buffers_cb_.Run(queue_with_config.audio_queue,
                                queue_with_config.video_queue,
                                empty_text_map)) {
@@ -628,5 +638,14 @@ bool Mp2tStreamParser::EmitRemainingBuffers() {
   return true;
 }
 
+void Mp2tStreamParser::ApplyTimeOffset(
+    StreamParser::BufferQueue* buffer_queue) {
+  for (StreamParser::BufferQueue::iterator it = buffer_queue->begin();
+       it != buffer_queue->end(); ++it) {
+    (*it)->SetDecodeTimestamp((*it)->GetDecodeTimestamp() + time_offset_);
+    (*it)->set_timestamp((*it)->timestamp() + time_offset_);
+  }
+}
+
 }  // namespace mp2t
 }  // namespace media