MSE: Increase log visibility when unfragmented MP4 causes parse failure

media/formats/mp4/box_definitions.cc

Index: media/formats/mp4/box_definitions.cc
diff --git a/media/formats/mp4/box_definitions.cc b/media/formats/mp4/box_definitions.cc
index a08cb26d560ca1b5e99ce066a269017af5d90a9e..93c1dd20266676ab2abdb7db5edd8e777cf34a74 100644
--- a/media/formats/mp4/box_definitions.cc
+++ b/media/formats/mp4/box_definitions.cc
@@ -657,12 +657,15 @@ 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) {}