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

Side by Side 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 unified diff | Download patch
« no previous file with comments | « media/ffmpeg/ffmpeg_regression_tests.cc ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "media/filters/ffmpeg_demuxer.h" 5 #include "media/filters/ffmpeg_demuxer.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <memory> 8 #include <memory>
9 #include <set> 9 #include <set>
10 #include <utility> 10 #include <utility>
(...skipping 457 matching lines...) Expand 10 before | Expand all | Expand 10 after
468 // https://crbug.com/394418 468 // https://crbug.com/394418
469 DVLOG(1) << "FFmpeg returned a buffer with a negative duration! " 469 DVLOG(1) << "FFmpeg returned a buffer with a negative duration! "
470 << packet->duration; 470 << packet->duration;
471 buffer->set_duration(kNoTimestamp); 471 buffer->set_duration(kNoTimestamp);
472 } 472 }
473 473
474 // Note: If pts is AV_NOPTS_VALUE, stream_timestamp will be kNoTimestamp. 474 // Note: If pts is AV_NOPTS_VALUE, stream_timestamp will be kNoTimestamp.
475 const base::TimeDelta stream_timestamp = 475 const base::TimeDelta stream_timestamp =
476 ConvertStreamTimestamp(stream_->time_base, packet->pts); 476 ConvertStreamTimestamp(stream_->time_base, packet->pts);
477 477
478 if (stream_timestamp != kNoTimestamp) { 478 if (stream_timestamp == kNoTimestamp) {
479 const bool is_audio = type() == AUDIO; 479 demuxer_->NotifyDemuxerError(DEMUXER_ERROR_COULD_NOT_PARSE);
480 return;
481 }
480 482
481 // If this file has negative timestamps don't rebase any other stream types 483 const bool is_audio = type() == AUDIO;
482 // against the negative starting time. 484
483 base::TimeDelta start_time = demuxer_->start_time(); 485 // If this file has negative timestamps don't rebase any other stream types
484 if (fixup_negative_timestamps_ && !is_audio && 486 // against the negative starting time.
485 start_time < base::TimeDelta()) { 487 base::TimeDelta start_time = demuxer_->start_time();
486 start_time = base::TimeDelta(); 488 if (fixup_negative_timestamps_ && !is_audio &&
489 start_time < base::TimeDelta()) {
490 start_time = base::TimeDelta();
491 }
492
493 // Don't rebase timestamps for positive start times, the HTML Media Spec
494 // details this in section "4.8.10.6 Offsets into the media resource." We
495 // will still need to rebase timestamps before seeking with FFmpeg though.
496 if (start_time > base::TimeDelta())
497 start_time = base::TimeDelta();
498
499 buffer->set_timestamp(stream_timestamp - start_time);
500
501 // If enabled, and no codec delay is present, mark audio packets with
502 // negative timestamps for post-decode discard.
503 if (fixup_negative_timestamps_ && is_audio &&
504 stream_timestamp < base::TimeDelta() &&
505 buffer->duration() != kNoTimestamp) {
506 if (!audio_decoder_config().codec_delay()) {
507 DCHECK_EQ(buffer->discard_padding().first, base::TimeDelta());
508
509 if (stream_timestamp + buffer->duration() < base::TimeDelta()) {
510 DCHECK_EQ(buffer->discard_padding().second, base::TimeDelta());
511
512 // Discard the entire packet if it's entirely before zero.
513 buffer->set_discard_padding(
514 std::make_pair(kInfiniteDuration, base::TimeDelta()));
515 } else {
516 // Only discard part of the frame if it overlaps zero.
517 buffer->set_discard_padding(std::make_pair(
518 -stream_timestamp, buffer->discard_padding().second));
519 }
520 } else {
521 // Verify that codec delay would cover discard and that we don't need to
522 // mark the packet for post decode discard. Since timestamps may be in
523 // milliseconds and codec delay in nanosecond precision, round up to the
524 // nearest millisecond. See enable_negative_timestamp_fixups().
525 DCHECK_LE(-std::ceil(FramesToTimeDelta(
526 audio_decoder_config().codec_delay(),
527 audio_decoder_config().samples_per_second())
528 .InMillisecondsF()),
529 stream_timestamp.InMillisecondsF());
487 } 530 }
488
489 // Don't rebase timestamps for positive start times, the HTML Media Spec
490 // details this in section "4.8.10.6 Offsets into the media resource." We
491 // will still need to rebase timestamps before seeking with FFmpeg though.
492 if (start_time > base::TimeDelta())
493 start_time = base::TimeDelta();
494
495 buffer->set_timestamp(stream_timestamp - start_time);
496
497 // If enabled, and no codec delay is present, mark audio packets with
498 // negative timestamps for post-decode discard.
499 if (fixup_negative_timestamps_ && is_audio &&
500 stream_timestamp < base::TimeDelta() &&
501 buffer->duration() != kNoTimestamp) {
502 if (!audio_decoder_config().codec_delay()) {
503 DCHECK_EQ(buffer->discard_padding().first, base::TimeDelta());
504
505 if (stream_timestamp + buffer->duration() < base::TimeDelta()) {
506 DCHECK_EQ(buffer->discard_padding().second, base::TimeDelta());
507
508 // Discard the entire packet if it's entirely before zero.
509 buffer->set_discard_padding(
510 std::make_pair(kInfiniteDuration, base::TimeDelta()));
511 } else {
512 // Only discard part of the frame if it overlaps zero.
513 buffer->set_discard_padding(std::make_pair(
514 -stream_timestamp, buffer->discard_padding().second));
515 }
516 } else {
517 // Verify that codec delay would cover discard and that we don't need to
518 // mark the packet for post decode discard. Since timestamps may be in
519 // milliseconds and codec delay in nanosecond precision, round up to the
520 // nearest millisecond. See enable_negative_timestamp_fixups().
521 DCHECK_LE(-std::ceil(FramesToTimeDelta(
522 audio_decoder_config().codec_delay(),
523 audio_decoder_config().samples_per_second())
524 .InMillisecondsF()),
525 stream_timestamp.InMillisecondsF());
526 }
527 }
528 } else {
529 // If this happens on the first packet, decoders will throw an error.
530 buffer->set_timestamp(kNoTimestamp);
531 } 531 }
532 532
533 if (last_packet_timestamp_ != kNoTimestamp) { 533 if (last_packet_timestamp_ != kNoTimestamp) {
534 // FFmpeg doesn't support chained ogg correctly. Instead of guaranteeing 534 // FFmpeg doesn't support chained ogg correctly. Instead of guaranteeing
535 // continuity across links in the chain it uses the timestamp information 535 // continuity across links in the chain it uses the timestamp information
536 // from each link directly. Doing so can lead to timestamps which appear to 536 // from each link directly. Doing so can lead to timestamps which appear to
537 // go backwards in time. 537 // go backwards in time.
538 // 538 //
539 // If the new link starts with a negative timestamp or a timestamp less than 539 // If the new link starts with a negative timestamp or a timestamp less than
540 // the original (positive) |start_time|, we will get a negative timestamp 540 // the original (positive) |start_time|, we will get a negative timestamp
541 // here. It's also possible FFmpeg returns kNoTimestamp here if it's not 541 // here.
542 // able to work out a timestamp using the previous link and the next.
543 // 542 //
544 // Fixing chained ogg is non-trivial, so for now just reuse the last good 543 // Fixing chained ogg is non-trivial, so for now just reuse the last good
545 // timestamp. The decoder will rewrite the timestamps to be sample accurate 544 // timestamp. The decoder will rewrite the timestamps to be sample accurate
546 // later. See http://crbug.com/396864. 545 // later. See http://crbug.com/396864.
547 if (fixup_negative_timestamps_ && 546 if (fixup_negative_timestamps_ &&
548 (buffer->timestamp() == kNoTimestamp || 547 buffer->timestamp() < last_packet_timestamp_) {
549 buffer->timestamp() < last_packet_timestamp_)) {
550 buffer->set_timestamp(last_packet_timestamp_ + 548 buffer->set_timestamp(last_packet_timestamp_ +
551 (last_packet_duration_ != kNoTimestamp 549 (last_packet_duration_ != kNoTimestamp
552 ? last_packet_duration_ 550 ? last_packet_duration_
553 : base::TimeDelta::FromMicroseconds(1))); 551 : base::TimeDelta::FromMicroseconds(1)));
554 } 552 }
555 553
556 if (buffer->timestamp() == kNoTimestamp) {
557 // If we didn't get a valid timestamp and didn't fix it up, then fail.
558 demuxer_->NotifyDemuxerError(DEMUXER_ERROR_COULD_NOT_PARSE);
559 return;
560 }
561
562 // The demuxer should always output positive timestamps. 554 // The demuxer should always output positive timestamps.
563 DCHECK(buffer->timestamp() >= base::TimeDelta()); 555 DCHECK(buffer->timestamp() >= base::TimeDelta());
564 556
565 if (last_packet_timestamp_ < buffer->timestamp()) { 557 if (last_packet_timestamp_ < buffer->timestamp()) {
566 buffered_ranges_.Add(last_packet_timestamp_, buffer->timestamp()); 558 buffered_ranges_.Add(last_packet_timestamp_, buffer->timestamp());
567 demuxer_->NotifyBufferingChanged(); 559 demuxer_->NotifyBufferingChanged();
568 } 560 }
569 } 561 }
570 562
571 if (packet.get()->flags & AV_PKT_FLAG_KEY) 563 if (packet.get()->flags & AV_PKT_FLAG_KEY)
(...skipping 1244 matching lines...) Expand 10 before | Expand all | Expand 10 after
1816 1808
1817 void FFmpegDemuxer::SetLiveness(DemuxerStream::Liveness liveness) { 1809 void FFmpegDemuxer::SetLiveness(DemuxerStream::Liveness liveness) {
1818 DCHECK(task_runner_->BelongsToCurrentThread()); 1810 DCHECK(task_runner_->BelongsToCurrentThread());
1819 for (const auto& stream : streams_) { 1811 for (const auto& stream : streams_) {
1820 if (stream) 1812 if (stream)
1821 stream->SetLiveness(liveness); 1813 stream->SetLiveness(liveness);
1822 } 1814 }
1823 } 1815 }
1824 1816
1825 } // namespace media 1817 } // namespace media
OLDNEW
« 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