Fix int-overflow due to absent stream timestamp

media/filters/ffmpeg_demuxer.cc

 // Copyright (c) 2012 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/ffmpeg_demuxer.h"
 
 #include <algorithm>
 #include <memory>
 #include <set>
 #include <utility>
@@ skipping 457 matching lines @@
     // https://crbug.com/394418
     DVLOG(1) << "FFmpeg returned a buffer with a negative duration! "
              << packet->duration;
     buffer->set_duration(kNoTimestamp);
   }
 
   // Note: If pts is AV_NOPTS_VALUE, stream_timestamp will be kNoTimestamp.
   const base::TimeDelta stream_timestamp =
       ConvertStreamTimestamp(stream_->time_base, packet->pts);
 
-  if (stream_timestamp != kNoTimestamp) {
-    const bool is_audio = type() == AUDIO;
+  if (stream_timestamp == kNoTimestamp) {
+    demuxer_->NotifyDemuxerError(DEMUXER_ERROR_COULD_NOT_PARSE);
+    return;
+  }
 
-    // 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();
+  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 {
+        // 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());
     }
-
-    // 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));
-        }
-      } 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) {
     // FFmpeg doesn't support chained ogg correctly.  Instead of guaranteeing
     // continuity across links in the chain it uses the timestamp information
     // from each link directly.  Doing so can lead to timestamps which appear to
     // go backwards in time.
     //
     // If the new link starts with a negative timestamp or a timestamp less than
     // the original (positive) |start_time|, we will get a negative timestamp
     // here.  It's also possible FFmpeg returns kNoTimestamp here if it's not
     // able to work out a timestamp using the previous link and the next.
     //
     // Fixing chained ogg is non-trivial, so for now just reuse the last good
     // 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());
 
     if (last_packet_timestamp_ < buffer->timestamp()) {
       buffered_ranges_.Add(last_packet_timestamp_, buffer->timestamp());
       demuxer_->NotifyBufferingChanged();
     }
   }
 
   if (packet.get()->flags & AV_PKT_FLAG_KEY)
@@ skipping 1244 matching lines @@
 
 void FFmpegDemuxer::SetLiveness(DemuxerStream::Liveness liveness) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   for (const auto& stream : streams_) {
     if (stream)
       stream->SetLiveness(liveness);
   }
 }
 
 }  // namespace media