Skip to content

Commit

Permalink
MSE - WebM: Fix default duration precision capping
Browse files Browse the repository at this point in the history
Sub-microsecond timescales previously could result in incorrect results
that fail the assumption that the result must either be kNoTimestamp or
at least 1 microsecond.

BUG=937846
TEST=WebMTracksParserTest.PrecisionCapping (plus no local regression of
     existing media_unittests)

Change-Id: I2519f23a730e5ddca4dca2a30d749e7484c3cb6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1529699
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#642085}
  • Loading branch information
wolenetz authored and Commit Bot committed Mar 19, 2019
1 parent 53ed26b commit efd59e7
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 52 deletions.
4 changes: 2 additions & 2 deletions media/formats/webm/webm_cluster_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ enum {
};

WebMClusterParser::WebMClusterParser(
int64_t timecode_scale,
int64_t timecode_scale_ns,
int audio_track_num,
base::TimeDelta audio_default_duration,
int video_track_num,
Expand All @@ -47,7 +47,7 @@ WebMClusterParser::WebMClusterParser(
const std::string& video_encryption_key_id,
const AudioCodec audio_codec,
MediaLog* media_log)
: timecode_multiplier_(timecode_scale / 1000.0),
: timecode_multiplier_(timecode_scale_ns / 1000.0),
ignored_tracks_(ignored_tracks),
audio_encryption_key_id_(audio_encryption_key_id),
video_encryption_key_id_(video_encryption_key_id),
Expand Down
2 changes: 1 addition & 1 deletion media/formats/webm/webm_cluster_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class MEDIA_EXPORT WebMClusterParser : public WebMParserClient {
typedef std::map<int, Track> TextTrackMap;

public:
WebMClusterParser(int64_t timecode_scale,
WebMClusterParser(int64_t timecode_scale_ns,
int audio_track_num,
base::TimeDelta audio_default_duration,
int video_track_num,
Expand Down
19 changes: 8 additions & 11 deletions media/formats/webm/webm_info_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,16 @@

namespace media {

// Default timecode scale if the TimecodeScale element is
// not specified in the INFO element.
// Default timecode scale, in nanoseconds, if the TimecodeScale element is not
// specified in the INFO element.
static const int kWebMDefaultTimecodeScale = 1000000;

WebMInfoParser::WebMInfoParser()
: timecode_scale_(-1),
duration_(-1) {
}
WebMInfoParser::WebMInfoParser() : timecode_scale_ns_(-1), duration_(-1) {}

WebMInfoParser::~WebMInfoParser() = default;

int WebMInfoParser::Parse(const uint8_t* buf, int size) {
timecode_scale_ = -1;
timecode_scale_ns_ = -1;
duration_ = -1;

WebMListParser parser(kWebMIdInfo, this);
Expand All @@ -37,10 +34,10 @@ int WebMInfoParser::Parse(const uint8_t* buf, int size) {
WebMParserClient* WebMInfoParser::OnListStart(int id) { return this; }

bool WebMInfoParser::OnListEnd(int id) {
if (id == kWebMIdInfo && timecode_scale_ == -1) {
if (id == kWebMIdInfo && timecode_scale_ns_ == -1) {
// Set timecode scale to default value if it isn't present in
// the Info element.
timecode_scale_ = kWebMDefaultTimecodeScale;
timecode_scale_ns_ = kWebMDefaultTimecodeScale;
}
return true;
}
Expand All @@ -54,12 +51,12 @@ bool WebMInfoParser::OnUInt(int id, int64_t val) {
return false;
}

if (timecode_scale_ != -1) {
if (timecode_scale_ns_ != -1) {
DVLOG(1) << "Multiple values for id " << std::hex << id << " specified";
return false;
}

timecode_scale_ = val;
timecode_scale_ns_ = val;
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions media/formats/webm/webm_info_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class MEDIA_EXPORT WebMInfoParser : public WebMParserClient {
// Returns the number of bytes parsed on success.
int Parse(const uint8_t* buf, int size);

int64_t timecode_scale() const { return timecode_scale_; }
int64_t timecode_scale_ns() const { return timecode_scale_ns_; }
double duration() const { return duration_; }
base::Time date_utc() const { return date_utc_; }

Expand All @@ -41,7 +41,7 @@ class MEDIA_EXPORT WebMInfoParser : public WebMParserClient {
bool OnBinary(int id, const uint8_t* data, int size) override;
bool OnString(int id, const std::string& str) override;

int64_t timecode_scale_;
int64_t timecode_scale_ns_;
double duration_;
base::Time date_utc_;

Expand Down
9 changes: 5 additions & 4 deletions media/formats/webm/webm_stream_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ int WebMStreamParser::ParseInfoAndTracks(const uint8_t* data, int size) {

bytes_parsed += result;

double timecode_scale_in_us = info_parser.timecode_scale() / 1000.0;
int64_t timecode_scale_in_ns = info_parser.timecode_scale_ns();
double timecode_scale_in_us = timecode_scale_in_ns / 1000.0;
InitParameters params(kInfiniteDuration);

if (info_parser.duration() > 0) {
Expand Down Expand Up @@ -237,10 +238,10 @@ int WebMStreamParser::ParseInfoAndTracks(const uint8_t* data, int size) {
}

cluster_parser_.reset(new WebMClusterParser(
info_parser.timecode_scale(), tracks_parser.audio_track_num(),
tracks_parser.GetAudioDefaultDuration(timecode_scale_in_us),
timecode_scale_in_ns, tracks_parser.audio_track_num(),
tracks_parser.GetAudioDefaultDuration(timecode_scale_in_ns),
tracks_parser.video_track_num(),
tracks_parser.GetVideoDefaultDuration(timecode_scale_in_us),
tracks_parser.GetVideoDefaultDuration(timecode_scale_in_ns),
tracks_parser.text_tracks(), tracks_parser.ignored_tracks(),
tracks_parser.audio_encryption_key_id(),
tracks_parser.video_encryption_key_id(), audio_config.codec(),
Expand Down
41 changes: 22 additions & 19 deletions media/formats/webm/webm_tracks_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,6 @@ static TextKind CodecIdToTextKind(const std::string& codec_id) {
return kTextNone;
}

static base::TimeDelta PrecisionCappedDefaultDuration(
const double timecode_scale_in_us,
const int64_t duration_in_ns) {
if (duration_in_ns <= 0)
return kNoTimestamp;

int64_t mult = duration_in_ns / 1000;
mult /= timecode_scale_in_us;
if (mult == 0)
return kNoTimestamp;

mult = static_cast<double>(mult) * timecode_scale_in_us;
return base::TimeDelta::FromMicroseconds(mult);
}

WebMTracksParser::WebMTracksParser(MediaLog* media_log, bool ignore_text_tracks)
: ignore_text_tracks_(ignore_text_tracks),
media_log_(media_log),
Expand All @@ -55,6 +40,24 @@ WebMTracksParser::WebMTracksParser(MediaLog* media_log, bool ignore_text_tracks)

WebMTracksParser::~WebMTracksParser() = default;

base::TimeDelta WebMTracksParser::PrecisionCappedDefaultDuration(
const int64_t timecode_scale_in_ns,
const int64_t duration_in_ns) const {
DCHECK_GT(timecode_scale_in_ns, 0);
if (duration_in_ns <= 0)
return kNoTimestamp;

// Calculate the (integral) number of complete |timecode_scale_in_ns|
// intervals that fit within |duration_in_ns|.
int64_t intervals = duration_in_ns / timecode_scale_in_ns;

int64_t result_us = (intervals * timecode_scale_in_ns) / 1000;
if (result_us == 0)
return kNoTimestamp;

return base::TimeDelta::FromMicroseconds(result_us);
}

void WebMTracksParser::Reset() {
ResetTrackEntry();
reset_on_next_parse_ = false;
Expand Down Expand Up @@ -103,14 +106,14 @@ int WebMTracksParser::Parse(const uint8_t* buf, int size) {
}

base::TimeDelta WebMTracksParser::GetAudioDefaultDuration(
const double timecode_scale_in_us) const {
return PrecisionCappedDefaultDuration(timecode_scale_in_us,
const int64_t timecode_scale_in_ns) const {
return PrecisionCappedDefaultDuration(timecode_scale_in_ns,
audio_default_duration_);
}

base::TimeDelta WebMTracksParser::GetVideoDefaultDuration(
const double timecode_scale_in_us) const {
return PrecisionCappedDefaultDuration(timecode_scale_in_us,
const int64_t timecode_scale_in_ns) const {
return PrecisionCappedDefaultDuration(timecode_scale_in_ns,
video_default_duration_);
}

Expand Down
19 changes: 16 additions & 3 deletions media/formats/webm/webm_tracks_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ class MEDIA_EXPORT WebMTracksParser : public WebMParserClient {

// If TrackEntry DefaultDuration field existed for the associated audio or
// video track, returns that value converted from ns to base::TimeDelta with
// precision not greater than |timecode_scale_in_us|. Defaults to
// precision not greater than |timecode_scale_in_ns|. Defaults to
// kNoTimestamp.
base::TimeDelta GetAudioDefaultDuration(
const double timecode_scale_in_us) const;
const int64_t timecode_scale_in_ns) const;
base::TimeDelta GetVideoDefaultDuration(
const double timecode_scale_in_us) const;
const int64_t timecode_scale_in_ns) const;

const std::set<int64_t>& ignored_tracks() const { return ignored_tracks_; }

Expand Down Expand Up @@ -93,6 +93,19 @@ class MEDIA_EXPORT WebMTracksParser : public WebMParserClient {
}

private:
// To test PrecisionCappedDefaultDuration.
FRIEND_TEST_ALL_PREFIXES(WebMTracksParserTest, PrecisionCapping);

// Returns the conversion of |duration_in_ns| to a microsecond-granularity
// TimeDelta with precision no greater than |timecode_scale_in_ns|.
// Returns kNoTimestamp if |duration_in_ns| is <= 0, or the capped precision
// of the converted |duration_in_ns| is < 1 microsecond.
// Commonly, |timecode_scale_in_ns| is 1000000 (1 millisecond), though the
// muxed stream could have used a different time scale.
base::TimeDelta PrecisionCappedDefaultDuration(
const int64_t timecode_scale_in_ns,
const int64_t duration_in_ns) const;

void Reset();
void ResetTrackEntry();

Expand Down
64 changes: 54 additions & 10 deletions media/formats/webm/webm_tracks_parser_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ using ::testing::_;

namespace media {

static const double kDefaultTimecodeScaleInUs = 1000.0; // 1 ms resolution
// WebM muxing commonly uses 1 millisecond resolution.
static const int64_t kOneMsInNs = 1000000;

class WebMTracksParserTest : public testing::Test {
public:
Expand Down Expand Up @@ -152,10 +153,8 @@ TEST_F(WebMTracksParserTest, AudioVideoDefaultDurationUnset) {
EXPECT_LE(0, result);
EXPECT_EQ(static_cast<int>(buf.size()), result);

EXPECT_EQ(kNoTimestamp,
parser->GetAudioDefaultDuration(kDefaultTimecodeScaleInUs));
EXPECT_EQ(kNoTimestamp,
parser->GetVideoDefaultDuration(kDefaultTimecodeScaleInUs));
EXPECT_EQ(kNoTimestamp, parser->GetAudioDefaultDuration(kOneMsInNs));
EXPECT_EQ(kNoTimestamp, parser->GetVideoDefaultDuration(kOneMsInNs));

const VideoDecoderConfig& video_config = parser->video_decoder_config();
EXPECT_TRUE(video_config.IsValidConfig());
Expand Down Expand Up @@ -183,14 +182,14 @@ TEST_F(WebMTracksParserTest, AudioVideoDefaultDurationSet) {
EXPECT_EQ(static_cast<int>(buf.size()), result);

EXPECT_EQ(base::TimeDelta::FromMicroseconds(12000),
parser->GetAudioDefaultDuration(kDefaultTimecodeScaleInUs));
parser->GetAudioDefaultDuration(kOneMsInNs));
EXPECT_EQ(base::TimeDelta::FromMicroseconds(985000),
parser->GetVideoDefaultDuration(5000.0)); // 5 ms resolution
EXPECT_EQ(kNoTimestamp, parser->GetAudioDefaultDuration(12346.0));
parser->GetVideoDefaultDuration(5000000)); // 5 ms resolution
EXPECT_EQ(kNoTimestamp, parser->GetAudioDefaultDuration(12346000));
EXPECT_EQ(base::TimeDelta::FromMicroseconds(12345),
parser->GetAudioDefaultDuration(12345.0));
parser->GetAudioDefaultDuration(12345000));
EXPECT_EQ(base::TimeDelta::FromMicroseconds(12003),
parser->GetAudioDefaultDuration(1000.3)); // 1.0003 ms resolution
parser->GetAudioDefaultDuration(1000300)); // 1.0003 ms resolution
}

TEST_F(WebMTracksParserTest, InvalidZeroDefaultDurationSet) {
Expand Down Expand Up @@ -251,4 +250,49 @@ TEST_F(WebMTracksParserTest, HighTrackUID) {
EXPECT_GT(parser->Parse(&buf[0], buf.size()),0);
}

TEST_F(WebMTracksParserTest, PrecisionCapping) {
struct CappingCases {
int64_t scale_ns;
int64_t duration_ns;
base::TimeDelta expected_result;
};

const CappingCases kCappingCases[] = {
{kOneMsInNs, -1, kNoTimestamp},
{kOneMsInNs, 0, kNoTimestamp},
{kOneMsInNs, 1, kNoTimestamp},
{kOneMsInNs, 999999, kNoTimestamp},
{kOneMsInNs, 1000000, base::TimeDelta::FromMilliseconds(1)},
{kOneMsInNs, 1000001, base::TimeDelta::FromMilliseconds(1)},
{kOneMsInNs, 1999999, base::TimeDelta::FromMilliseconds(1)},
{kOneMsInNs, 2000000, base::TimeDelta::FromMilliseconds(2)},
{1, -1, kNoTimestamp},
{1, 0, kNoTimestamp},

// Result < 1us, so kNoTimestamp
{1, 1, kNoTimestamp},
{1, 999, kNoTimestamp},

{1, 1000, base::TimeDelta::FromMicroseconds(1)},
{1, 1999, base::TimeDelta::FromMicroseconds(1)},
{1, 2000, base::TimeDelta::FromMicroseconds(2)},

{64, 1792, base::TimeDelta::FromMicroseconds(1)},
};

std::unique_ptr<WebMTracksParser> parser(
new WebMTracksParser(&media_log_, false));

for (size_t i = 0; i < base::size(kCappingCases); ++i) {
InSequence s;
int64_t scale_ns = kCappingCases[i].scale_ns;
int64_t duration_ns = kCappingCases[i].duration_ns;
base::TimeDelta expected_result = kCappingCases[i].expected_result;

EXPECT_EQ(parser->PrecisionCappedDefaultDuration(scale_ns, duration_ns),
expected_result)
<< i << ": " << scale_ns << ", " << duration_ns;
}
}

} // namespace media

0 comments on commit efd59e7

Please sign in to comment.