MSE: Increase log visibility when unfragmented MP4 causes parse failure

media/formats/mp4/rcheck.h

Index: media/formats/mp4/rcheck.h
diff --git a/media/formats/mp4/rcheck.h b/media/formats/mp4/rcheck.h
index fb0f8f27d4ec66f965f18609bcbaacd50186c7bf..c1ff7c36e0341dcc2521efd1c1f0f96ec44e1a18 100644
--- a/media/formats/mp4/rcheck.h
+++ b/media/formats/mp4/rcheck.h
@@ -6,13 +6,25 @@
 #define MEDIA_FORMATS_MP4_RCHECK_H_
 
 #include "base/logging.h"
+#include "media/base/media_log.h"
 
-#define RCHECK(x) \
-    do { \
-      if (!(x)) { \
-        DLOG(ERROR) << "Failure while parsing MP4: " << #x; \
-        return false; \
-      } \
-    } while (0)
+#define RCHECK_MEDIA_LOGGED(condition, log_cb, msg)                 \
+  do {                                                              \
+    if (!(condition)) {                                             \
+      DLOG(ERROR) << "Failure while parsing MP4: " << #condition;   \

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

Noob c++ question. What is the value of #condition? Does this somehow stringify
the condition, so we get some output like

"Failure while parsing MP4: a() && b() || c()"

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

Yep. Exactly. See https://gcc.gnu.org/onlinedocs/cpp/Stringification.html
For example, see https://code.google.com/p/chromium/issues/detail?id=487410#c5
demonstrating the DLOG behavior prior to this change for a failed RCHECK.
[1:1:0514/131527:ERROR:box_reader.cc(197)] Failure while parsing MP4: itr !=
children_.end()
Also, I don't think stringified code fragments are appropriate for
chrome://media-internals, hence I've added the msg parameter. So we get DLOG
detail still in debug builds, plus a user-readable error message in
chrome://media-internals in all builds.

See also DCHECK that does similar stringification
(https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&l=644).

However, I'll remove the unneccessary << since the compiler can concat these
strings statically.

+      MEDIA_LOG(ERROR, log_cb) << "Failure parsing MP4: " << (msg); \
+      return false;                                                 \
+    }                                                               \
+  } while (0)
+
+// TODO(wolenetz,chcunningham): Eliminate usage of this macro in favor of
+// RCHECK_MEDIA_LOGGED. See https://crbug.com/487410.

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

Do we really want to Elminate? Basically, are you saying that *all* of the
dvlogs that would be emitted here should be replaced with the combined
media_log/dvlog from RCHECK_MEDIA_LOGGED? 

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

Yeah, my wording was too strong. I'll relax it. There are RCHECKs all over the
place, not all of which might have a usable log message. There are other places
(nearer to mp4_stream_parser) where RCHECKs aren't used, but log messages would
be good to add.

+#define RCHECK(condition)                                         \
+  do {                                                            \
+    if (!(condition)) {                                           \
+      DLOG(ERROR) << "Failure while parsing MP4: " << #condition; \
+      return false;                                               \
+    }                                                             \
+  } while (0)
 
 #endif  // MEDIA_FORMATS_MP4_RCHECK_H_