Fix int-overflow due to absent stream timestamp

media/filters/ffmpeg_demuxer.cc

Index: media/filters/ffmpeg_demuxer.cc
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 218989c140d58e384dd6ea540c51c76ed676a5d5..a9936b1d41c2ac98253d2750d963601a865a3981 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -475,59 +475,59 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
   const base::TimeDelta stream_timestamp =
       ConvertStreamTimestamp(stream_->time_base, packet->pts);
 
-  if (stream_timestamp != kNoTimestamp) {
-    const bool is_audio = type() == AUDIO;
-
-    // If this file has negative timestamps don't rebase any other stream types
-    // against the negative starting time.
-    base::TimeDelta start_time = demuxer_->start_time();
-    if (fixup_negative_timestamps_ && !is_audio &&
-        start_time < base::TimeDelta()) {
-      start_time = base::TimeDelta();
-    }
+  if (stream_timestamp == kNoTimestamp) {
+    demuxer_->NotifyDemuxerError(DEMUXER_ERROR_COULD_NOT_PARSE);
+    return;
+  }
 
-    // Don't rebase timestamps for positive start times, the HTML Media Spec
-    // details this in section "4.8.10.6 Offsets into the media resource." We
-    // will still need to rebase timestamps before seeking with FFmpeg though.
-    if (start_time > base::TimeDelta())
-      start_time = base::TimeDelta();
-
-    buffer->set_timestamp(stream_timestamp - start_time);
-
-    // If enabled, and no codec delay is present, mark audio packets with
-    // negative timestamps for post-decode discard.
-    if (fixup_negative_timestamps_ && is_audio &&
-        stream_timestamp < base::TimeDelta() &&
-        buffer->duration() != kNoTimestamp) {
-      if (!audio_decoder_config().codec_delay()) {
-        DCHECK_EQ(buffer->discard_padding().first, base::TimeDelta());
-
-        if (stream_timestamp + buffer->duration() < base::TimeDelta()) {
-          DCHECK_EQ(buffer->discard_padding().second, base::TimeDelta());
-
-          // Discard the entire packet if it's entirely before zero.
-          buffer->set_discard_padding(
-              std::make_pair(kInfiniteDuration, base::TimeDelta()));
-        } else {
-          // Only discard part of the frame if it overlaps zero.
-          buffer->set_discard_padding(std::make_pair(
-              -stream_timestamp, buffer->discard_padding().second));
-        }
+  const bool is_audio = type() == AUDIO;
+
+  // If this file has negative timestamps don't rebase any other stream types
+  // against the negative starting time.
+  base::TimeDelta start_time = demuxer_->start_time();
+  if (fixup_negative_timestamps_ && !is_audio &&
+      start_time < base::TimeDelta()) {
+    start_time = base::TimeDelta();
+  }
+
+  // Don't rebase timestamps for positive start times, the HTML Media Spec
+  // details this in section "4.8.10.6 Offsets into the media resource." We
+  // will still need to rebase timestamps before seeking with FFmpeg though.
+  if (start_time > base::TimeDelta())
+    start_time = base::TimeDelta();
+
+  buffer->set_timestamp(stream_timestamp - start_time);
+
+  // If enabled, and no codec delay is present, mark audio packets with
+  // negative timestamps for post-decode discard.
+  if (fixup_negative_timestamps_ && is_audio &&
+      stream_timestamp < base::TimeDelta() &&
+      buffer->duration() != kNoTimestamp) {
+    if (!audio_decoder_config().codec_delay()) {
+      DCHECK_EQ(buffer->discard_padding().first, base::TimeDelta());
+
+      if (stream_timestamp + buffer->duration() < base::TimeDelta()) {
+        DCHECK_EQ(buffer->discard_padding().second, base::TimeDelta());
+
+        // Discard the entire packet if it's entirely before zero.
+        buffer->set_discard_padding(
+            std::make_pair(kInfiniteDuration, base::TimeDelta()));
       } else {
-        // Verify that codec delay would cover discard and that we don't need to
-        // mark the packet for post decode discard.  Since timestamps may be in
-        // milliseconds and codec delay in nanosecond precision, round up to the
-        // nearest millisecond.  See enable_negative_timestamp_fixups().
-        DCHECK_LE(-std::ceil(FramesToTimeDelta(
-                                 audio_decoder_config().codec_delay(),
-                                 audio_decoder_config().samples_per_second())
-                                 .InMillisecondsF()),
-                  stream_timestamp.InMillisecondsF());
+        // Only discard part of the frame if it overlaps zero.
+        buffer->set_discard_padding(std::make_pair(
+            -stream_timestamp, buffer->discard_padding().second));
       }
+    } else {
+      // Verify that codec delay would cover discard and that we don't need to
+      // mark the packet for post decode discard.  Since timestamps may be in
+      // milliseconds and codec delay in nanosecond precision, round up to the
+      // nearest millisecond.  See enable_negative_timestamp_fixups().
+      DCHECK_LE(-std::ceil(FramesToTimeDelta(
+                               audio_decoder_config().codec_delay(),
+                               audio_decoder_config().samples_per_second())
+                               .InMillisecondsF()),
+                stream_timestamp.InMillisecondsF());
     }
-  } else {
-    // If this happens on the first packet, decoders will throw an error.
-    buffer->set_timestamp(kNoTimestamp);
   }
 
   if (last_packet_timestamp_ != kNoTimestamp) {
@@ -545,20 +545,13 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
     // timestamp.  The decoder will rewrite the timestamps to be sample accurate

[DaleCurtis, 2017/01/13 22:05:00]

Comment seems busted?

[tguilbert, 2017/01/13 23:52:02]

Done. (Assuming that was the portion of the comment that you were pointing out
as busted)

     // later.  See http://crbug.com/396864.
     if (fixup_negative_timestamps_ &&

[DaleCurtis, 2017/01/13 22:05:00]

Do any of the tests hit this block of code? It's the only place I could see that
we'll error out eearlier on now.

[tguilbert, 2017/01/13 23:52:02]

Confirming that BasicPlaybackChainedOgg and BasicPlaybackChainedOggVideo hit
this code block (Thanks for adding them 3 years ago :) )

-        (buffer->timestamp() == kNoTimestamp ||
-         buffer->timestamp() < last_packet_timestamp_)) {
+        buffer->timestamp() < last_packet_timestamp_) {
       buffer->set_timestamp(last_packet_timestamp_ +
                             (last_packet_duration_ != kNoTimestamp
                                  ? last_packet_duration_
                                  : base::TimeDelta::FromMicroseconds(1)));
     }
 
-    if (buffer->timestamp() == kNoTimestamp) {
-      // If we didn't get a valid timestamp and didn't fix it up, then fail.
-      demuxer_->NotifyDemuxerError(DEMUXER_ERROR_COULD_NOT_PARSE);
-      return;
-    }
-
     // The demuxer should always output positive timestamps.
     DCHECK(buffer->timestamp() >= base::TimeDelta());