Bug 1519617 - Update MoofParser to handle tracks using track_id 0. r=jya
☠☠ backed out by a559b84032a8 ☠ ☠
authorBryce Van Dyk <bvandyk@mozilla.com>
Mon, 14 Jan 2019 16:33:03 +0000
changeset 453761 dad40f23f4c19891cbd7e6f805bf1eda699d7b16
parent 453760 944a2fbd7454ba098144293f95f5fe6011339073
child 453762 b430ac03ce29256a1775ff46fd2864301b19b623
push id35372
push usercbrindusan@mozilla.com
push dateMon, 14 Jan 2019 21:49:33 +0000
treeherdermozilla-central@50b3268954b1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1519617
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1519617 - Update MoofParser to handle tracks using track_id 0. r=jya Using track_id 0 is forbidden by the mp4 spec, however, some sites still serve media using this track_id. We've been using the 0 track ID to trigger special handling in the MoofParser where we will parse multiple tracks, and this led us to be tolerant of tracks using this reserved id (though we likely had some bugs due to this). Since sites are using this track_id, and as other browsers (and Firefox until I broke this) tolerate such media, we should too. In order to do so correctly, we should no longer us track_id=0 as a special case in the MoofParser, and instead have an explicit flag, which is what this patch does. Differential Revision: https://phabricator.services.mozilla.com/D16428
dom/media/gtest/mp4_demuxer/TestParser.cpp
dom/media/mediasource/ContainerParser.cpp
dom/media/mp4/MoofParser.cpp
dom/media/mp4/MoofParser.h
--- a/dom/media/gtest/mp4_demuxer/TestParser.cpp
+++ b/dom/media/gtest/mp4_demuxer/TestParser.cpp
@@ -89,17 +89,17 @@ TEST(MP4Metadata, EmptyStream) {
   // We can seek anywhere in any MPEG4.
   EXPECT_TRUE(metadata.CanSeek());
   EXPECT_FALSE(metadata.Crypto().Ref()->valid);
 }
 
 TEST(MoofParser, EmptyStream) {
   RefPtr<ByteStream> stream = new TestStream(nullptr, 0);
 
-  MoofParser parser(stream, 0, false);
+  MoofParser parser(stream, 0, false, true);
   EXPECT_EQ(0u, parser.mOffset);
   EXPECT_TRUE(parser.ReachedEnd());
 
   MediaByteRangeSet byteRanges;
   EXPECT_FALSE(parser.RebuildFragmentedIndex(byteRanges));
 
   EXPECT_TRUE(parser.GetCompositionRange(byteRanges).IsNull());
   EXPECT_TRUE(parser.mInitRange.IsEmpty());
@@ -399,17 +399,17 @@ TEST(MoofParser, test_case_mp4) {
   length = ArrayLength(testFiles);
 
   for (size_t test = 0; test < length; ++test) {
     nsTArray<uint8_t> buffer = ReadTestFile(tests[test].mFilename);
     ASSERT_FALSE(buffer.IsEmpty());
     RefPtr<ByteStream> stream =
         new TestStream(buffer.Elements(), buffer.Length());
 
-    MoofParser parser(stream, 0, false);
+    MoofParser parser(stream, 0, false, true);
     EXPECT_EQ(0u, parser.mOffset) << tests[test].mFilename;
     EXPECT_FALSE(parser.ReachedEnd()) << tests[test].mFilename;
     EXPECT_TRUE(parser.mInitRange.IsEmpty()) << tests[test].mFilename;
 
     RefPtr<MediaByteBuffer> metadataBuffer = parser.Metadata();
     EXPECT_TRUE(metadataBuffer) << tests[test].mFilename;
 
     EXPECT_FALSE(parser.mInitRange.IsEmpty()) << tests[test].mFilename;
--- a/dom/media/mediasource/ContainerParser.cpp
+++ b/dom/media/mediasource/ContainerParser.cpp
@@ -521,17 +521,18 @@ class MP4ContainerParser : public Contai
     if (initSegment) {
       mResource = new SourceBufferResource();
       DDLINKCHILD("resource", mResource.get());
       mStream = new MP4Stream(mResource);
       // We use a timestampOffset of 0 for ContainerParser, and require
       // consumers of ParseStartAndEndTimestamps to add their timestamp offset
       // manually. This allows the ContainerParser to be shared across different
       // timestampOffsets.
-      mParser = new MoofParser(mStream, 0, /* aIsAudio = */ false);
+      mParser = new MoofParser(mStream, 0, /* aIsAudio = */ false,
+                               /* aIsMultitrackParser */ true);
       DDLINKCHILD("parser", mParser.get());
       mInitData = new MediaByteBuffer();
       mCompleteInitSegmentRange = MediaByteRange();
       mCompleteMediaHeaderRange = MediaByteRange();
       mCompleteMediaSegmentRange = MediaByteRange();
       mGlobalOffset = mTotalParsed;
     } else if (!mStream || !mParser) {
       mTotalParsed += aData->Length();
--- a/dom/media/mp4/MoofParser.cpp
+++ b/dom/media/mp4/MoofParser.cpp
@@ -222,21 +222,21 @@ void MoofParser::ParseMoov(Box& aBox) {
 }
 
 void MoofParser::ParseTrak(Box& aBox) {
   Tkhd tkhd;
   for (Box box = aBox.FirstChild(); box.IsAvailable(); box = box.Next()) {
     if (box.IsType("tkhd")) {
       tkhd = Tkhd(box);
     } else if (box.IsType("mdia")) {
-      if (!mTrex.mTrackId || tkhd.mTrackId == mTrex.mTrackId) {
+      if (mIsMultitrackParser || tkhd.mTrackId == mTrex.mTrackId) {
         ParseMdia(box, tkhd);
       }
     } else if (box.IsType("edts") &&
-               (!mTrex.mTrackId || tkhd.mTrackId == mTrex.mTrackId)) {
+               (mIsMultitrackParser || tkhd.mTrackId == mTrex.mTrackId)) {
       mEdts = Edts(box);
     }
   }
 }
 
 void MoofParser::ParseMdia(Box& aBox, Tkhd& aTkhd) {
   for (Box box = aBox.FirstChild(); box.IsAvailable(); box = box.Next()) {
     if (box.IsType("mdhd")) {
@@ -246,22 +246,18 @@ void MoofParser::ParseMdia(Box& aBox, Tk
     }
   }
 }
 
 void MoofParser::ParseMvex(Box& aBox) {
   for (Box box = aBox.FirstChild(); box.IsAvailable(); box = box.Next()) {
     if (box.IsType("trex")) {
       Trex trex = Trex(box);
-      if (!mTrex.mTrackId || trex.mTrackId == mTrex.mTrackId) {
-        auto trackId = mTrex.mTrackId;
+      if (mIsMultitrackParser || trex.mTrackId == mTrex.mTrackId) {
         mTrex = trex;
-        // Keep the original trackId, as should it be 0 we want to continue
-        // parsing all tracks.
-        mTrex.mTrackId = trackId;
       }
     }
   }
 }
 
 void MoofParser::ParseMinf(Box& aBox) {
   for (Box box = aBox.FirstChild(); box.IsAvailable(); box = box.Next()) {
     if (box.IsType("stbl")) {
@@ -294,18 +290,18 @@ void MoofParser::ParseStbl(Box& aBox) {
           return;
         }
       }
     }
   }
 }
 
 void MoofParser::ParseStsd(Box& aBox) {
-  if (mTrex.mTrackId == 0) {
-    // If mTrex.mTrackId is 0, then the parser is being used to read multiple
+  if (mIsMultitrackParser) {
+    // If mIsMultitrackParser, then the parser is being used to read multiple
     // tracks metadata, and it is not a sane operation to try and map multiple
     // sample description boxes, from different tracks, onto the parser, which
     // is modeled around storing metadata for a single track.
     return;
   }
   MOZ_ASSERT(
       mSampleDescriptions.IsEmpty(),
       "Shouldn't have any sample descriptions when starting to parse stsd");
--- a/dom/media/mp4/MoofParser.h
+++ b/dom/media/mp4/MoofParser.h
@@ -262,24 +262,28 @@ class Moof final : public Atom {
   bool ProcessCencAuxInfo(AtomType aScheme);
   uint64_t mMaxRoundingError;
 };
 
 DDLoggedTypeDeclName(MoofParser);
 
 class MoofParser : public DecoderDoctorLifeLogger<MoofParser> {
  public:
-  MoofParser(ByteStream* aSource, uint32_t aTrackId, bool aIsAudio)
+  MoofParser(ByteStream* aSource, uint32_t aTrackId, bool aIsAudio,
+             bool aIsMultitrackParser = false)
       : mSource(aSource),
         mOffset(0),
         mTrex(aTrackId),
         mIsAudio(aIsAudio),
-        mLastDecodeTime(0) {
-    // Setting the mTrex.mTrackId to 0 is a nasty work around for calculating
-    // the composition range for MSE. We need an array of tracks.
+        mLastDecodeTime(0),
+        mIsMultitrackParser(aIsMultitrackParser) {
+    // Setting mIsMultitrackParser is a nasty work around for calculating
+    // the composition range for MSE that causes the parser to parse multiple
+    // tracks. Ideally we'd store an array of tracks with different metadata
+    // for each.
     DDLINKCHILD("source", aSource);
   }
   bool RebuildFragmentedIndex(const mozilla::MediaByteRangeSet& aByteRanges);
   // If *aCanEvict is set to true. then will remove all moofs already parsed
   // from index then rebuild the index. *aCanEvict is set to true upon return if
   // some moofs were removed.
   bool RebuildFragmentedIndex(const mozilla::MediaByteRangeSet& aByteRanges,
                               bool* aCanEvict);
@@ -321,12 +325,13 @@ class MoofParser : public DecoderDoctorL
   nsTArray<Moof>& Moofs() { return mMoofs; }
 
  private:
   void ScanForMetadata(mozilla::MediaByteRange& aMoov);
   nsTArray<Moof> mMoofs;
   nsTArray<MediaByteRange> mMediaRanges;
   bool mIsAudio;
   uint64_t mLastDecodeTime;
+  bool mIsMultitrackParser;
 };
 }  // namespace mozilla
 
 #endif