Bug 1515471 - Tidy mp4parser test cases. r=jya
authorBryce Van Dyk <bvandyk@mozilla.com>
Thu, 03 Jan 2019 14:14:24 +0000
changeset 509670 416d0d25d932ececd8f54036429f3fb91e4c351d
parent 509669 e9b36f8f6c2231074a9de8489d0cdfecfe78d7be
child 509671 8bbbc4ae3211427848d827d96c62dd0220374b6f
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1515471, 1224019
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 1515471 - Tidy mp4parser test cases. r=jya Driveby tidying: - Tidy and clarify comment impacted by global clang-formatting. - Update comments on two test cases disabled by bug 1224019. These tests no longer have log spam issues, and have some value as sanity tests, but remain disabled due to being resource hogs. Comments indicate this. Differential Revision: https://phabricator.services.mozilla.com/D15441
dom/media/gtest/mp4_demuxer/TestParser.cpp
--- a/dom/media/gtest/mp4_demuxer/TestParser.cpp
+++ b/dom/media/gtest/mp4_demuxer/TestParser.cpp
@@ -161,18 +161,18 @@ struct TestFileData {
   bool mHasCrypto;
   uint64_t mMoofReachedOffset;  // or 0 for the end.
   bool mValidMoof;
   bool mHeader;
   int8_t mAudioProfile;
 };
 
 static const TestFileData testFiles[] = {
-    // filename                     parse #V dur   w    h  #A dur  crypt  off
-    // moof  headr  audio_profile
+    // filename parses? #V hasVideoIndex vDur w h #A aDur hasCrypto? moofOffset
+    // validMoof? hasHeader? audio_profile
     {"test_case_1156505.mp4", false, 0, false, -1, 0, 0, 0, -1, false, 152,
      false, false, 0},  // invalid ''trak box
     {"test_case_1181213.mp4", true, 1, true, 416666, 320, 240, 1, 477460, true,
      0, false, false, 2},
     {"test_case_1181215.mp4", true, 0, false, -1, 0, 0, 0, -1, false, 0, false,
      false, 0},
     {"test_case_1181220.mp4", false, 0, false, -1, 0, 0, 0, -1, false, 0, false,
      false, 0},  // invalid audio 'sinf' box
@@ -344,20 +344,23 @@ TEST(MP4Metadata, test_case_mp4) {
         << tests[test].mFilename;
     // We can see anywhere in any MPEG4.
     EXPECT_TRUE(metadata.CanSeek()) << tests[test].mFilename;
     EXPECT_EQ(tests[test].mHasCrypto, metadata.Crypto().Ref()->valid)
         << tests[test].mFilename;
   }
 }
 
-// Bug 1224019: This test produces way to much output, disabling for now.
+// This test was disabled by Bug 1224019 for producing way too much output.
+// This test no longer produces such output, as we've moved away from
+// stagefright, but it does take a long time to run. I can be useful to enable
+// as a sanity check on changes to the parser, but is too taxing to run as part
+// of normal test execution.
 #if 0
-TEST(MPEG4Metadata, test_case_mp4_subsets)
-{
+TEST(MP4Metadata, test_case_mp4_subsets) {
   static const size_t step = 1u;
   for (size_t test = 0; test < ArrayLength(testFiles); ++test) {
     nsTArray<uint8_t> buffer = ReadTestFile(testFiles[test].mFilename);
     ASSERT_FALSE(buffer.IsEmpty());
     ASSERT_LE(step, buffer.Length());
     // Just exercizing the parser starting at different points through the file,
     // making sure it doesn't crash.
     // No checks because results would differ for each position.
@@ -428,20 +431,23 @@ TEST(MoofParser, test_case_mp4) {
         << tests[test].mFilename;
     EXPECT_TRUE(parser.FirstCompleteMediaSegment().IsEmpty())
         << tests[test].mFilename;
     EXPECT_EQ(tests[test].mHeader, !parser.FirstCompleteMediaHeader().IsEmpty())
         << tests[test].mFilename;
   }
 }
 
-// Bug 1224019: This test produces way to much output, disabling for now.
+// This test was disabled by Bug 1224019 for producing way too much output.
+// This test no longer produces such output, as we've moved away from
+// stagefright, but it does take a long time to run. I can be useful to enable
+// as a sanity check on changes to the parser, but is too taxing to run as part
+// of normal test execution.
 #if 0
-TEST(MoofParser, test_case_mp4_subsets)
-{
+TEST(MoofParser, test_case_mp4_subsets) {
   const size_t step = 1u;
   for (size_t test = 0; test < ArrayLength(testFiles); ++test) {
     nsTArray<uint8_t> buffer = ReadTestFile(testFiles[test].mFilename);
     ASSERT_FALSE(buffer.IsEmpty());
     ASSERT_LE(step, buffer.Length());
     // Just exercizing the parser starting at different points through the file,
     // making sure it doesn't crash.
     // No checks because results would differ for each position.