From 49dd6e49bb305aec102b2753c9a6c5333195d90e Mon Sep 17 00:00:00 2001
From: KongQun Yang <kqyang@google.com>
Date: Wed, 7 May 2014 14:57:39 -0700
Subject: [PATCH] Always generate version 1 SampleGroupDescription box

Version 0 is obsoleted. Also fixes unittest typos.

Change-Id: I0e536ba316ac07ce939dae71752e00db19e3db58
---
 media/formats/mp4/box_definitions.cc          | 36 +++++--------------
 media/formats/mp4/box_definitions_unittest.cc | 15 +++++---
 2 files changed, 18 insertions(+), 33 deletions(-)

diff --git a/media/formats/mp4/box_definitions.cc b/media/formats/mp4/box_definitions.cc
index 800d87b2f0..32dda6ba85 100644
--- a/media/formats/mp4/box_definitions.cc
+++ b/media/formats/mp4/box_definitions.cc
@@ -1666,7 +1666,7 @@ bool SampleGroupDescription::ReadWrite(BoxBuffer* buffer) {
   if (version == 1) {
     if (buffer->Reading()) {
       RCHECK(buffer->ReadWriteUInt32(&default_length));
-      RCHECK(default_length == 0 || default_length == kEntrySize);
+      RCHECK(default_length == 0 || default_length >= kEntrySize);
     } else {
       default_length = kEntrySize;
       RCHECK(buffer->ReadWriteUInt32(&default_length));
@@ -1681,7 +1681,7 @@ bool SampleGroupDescription::ReadWrite(BoxBuffer* buffer) {
       if (buffer->Reading() && default_length == 0) {
         uint32 description_length = 0;
         RCHECK(buffer->ReadWriteUInt32(&description_length));
-        RCHECK(description_length == kEntrySize);
+        RCHECK(description_length >= kEntrySize);
       }
     }
 
@@ -1707,6 +1707,8 @@ bool SampleGroupDescription::ReadWrite(BoxBuffer* buffer) {
 }
 
 uint32 SampleGroupDescription::ComputeSize() {
+  // Version 0 is obsoleted, so always generate version 1 box.
+  version = 1;
   // This box is optional. Skip it if it is not used.
   atom_size = 0;
   if (!entries.empty()) {
@@ -1733,6 +1735,10 @@ bool TrackFragment::ReadWrite(BoxBuffer* buffer) {
     DCHECK(buffer->reader());
     RCHECK(buffer->reader()->TryReadChildren(&runs));
 
+    // There could be multiple SampleGroupDescription and SampleToGroup boxes
+    // with different grouping types. For common encryption, the relevant
+    // grouping type is 'seig'. Continue reading until 'seig' is found, or
+    // until running out of child boxes.
     while (sample_to_group.grouping_type != FOURCC_SEIG &&
            buffer->reader()->ChildExist(&sample_to_group)) {
       RCHECK(buffer->reader()->ReadChild(&sample_to_group));
@@ -1741,32 +1747,6 @@ bool TrackFragment::ReadWrite(BoxBuffer* buffer) {
            buffer->reader()->ChildExist(&sample_group_description)) {
       RCHECK(buffer->reader()->ReadChild(&sample_group_description));
     }
-    if (sample_to_group.grouping_type == FOURCC_SEIG) {
-      // SampleGroupDescription box can appear in either 'moov...stbl' or
-      // 'moov.traf'. The first case is not supported for now, so we require
-      // a companion SampleGroupDescription box to coexist with the
-      // SampleToGroup box.
-      if (sample_group_description.grouping_type != FOURCC_SEIG) {
-        NOTIMPLEMENTED()
-            << "SampleGroupDescription box in 'moov' is not supported.";
-        return false;
-      }
-      for (std::vector<SampleToGroupEntry>::iterator it =
-               sample_to_group.entries.begin();
-           it != sample_to_group.entries.end();
-           ++it) {
-        if ((it->group_description_index & 0x10000) == 0) {
-          NOTIMPLEMENTED()
-              << "SampleGroupDescription box in 'moov' is not supported.";
-          return false;
-        }
-        it->group_description_index &= 0x0FFFF;
-        RCHECK(it->group_description_index <=
-               sample_group_description.entries.size());
-      }
-    } else {
-      RCHECK(sample_group_description.grouping_type != FOURCC_SEIG);
-    }
   } else {
     for (uint32 i = 0; i < runs.size(); ++i)
       RCHECK(runs[i].ReadWrite(buffer));
diff --git a/media/formats/mp4/box_definitions_unittest.cc b/media/formats/mp4/box_definitions_unittest.cc
index 7d30b2c8c8..f4917d9761 100644
--- a/media/formats/mp4/box_definitions_unittest.cc
+++ b/media/formats/mp4/box_definitions_unittest.cc
@@ -630,12 +630,13 @@ class BoxDefinitionsTestGeneral : public testing::Test {
     sbgp->entries.resize(2);
     sbgp->entries[0].sample_count = 3;
     sbgp->entries[0].group_description_index = 0x10002;
-    sbgp->entries[0].sample_count = 1212;
-    sbgp->entries[0].group_description_index = 0x10001;
+    sbgp->entries[1].sample_count = 1212;
+    sbgp->entries[1].group_description_index = 0x10001;
   }
 
   void Modify(SampleToGroup* sbgp) {
     sbgp->entries.resize(1);
+    sbgp->entries[0].sample_count = 5;
     sbgp->entries[0].group_description_index = 0x10001;
   }
 
@@ -649,11 +650,13 @@ class BoxDefinitionsTestGeneral : public testing::Test {
     sgpd->entries[1].is_encrypted = false;
     sgpd->entries[1].iv_size = 0;
     sgpd->entries[1].key_id.resize(16);
+    sgpd->version = 1;
   }
 
-  void Modify(SampleGroupDescription* sbgp) {
-    sbgp->entries.resize(1);
-    sbgp->entries[0].key_id[4] = 88;
+  void Modify(SampleGroupDescription* sgpd) {
+    sgpd->entries.resize(1);
+    sgpd->entries[0].key_id[4] = 88;
+    sgpd->version = 1;
   }
 
   void Fill(TrackFragment* traf) {
@@ -668,6 +671,8 @@ class BoxDefinitionsTestGeneral : public testing::Test {
   void Modify(TrackFragment* traf) {
     Modify(&traf->header);
     Modify(&traf->decode_time);
+    Fill(&traf->sample_to_group);
+    Fill(&traf->sample_group_description);
   }
 
   void Fill(MovieFragment* moof) {