Mpeg2 TS - Fail when no valid timestamp in the ADTS parser.

media/formats/mp2t/es_parser_adts.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/formats/mp2t/es_parser_adts.h"
 
 #include <list>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ skipping 127 matching lines @@
 
   // Look for every ADTS frame in the ES buffer.
   AdtsFrame adts_frame;
   while (LookForAdtsFrame(&adts_frame)) {
     // Update the audio configuration if needed.
     DCHECK_GE(adts_frame.size, kADTSHeaderMinSize);
     if (!UpdateAudioConfiguration(adts_frame.data))
       return false;
 
     // Get the PTS & the duration of this access unit.
     while (!pts_list_.empty() &&

[wolenetz, 2014/07/15 19:57:42]

I'm curious since I haven't looked through this code in detail before now: Is it
possible for |pts_list_| to contain more than one PTS, each with a queue offset
<= the adts_frame.queue_offset here? Why? (And if now, why loop with a
while(){}?)

[damienv1, 2014/07/15 21:26:59]

How many PTS are in this list all depend on the stream: e.g. if you have
multiple frames per PES, if an ADTS frame spans over multiple PES (although this
is not advised to do so). 
The loop aims at getting the PTS associated to the byte position that comes just
before the audio frame byte position (that's how the PTS is defined in Mpeg2
TS).

[wolenetz, 2014/07/15 21:59:56]

Acknowledged.

            pts_list_.front().first <= adts_frame.queue_offset) {
       audio_timestamp_helper_->SetBaseTimestamp(pts_list_.front().second);
       pts_list_.pop_front();
     }
 
+    if (audio_timestamp_helper_->base_timestamp() == kNoTimestamp()) {
+      DVLOG(1) << "Audio frame with unknown timestamp";
+      return false;
+    }
     base::TimeDelta current_pts = audio_timestamp_helper_->GetTimestamp();
     base::TimeDelta frame_duration =
         audio_timestamp_helper_->GetFrameDuration(kSamplesPerAACFrame);
 
     // Emit an audio frame.
     bool is_key_frame = true;
 
     // TODO(wolenetz/acolwell): Validate and use a common cross-parser TrackId
     // type and allow multiple audio tracks. See https://crbug.com/341581.
     scoped_refptr<StreamParserBuffer> stream_parser_buffer =
@@ skipping 77 matching lines @@
       arraysize(extra_data),
       false);
 
   if (!audio_decoder_config.Matches(last_audio_decoder_config_)) {
     DVLOG(1) << "Sampling frequency: " << samples_per_second;
     DVLOG(1) << "Extended sampling frequency: " << extended_samples_per_second;
     DVLOG(1) << "Channel config: " << channel_configuration;
     DVLOG(1) << "Adts profile: " << adts_profile;
     // Reset the timestamp helper to use a new time scale.
     if (audio_timestamp_helper_) {
       base::TimeDelta base_timestamp = audio_timestamp_helper_->GetTimestamp();

[wolenetz, 2014/07/15 19:57:42]

What if base timestamp was not previously set? Is this possible, and if so, it
looks to me like a DCHECK would be hit at
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/audio_t...

[damienv1, 2014/07/15 21:26:59]

Good catch. Thanks.

       audio_timestamp_helper_.reset(
         new AudioTimestampHelper(samples_per_second));
       audio_timestamp_helper_->SetBaseTimestamp(base_timestamp);
     } else {
       audio_timestamp_helper_.reset(
           new AudioTimestampHelper(samples_per_second));
     }
     // Audio config notification.
     last_audio_decoder_config_ = audio_decoder_config;
     new_audio_config_cb_.Run(audio_decoder_config);
   }
 
   return true;
 }
 
 }  // namespace mp2t
 }  // namespace media