Remove top level box check in BoxReader

Change-Id: I3d0e5865851030264d7174031a98031b413e90ce

packager/media/formats/mp4/box_definitions_unittest.cc

@@ -49,7 +49,7 @@ class BoxDefinitionsTestGeneral : public testing::Test {
     buffer_->AppendInt(static_cast<uint32_t>(FOURCC_skip));
     buffer_->AppendBuffer(buffer);
     bool err = false;
-    return BoxReader::ReadTopLevelBox(buffer_->Buffer(), buffer_->Size(), &err);
+    return BoxReader::ReadBox(buffer_->Buffer(), buffer_->Size(), &err);
   }
 
   bool ReadBack(Box* box) {

packager/media/formats/mp4/box_reader.cc

@@ -34,9 +34,9 @@ BoxReader::~BoxReader() {
 }
 
 // static
-BoxReader* BoxReader::ReadTopLevelBox(const uint8_t* buf,
+BoxReader* BoxReader::ReadBox(const uint8_t* buf,
-                                      const size_t buf_size,
+                              const size_t buf_size,
-                                      bool* err) {
+                              bool* err) {
   scoped_ptr<BoxReader> reader(new BoxReader(buf, buf_size));
   if (!reader->ReadHeader(err))
     return NULL;
@@ -45,11 +45,6 @@ BoxReader* BoxReader::ReadTopLevelBox(const uint8_t* buf,
   if (reader->type() == FOURCC_mdat)
     return reader.release();
 
-  if (!IsValidTopLevelBox(reader->type())) {
-    *err = true;
-    return NULL;
-  }
-
   if (reader->size() <= buf_size)
     return reader.release();
 
@@ -57,50 +52,19 @@ BoxReader* BoxReader::ReadTopLevelBox(const uint8_t* buf,
 }
 
 // static
-bool BoxReader::StartTopLevelBox(const uint8_t* buf,
+bool BoxReader::StartBox(const uint8_t* buf,
-                                 const size_t buf_size,
+                         const size_t buf_size,
-                                 FourCC* type,
+                         FourCC* type,
-                                 uint64_t* box_size,
+                         uint64_t* box_size,
-                                 bool* err) {
+                         bool* err) {
   BoxReader reader(buf, buf_size);
   if (!reader.ReadHeader(err))
     return false;
-  if (!IsValidTopLevelBox(reader.type())) {
-    *err = true;
-    return false;
-  }
   *type = reader.type();
   *box_size = reader.size();
   return true;
 }
 
-// static
-bool BoxReader::IsValidTopLevelBox(const FourCC& type) {
-  switch (type) {
-    case FOURCC_ftyp:
-    case FOURCC_pdin:
-    case FOURCC_bloc:
-    case FOURCC_moov:
-    case FOURCC_moof:
-    case FOURCC_mfra:
-    case FOURCC_mdat:
-    case FOURCC_free:
-    case FOURCC_skip:
-    case FOURCC_meta:
-    case FOURCC_meco:
-    case FOURCC_styp:
-    case FOURCC_sidx:
-    case FOURCC_ssix:
-    case FOURCC_prft:
-    case FOURCC_uuid:
-      return true;
-    default:
-      // Hex is used to show nonprintable characters and aid in debugging
-      LOG(ERROR) << "Unrecognized top-level box type 0x" << std::hex << type;
-      return false;
-  }
-}
-
 bool BoxReader::ScanChildren() {
   DCHECK(!scanned_);
   scanned_ = true;

packager/media/formats/mp4/box_reader.h

@@ -35,9 +35,9 @@ class BoxReader : public BufferReader {
   ///         function may return NULL if an intact, complete box is not
   ///         available in the buffer. For MDAT box only, a BoxReader object is
   ///         returned as long as the box header is available.
-  static BoxReader* ReadTopLevelBox(const uint8_t* buf,
+  static BoxReader* ReadBox(const uint8_t* buf,
-                                    const size_t buf_size,
+                            const size_t buf_size,
-                                    bool* err);
+                            bool* err);
 
   /// Read the box header from the current buffer.
   /// @param buf is not retained.
@@ -48,17 +48,11 @@ class BoxReader : public BufferReader {
   ///             reading the box.
   /// @return true if there is enough data to read the header and the header is
   ///         sane, which does not imply that the entire box is in the buffer.
-  static bool StartTopLevelBox(const uint8_t* buf,
+  static bool StartBox(const uint8_t* buf,
-                               const size_t buf_size,
+                       const size_t buf_size,
-                               FourCC* type,
+                       FourCC* type,
-                               uint64_t* box_size,
+                       uint64_t* box_size,
-                               bool* err) WARN_UNUSED_RESULT;
+                       bool* err) WARN_UNUSED_RESULT;
-
-  /// @return true if @a type is recognized to be the fourcc of a top-level box,
-  ///         false otherwise. This returns true for some boxes which we do not
-  ///         parse.
-  /// This method is helpful for debugging misaligned appends.
-  static bool IsValidTopLevelBox(const FourCC& type);
 
   /// Scan through all boxes within the current box, starting at the current
   /// buffer position. Must be called before any of the @b *Child functions

packager/media/formats/mp4/box_reader_unittest.cc

@@ -93,8 +93,7 @@ class BoxReaderTest : public testing::Test {
 TEST_F(BoxReaderTest, ExpectedOperationTest) {
   std::vector<uint8_t> buf = GetBuf();
   bool err;
-  scoped_ptr<BoxReader> reader(
+  scoped_ptr<BoxReader> reader(BoxReader::ReadBox(&buf[0], buf.size(), &err));
-      BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
   EXPECT_FALSE(err);
   EXPECT_TRUE(reader.get());
 
@@ -121,8 +120,7 @@ TEST_F(BoxReaderTest, OuterTooShortTest) {
   bool err;
 
   // Create a soft failure by truncating the outer box.
-  scoped_ptr<BoxReader> r(
+  scoped_ptr<BoxReader> r(BoxReader::ReadBox(&buf[0], buf.size() - 2, &err));
-      BoxReader::ReadTopLevelBox(&buf[0], buf.size() - 2, &err));
 
   EXPECT_FALSE(err);
   EXPECT_FALSE(r.get());
@@ -134,30 +132,16 @@ TEST_F(BoxReaderTest, InnerTooLongTest) {
 
   // Make an inner box too big for its outer box.
   buf[25] = 1;
-  scoped_ptr<BoxReader> reader(
+  scoped_ptr<BoxReader> reader(BoxReader::ReadBox(&buf[0], buf.size(), &err));
-      BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
 
   SkipBox box;
   EXPECT_FALSE(box.Parse(reader.get()));
 }
 
-TEST_F(BoxReaderTest, WrongFourCCTest) {
-  std::vector<uint8_t> buf = GetBuf();
-  bool err;
-
-  // Set an unrecognized top-level FourCC.
-  buf[5] = 1;
-  scoped_ptr<BoxReader> reader(
-      BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
-  EXPECT_FALSE(reader.get());
-  EXPECT_TRUE(err);
-}
-
 TEST_F(BoxReaderTest, ScanChildrenTest) {
   std::vector<uint8_t> buf = GetBuf();
   bool err;
-  scoped_ptr<BoxReader> reader(
+  scoped_ptr<BoxReader> reader(BoxReader::ReadBox(&buf[0], buf.size(), &err));
-      BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
 
   EXPECT_TRUE(reader->SkipBytes(16) && reader->ScanChildren());
 
@@ -180,8 +164,7 @@ TEST_F(BoxReaderTest, ReadAllChildrenTest) {
   // Modify buffer to exclude its last 'free' box.
   buf[3] = 0x38;
   bool err;
-  scoped_ptr<BoxReader> reader(
+  scoped_ptr<BoxReader> reader(BoxReader::ReadBox(&buf[0], buf.size(), &err));
-      BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
 
   std::vector<PsshBox> kids;
   EXPECT_TRUE(reader->SkipBytes(16) && reader->ReadAllChildren(&kids));
@@ -190,21 +173,16 @@ TEST_F(BoxReaderTest, ReadAllChildrenTest) {
 }
 
 TEST_F(BoxReaderTest, SkippingBloc) {
-  static const uint8_t kData[] = {0x00,
+  static const uint8_t kData[] = {
-                                  0x00,
+      0x00, 0x00, 0x00, 0x09,  // Box size.
-                                  0x00,
+      'b',  'l',  'o',  'c',   // FourCC.
-                                  0x09,  // Box size.
+      0x00,                    // Reserved byte.
-                                  'b',
+  };
-                                  'l',
-                                  'o',
-                                  'c',    // FourCC.
-                                  0x00};  // Reserved byte.
 
   std::vector<uint8_t> buf(kData, kData + sizeof(kData));
 
   bool err;
-  scoped_ptr<BoxReader> reader(
+  scoped_ptr<BoxReader> reader(BoxReader::ReadBox(&buf[0], buf.size(), &err));
-      BoxReader::ReadTopLevelBox(&buf[0], buf.size(), &err));
 
   EXPECT_FALSE(err);
   EXPECT_TRUE(reader);

packager/media/formats/mp4/mp4_media_parser.cc

@@ -192,10 +192,9 @@ bool MP4MediaParser::LoadMoov(const std::string& file_path) {
     uint64_t box_size;
     FourCC box_type;
     bool err;
-    if (!BoxReader::StartTopLevelBox(&buffer[0], kBoxHeaderReadSize, &box_type,
+    if (!BoxReader::StartBox(&buffer[0], kBoxHeaderReadSize, &box_type,
-                                     &box_size, &err)) {
+                             &box_size, &err)) {
-      LOG(ERROR) << "Could not start top level box from file '" << file_path
+      LOG(ERROR) << "Could not start box from file '" << file_path << "'";
-                 << "'";
       return false;
     }
     if (box_type == FOURCC_mdat) {
@@ -245,7 +244,7 @@ bool MP4MediaParser::ParseBox(bool* err) {
   if (!size)
     return false;
 
-  scoped_ptr<BoxReader> reader(BoxReader::ReadTopLevelBox(buf, size, err));
+  scoped_ptr<BoxReader> reader(BoxReader::ReadBox(buf, size, err));
   if (reader.get() == NULL)
     return false;
 
@@ -749,7 +748,7 @@ bool MP4MediaParser::ReadAndDiscardMDATsUntil(const int64_t offset) {
 
     FourCC type;
     uint64_t box_sz;
-    if (!BoxReader::StartTopLevelBox(buf, size, &type, &box_sz, &err))
+    if (!BoxReader::StartBox(buf, size, &type, &box_sz, &err))
       break;
 
     mdat_tail_ += box_sz;