Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(983)

Unified Diff: media/filters/ffmpeg_demuxer.cc

Issue 2635573002: Fix int-overflow due to absent stream timestamp (Closed)
Patch Set: Addressed comments Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « media/ffmpeg/ffmpeg_regression_tests.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/filters/ffmpeg_demuxer.cc
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 218989c140d58e384dd6ea540c51c76ed676a5d5..7d1fcea9835ee2ab4eb576fc146486f02fe7afe8 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) {
@@ -538,27 +538,19 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
//
// 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.
+ // here.
//
// 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
// later. See http://crbug.com/396864.
if (fixup_negative_timestamps_ &&
- (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());
« no previous file with comments | « media/ffmpeg/ffmpeg_regression_tests.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698