Skip to content

Commit

Permalink
Handle additional unused 'mdat' properly
Browse files Browse the repository at this point in the history
The spec allows having more than one 'mdat' boxes even if it is not
used.

This is happening on some mp4 files in the wild:
http://www.sample-videos.com/.

Fixes #298.

Change-Id: I729cc94bee095560b5c4b3ee8511323f25f7ad5a
  • Loading branch information
kqyang committed Dec 1, 2017
1 parent 22c758e commit 068e220
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 6 deletions.
22 changes: 16 additions & 6 deletions packager/media/formats/mp4/mp4_media_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,22 @@ bool MP4MediaParser::ParseBox(bool* err) {
return false;

if (reader->type() == FOURCC_mdat) {
// The code ends up here only if a MOOV box is not yet seen.
DCHECK(!moov_);

NOTIMPLEMENTED() << " Files with MDAT before MOOV is not supported yet.";
*err = true;
return false;
if (!moov_) {
// For seekable files, we seek to the 'moov' and load the 'moov' first
// then seek back (see LoadMoov function for details); we do not support
// having 'mdat' before 'moov' for non-seekable files. The code ends up
// here only if it is a non-seekable file.
NOTIMPLEMENTED() << " Non-seekable Files with 'mdat' box before 'moov' "
"box is not supported.";
*err = true;
return false;
} else {
// This can happen if there are unused 'mdat' boxes, which is unusual
// but allowed by the spec. Ignore the 'mdat' and proceed.
LOG(INFO)
<< "Ignore unused 'mdat' box - this could be as a result of extra "
"not usable 'mdat' or 'mdat' associated with unrecognized track.";
}
}

// Set up mdat offset for ReadMDATsUntil().
Expand Down
9 changes: 9 additions & 0 deletions packager/media/formats/mp4/mp4_media_parser_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,15 @@ TEST_F(MP4MediaParserTest, TrailingMoov) {
EXPECT_EQ(201u, num_samples_);
}

TEST_F(MP4MediaParserTest, TrailingMoovAndAdditionalMdat) {
// The additional mdat should just be ignored, so the parse is still
// successful with the same result.
EXPECT_TRUE(
ParseMP4File("bear-640x360-trailing-moov-additional-mdat.mp4", 1024));
EXPECT_EQ(2u, num_streams_);
EXPECT_EQ(201u, num_samples_);
}

TEST_F(MP4MediaParserTest, Flush) {
// Flush while reading sample data, then start a new stream.
InitializeParser(NULL);
Expand Down
1 change: 1 addition & 0 deletions packager/media/test/data/README
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ bear-640x360.mp4 - Same as above, but in a different resolution.
bear-640x360-hevc.mp4 - Same content, but encoded with HEVC.
bear-320x180.mp4 - Same as above, but in a different resolution.
bear-640x360-trailing-moov.mp4 - Same content, but with moov box in the end.
bear-640x360-trailing-moov-additional-mdat.mp4 - Same content, but with moov box in the end and an additional unused mdat, which should be ignored.
bear-640x360-av_frag.mp4 - Same content, but in fragmented mp4.
bear-640x360-aac_lc-silent_right.mp4 - Audio only, stereo, but right channel is silent, with AAC-LC profile.
bear-640x360-aac_he-silent_right.mp4 - Same as above, but with AAC-HE profile.
Expand Down
Binary file not shown.

0 comments on commit 068e220

Please sign in to comment.