MSE: Increase log visibility when unfragmented MP4 causes parse failure

media/formats/mp4/box_definitions.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/formats/mp4/box_definitions.h"
 
 #include "base/logging.h"
 #include "media/formats/mp4/es_descriptor.h"
 #include "media/formats/mp4/rcheck.h"
 
@@ skipping 639 matching lines @@
   return reader->ScanChildren() &&
          reader->MaybeReadChild(&header) &&
          reader->ReadChildren(&tracks);
 }
 
 Movie::Movie() : fragmented(false) {}
 Movie::~Movie() {}
 FourCC Movie::BoxType() const { return FOURCC_MOOV; }
 
 bool Movie::Parse(BoxReader* reader) {
-  return reader->ScanChildren() &&
-         reader->ReadChild(&header) &&
-         reader->ReadChildren(&tracks) &&
-         // Media Source specific: 'mvex' required
-         reader->ReadChild(&extends) &&
-         reader->MaybeReadChildren(&pssh);
+  RCHECK(reader->ScanChildren() && reader->ReadChild(&header) &&
+         reader->ReadChildren(&tracks));
+
+  RCHECK_MEDIA_LOGGED(reader->ReadChild(&extends), reader->log_cb(),
+                      "Detected unfragmented MP4. Media Source Extensions "

[chcunningham, 2015/05/15 17:19:33]

Do we know that all scenarios of returning false for ReadChild are due to
missing mvex? It seems like we might give this error message for other
categories of parsing errors. I looked for a place in Reader where we check
specifically for the mvex, but I did not find one. Is there such a place? Can we
put the error there instead?

[wolenetz, 2015/05/15 18:16:25]

ReadChild() inspects the passed parameter's overridden Box::BoxType() (which is
an mvex
(https://code.google.com/p/chromium/codesearch#chromium/src/media/formats/mp4/...)
in the case of passing &extends here) to find what kind of child it is expected
to find and read. If it fails to find or read that child, it returns false. This
is documented in ReadChild()'s declaration
(https://code.google.com/p/chromium/codesearch#chromium/src/media/formats/mp4/...).

This change doesn't change the behavior that a MVEX is required to be read here.
It just introduces a log if (finding or) reading that box fails.

+                      "requires ISO BMFF moov to contain mvex to indicate that "
+                      "Movie Fragments are to be expected.");
+
+  return reader->MaybeReadChildren(&pssh);
 }
 
 TrackFragmentDecodeTime::TrackFragmentDecodeTime() : decode_time(0) {}
 TrackFragmentDecodeTime::~TrackFragmentDecodeTime() {}
 FourCC TrackFragmentDecodeTime::BoxType() const { return FOURCC_TFDT; }
 
 bool TrackFragmentDecodeTime::Parse(BoxReader* reader) {
   RCHECK(reader->ReadFullBoxHeader());
   if (reader->version() == 1)
     return reader->Read8(&decode_time);
@@ skipping 271 matching lines @@
 SampleDependsOn IndependentAndDisposableSamples::sample_depends_on(
     size_t i) const {
   if (i >= sample_depends_on_.size())
     return kSampleDependsOnUnknown;
 
   return sample_depends_on_[i];
 }
 
 }  // namespace mp4
 }  // namespace media