Implemented HEVC video demuxing and parsing

content/browser/media/media_canplaytype_browsertest.cc

Index: content/browser/media/media_canplaytype_browsertest.cc
diff --git a/content/browser/media/media_canplaytype_browsertest.cc b/content/browser/media/media_canplaytype_browsertest.cc
index b44a3a1f752a71eb1916f6e7d20322aa81c55a92..724e0ba726b9e8500ea8605c0133eb27dc41ca2c 100644
--- a/content/browser/media/media_canplaytype_browsertest.cc
+++ b/content/browser/media/media_canplaytype_browsertest.cc
@@ -173,6 +173,19 @@ class MediaCanPlayTypeTest : public MediaBrowserTest {
     EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"mp4ax\"'"));
 
     EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"unknown\"'"));
+
+    // Don't allow incomplete/ambiguous codec ids for HEVC for now.

[ddorwin, 2015/03/25 01:21:59]

Remove " for now".

[servolk, 2015/03/25 02:20:20]

Done.

+    // Codec string must have info about codec level/profile, e.g. hvc1.1.L0.0
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hev1\"'"));
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hvc1\"'"));

[ddorwin, 2015/03/25 01:21:59]

Also test these with a trailing dot.
And valid complete codecs with a trailing dot.

[servolk, 2015/03/25 02:20:20]

Done.

+
+    // Invalid codecs that look like something similar to HEVC/H.265
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hev1x\"'"));
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hvc1x\"'"));
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hvc0.1.L0.0\"'"));
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hev0.1.L0.0\"'"));
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hvc2.1.L0.0\"'"));
+    EXPECT_EQ(kNot, CanPlay("'" + mime + "; codecs=\"hev2.1.L0.0\"'"));
   }
 
   void TestOGGUnacceptableCombinations(const std::string& mime) {
@@ -568,6 +581,19 @@ IN_PROC_BROWSER_TEST_F(MediaCanPlayTypeTest, CodecSupportTest_mp4) {
   EXPECT_EQ(kNot, CanPlay("'audio/x-m4a; codecs=\"avc3.64001F\"'"));
 
   TestMPEGUnacceptableCombinations("audio/x-m4a");
+
+  const char* kHevcSupported = kNot;
+#if defined(ENABLE_HEVC_DEMUXING)
+  kHevcSupported = kPropProbably;

[ddorwin, 2015/03/25 01:21:59]

Why not just #if/#else?

[servolk, 2015/03/25 02:20:20]

Something like:
#if defined(ENABLE_HEVC_DEMUXING)
  const char* kHevcSupported = kPropProbably;
#else
  const char* kHevcSupported = kPropNot;
#endif
?
That would be 5 lines instead of 4 and would have the same effect. So if we can
use just 4, why go for the longer one? I think this one is just as readable.

[ddorwin, 2015/03/25 18:02:58]

Because immediately changing the value looks weird. Also, it can't be used at
the file scope, which you may need.

[servolk, 2015/04/29 23:50:32]

Done.

[ddorwin, 2015/06/12 00:52:26]

It doesn't look like this was fully addressed. Added comments in PS38.

+#endif
+  // TODO(servolk): Add more unit test coverage once we have more info about

[ddorwin, 2015/03/25 01:21:59]

Detailed tests of the codec strings probably belong in an HEVC-specific test
like CodecSupportTest_Avc1Variants. So, you'd have:
CodecSupportTest_Hev1Variants
CodecSupportTest_Hvc1Variants

Something like CodecSupportTest_HevcLevels might also be appropriate.

[servolk, 2015/03/25 02:20:20]

Yes, but that's going to be done when we actually add handling of various
levels/profiles for HEVC.

[ddorwin, 2015/03/25 18:02:58]

I was just saying that the tests wouldn't be at the site of the TODO. You might
leave yourself more notes like those in my comments or just "in separate
test(s)."

[servolk, 2015/04/29 23:50:32]

I think this whole test could use some refactoring. There's a lot of duplication
here.

[ddorwin, 2015/06/12 00:52:26]

It is verbose, and that is intentional. We have a lot of test code to ensure we
do the right thing. Previously, we were not doing the right thing and didn't
know it. The logic that drives this is complex, so it's important to test
thoroughly.

It looks like you removed the TODO. I updated
https://code.google.com/p/chromium/issues/detail?id=482761 to capture this.

+  // various HEVC levels/profiles.
+  EXPECT_EQ(kHevcSupported, CanPlay("'video/mp4; codecs=\"hvc1.1.L0.0\"'"));

[ddorwin, 2015/03/25 01:21:59]

You are testing video/mp4 far away from the other video/mp4 tests. Also, you
should test in all four MP4 containers + two HLS variants like the other codecs.

[servolk, 2015/03/25 02:20:20]

Done (except HLS; HEVC is not supported via HLS at the moment)

[ddorwin, 2015/03/25 18:02:58]

Then you should have negative tests.

[servolk, 2015/04/29 23:50:32]

Done.

[ddorwin, 2015/06/12 00:52:26]

It doesn't look like this was done. Added comments in PS38.

+  EXPECT_EQ(kHevcSupported, CanPlay("'video/mp4; codecs=\"hev1.1.L0.0\"'"));
+  EXPECT_EQ(kHevcSupported,
+            CanPlay("'video/mp4; codecs=\"hvc1.1.L0.0, mp4a.40.5\"'"));
+  EXPECT_EQ(kHevcSupported,
+            CanPlay("'video/mp4; codecs=\"hev1.1.L0.0, mp4a.40.5\"'"));
 }
 
 // When modifying this test, also change CodecSupportTest_Avc3Variants.