Fix mp4 keyframe parsing, removing unused stss parsing.

media/formats/mp4/track_run_iterator_unittest.cc

Index: media/formats/mp4/track_run_iterator_unittest.cc
diff --git a/media/formats/mp4/track_run_iterator_unittest.cc b/media/formats/mp4/track_run_iterator_unittest.cc
index f9dde7204f1c169dd313739820a0743b906c2283..39122a56ed9590e35bb88fd29941b4cc56c3e14d 100644
--- a/media/formats/mp4/track_run_iterator_unittest.cc
+++ b/media/formats/mp4/track_run_iterator_unittest.cc
@@ -73,8 +73,6 @@ class TrackRunIteratorTest : public testing::Test {
     desc1.audio_entries.push_back(aud_desc);
     moov_.extends.tracks[0].track_id = 1;
     moov_.extends.tracks[0].default_sample_description_index = 1;
-    moov_.tracks[0].media.information.sample_table.sync_sample.is_present =
-        false;
     moov_.tracks[1].header.track_id = 2;
     moov_.tracks[1].media.header.timescale = kVideoScale;
     SampleDescription& desc2 =
@@ -86,11 +84,6 @@ class TrackRunIteratorTest : public testing::Test {
     desc2.video_entries.push_back(vid_desc);
     moov_.extends.tracks[1].track_id = 2;
     moov_.extends.tracks[1].default_sample_description_index = 1;
-    SyncSample& video_sync_sample =
-        moov_.tracks[1].media.information.sample_table.sync_sample;
-    video_sync_sample.is_present = true;
-    video_sync_sample.entries.resize(1);
-    video_sync_sample.entries[0] = 0;
 
     moov_.tracks[2].header.track_id = 3;
     moov_.tracks[2].media.information.sample_table.description.type = kHint;
@@ -170,8 +163,6 @@ class TrackRunIteratorTest : public testing::Test {
 
     while (iter->IsSampleValid()) {
       ss << " " << (iter->is_keyframe() ? "K" : "P");
-      if (iter->is_random_access_point())
-        ss << "R";
       iter->AdvanceSample();
     }
 
@@ -381,10 +372,10 @@ TEST_F(TrackRunIteratorTest, FirstSampleFlagTest) {
   SetFlagsOnSamples("US", &moof.tracks[1].runs[0]);
 
   ASSERT_TRUE(iter_->Init(moof));
-  EXPECT_EQ("1 KR KR KR KR KR KR KR KR KR KR", KeyframeAndRAPInfo(iter_.get()));
+  EXPECT_EQ("1 K K K K K K K K K K", KeyframeAndRAPInfo(iter_.get()));
 
   iter_->AdvanceRun();
-  EXPECT_EQ("2 KR P P P P P P P P P", KeyframeAndRAPInfo(iter_.get()));
+  EXPECT_EQ("2 K P P P P P P P P P", KeyframeAndRAPInfo(iter_.get()));
 }
 
 // Verify that parsing fails if a reserved value is in the sample flags.
@@ -672,45 +663,43 @@ TEST_F(TrackRunIteratorTest, UnexpectedOrderingTest) {
   EXPECT_EQ(iter_->GetMaxClearOffset(), 10000);
 }
 
-TEST_F(TrackRunIteratorTest, MissingAndEmptyStss) {
+TEST_F(TrackRunIteratorTest, KeyFrameFlagCombinations) {
+  // Setup both audio and video tracks to each have 6 samples covering all the
+  // combinations of mp4 "sync sample" and "depends on" relationships.
   MovieFragment moof = CreateFragment();
-
-  // Setup track 0 to not have an stss box, which means that all samples should
-  // be marked as random access points unless the kSampleIsNonSyncSample flag is
-  // set in the sample flags.
-  moov_.tracks[0].media.information.sample_table.sync_sample.is_present = false;
-  moov_.tracks[0].media.information.sample_table.sync_sample.entries.resize(0);
   moof.tracks[0].runs.resize(1);
-  moof.tracks[0].runs[0].sample_count = 6;
-  moof.tracks[0].runs[0].data_offset = 100;
-  SetFlagsOnSamples("US UN OS ON NS NN", &moof.tracks[0].runs[0]);
-
-  // Setup track 1 to have an stss box with no entries, which normally means
-  // that none of the samples should be random access points. If the
-  // kSampleIsNonSyncSample flag is NOT set though, the sample should be
-  // considered a random access point.
-  moov_.tracks[1].media.information.sample_table.sync_sample.is_present = true;
-  moov_.tracks[1].media.information.sample_table.sync_sample.entries.resize(0);
   moof.tracks[1].runs.resize(1);
+  moof.tracks[0].runs[0].sample_count = 6;
   moof.tracks[1].runs[0].sample_count = 6;
-  moof.tracks[1].runs[0].data_offset = 200;
+  SetFlagsOnSamples("US UN OS ON NS NN", &moof.tracks[0].runs[0]);
   SetFlagsOnSamples("US UN OS ON NS NN", &moof.tracks[1].runs[0]);
-
   iter_.reset(new TrackRunIterator(&moov_, media_log_));
 
   ASSERT_TRUE(iter_->Init(moof));
   EXPECT_TRUE(iter_->IsRunValid());
 
-  // Verify that all samples except for the ones that have the
-  // kSampleIsNonSyncSample flag set are marked as random access points.
-  EXPECT_EQ("1 KR P PR P KR K", KeyframeAndRAPInfo(iter_.get()));
+  // Keyframes should be marked according to downstream's expectations that
+  // keyframes serve as points of random access for seeking.
+
+  // For audio, any sync sample should be marked as a key frame. Whether a
+  // sample "depends on" other samples is not considered. Unlike video samples,
+  // audio samples are often marked as depending on other samples but are still
+  // workable for random access. While we allow for parsing of audio samples
+  // that are non-sync samples, we generally expect all audio samples to be sync
+  // samples and downstream will log and discard any non-sync audio samples.
+  EXPECT_EQ("1 K P K P K P", KeyframeAndRAPInfo(iter_.get()));
 
   iter_->AdvanceRun();
 
-  // Verify that nothing is marked as a random access point.
-  EXPECT_EQ("2 KR P PR P KR K", KeyframeAndRAPInfo(iter_.get()));
+  // For video, any key frame should be both a sync sample and have no known
+  // dependents. Ideally, a video sync sample should always be marked as having
+  // no dependents, but we occasionally encounter media where all samples are
+  // marked "sync" and we must rely on combining the two flags to pick out the
+  // true key frames. See http://crbug.com/310712 and http://crbug.com/507916.
+  // Realiably knowing the keyframes for video is also critical to SPS PPS
+  // insertion.
+  EXPECT_EQ("2 K P P P K P", KeyframeAndRAPInfo(iter_.get()));
 }
 
-
 }  // namespace mp4
 }  // namespace media