Android: enable H264&VP8 HW accelerator for MediaRecorder

content/browser/webrtc/webrtc_media_recorder_browsertest.cc

Index: content/browser/webrtc/webrtc_media_recorder_browsertest.cc
diff --git a/content/browser/webrtc/webrtc_media_recorder_browsertest.cc b/content/browser/webrtc/webrtc_media_recorder_browsertest.cc
index 7d945a9a38b9c9afa42bc7ad70a74a4017c0b505..45f96ef5ee39196cefe52d2dc88b52774f1a7364 100644
--- a/content/browser/webrtc/webrtc_media_recorder_browsertest.cc
+++ b/content/browser/webrtc/webrtc_media_recorder_browsertest.cc
@@ -53,12 +53,21 @@ class WebRtcMediaRecorderTest
         switches::kEnableBlinkFeatures, "GetUserMedia");
   }
 
-  void MaybeForceDisableEncodeAccelerator(bool disable) {
+  void MaybeForceDisableEncodeAccelerator(bool disable, std::string mime_type) {
     if (!disable)
       return;
+
+    bool disable_flag = true;
+#if defined(OS_ANDROID)
+    // There is no SW H264 on Android.

[emircan, 2017/04/07 17:33:04]

// OpenH264 encoder is not supported on Android.

[braveyao, 2017/04/08 00:56:26]

Done.

+    if (mime_type.find("AVC1"))
+      disable_flag = false;
+#endif
+
     // This flag is also used for encoding, https://crbug.com/616640.
-    base::CommandLine::ForCurrentProcess()->AppendSwitch(
-        switches::kDisableAcceleratedVideoDecode);
+    if (disable_flag)

[emircan, 2017/04/07 17:33:04]

How was this test running and passing on android before?

This method would result in running the same test case twice by ignoring the
|disable_accelerator| param. Why about removing l.25 "if !defined(OS_ANDROID)"
instead?

[braveyao, 2017/04/08 00:56:26]

Done.

+      base::CommandLine::ForCurrentProcess()->AppendSwitch(
+          switches::kDisableAcceleratedVideoDecode);
   }
 
  private:
@@ -74,14 +83,16 @@ IN_PROC_BROWSER_TEST_F(WebRtcMediaRecorderTest, StartAndStop) {
 }
 
 IN_PROC_BROWSER_TEST_P(WebRtcMediaRecorderTest, StartAndDataAvailable) {
-  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator);
+  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator,
+                                     GetParam().mime_type);
   MakeTypicalCall(base::StringPrintf("testStartAndDataAvailable(\"%s\");",
                                      GetParam().mime_type.c_str()),
                   kMediaRecorderHtmlFile);
 }
 
 IN_PROC_BROWSER_TEST_P(WebRtcMediaRecorderTest, StartWithTimeSlice) {
-  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator);
+  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator,
+                                     GetParam().mime_type);
   MakeTypicalCall(base::StringPrintf("testStartWithTimeSlice(\"%s\");",
                                      GetParam().mime_type.c_str()),
                   kMediaRecorderHtmlFile);
@@ -96,7 +107,8 @@ IN_PROC_BROWSER_TEST_F(WebRtcMediaRecorderTest, NoResumeWhenRecorderInactive) {
 }
 
 IN_PROC_BROWSER_TEST_P(WebRtcMediaRecorderTest, ResumeAndDataAvailable) {
-  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator);
+  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator,
+                                     GetParam().mime_type);
   MakeTypicalCall(base::StringPrintf("testResumeAndDataAvailable(\"%s\");",
                                      GetParam().mime_type.c_str()),
                   kMediaRecorderHtmlFile);
@@ -123,9 +135,17 @@ IN_PROC_BROWSER_TEST_F(WebRtcMediaRecorderTest, IllegalPauseThrowsDOMError) {
 IN_PROC_BROWSER_TEST_F(WebRtcMediaRecorderTest, TwoChannelAudioRecording) {
   MakeTypicalCall("testTwoChannelAudio();", kMediaRecorderHtmlFile);
 }
+#if defined(OS_ANDROID)
+// Basically HW codec is preferred on Android, which doesn't support alpha
+// channel. And this test doesn't work on some old platforms.

[emircan, 2017/04/07 17:33:04]

This test only inputs alpha channel video for recording. For Android VEA case,
alpha channel would be dropped and rest would be recorded, see the line below.
We know that only VPX software encoder returns true for CanEncodeAlphaChannel().
I don't see any reason why this tests should be disabled. 

https://cs.chromium.org/chromium/src/content/renderer/media_recorder/video_tr...

[braveyao, 2017/04/08 00:56:26]

Emmm you are right. The failure of this case on N5 is due to the request video
format. It seems that the minimum supported video format for H264 is different
on older platforms(e.g. same case works on N5X but fail on N5). Unfortunately I
didn't find any doc for it yet. I'll take deep look later, since this may affect
RTC call too and deserve another cl.

[braveyao, 2017/04/14 22:28:28]

Done.

+#define MAYBE_RecordWithTransparency DISABLED_RecordWithTransparency
+#else
+#define MAYBE_RecordWithTransparency RecordWithTransparency
+#endif
 
-IN_PROC_BROWSER_TEST_P(WebRtcMediaRecorderTest, RecordWithTransparency) {
-  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator);
+IN_PROC_BROWSER_TEST_P(WebRtcMediaRecorderTest, MAYBE_RecordWithTransparency) {
+  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator,
+                                     GetParam().mime_type);
   MakeTypicalCall(base::StringPrintf("testRecordWithTransparency(\"%s\");",
                                      GetParam().mime_type.c_str()),
                   kMediaRecorderHtmlFile);
@@ -168,7 +188,8 @@ IN_PROC_BROWSER_TEST_F(WebRtcMediaRecorderTest,
 #endif
 
 IN_PROC_BROWSER_TEST_P(WebRtcMediaRecorderTest, MAYBE_PeerConnection) {
-  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator);
+  MaybeForceDisableEncodeAccelerator(GetParam().disable_accelerator,
+                                     GetParam().mime_type);
   MakeTypicalCall(base::StringPrintf("testRecordRemotePeerConnection(\"%s\");",
                                      GetParam().mime_type.c_str()),
                   kMediaRecorderHtmlFile);