Skip to content

Commit

Permalink
[Fuchsia] Add support for disabling software-only video codecs.
Browse files Browse the repository at this point in the history
If the HARDWARE_VIDEO_DECODER_ONLY Context flag is set, then only
the video codecs which are decodable in hardware are reported as
playable.

* Adds "enable_software_video_decoder" GN flag for Cast Runner.
* Defines new command line switch --disable-software-video-decoders
  for propagating the flag across ContextProvider/Browser and
  Browser/Renderer process boundaries.
* Bypasses software codec factory registration if the switch is set.
* Uses hardcoded temporary logic for defining which codecs are hardware
  accelerated; will be replaced with a check against the Fuchsia
  media codec service later.
* Adds new browser tests to verify the effect of
  --disable-software-video-decoders.

Bug: 1000858
Change-Id: I780bd38eacf7537bfb1ceb52db2bad5a4ed8a879
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1786079
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Auto-Submit: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695253}
  • Loading branch information
Kevin Marshall authored and Commit Bot committed Sep 10, 2019
1 parent d3864a1 commit 4732c7a
Show file tree
Hide file tree
Showing 16 changed files with 238 additions and 6 deletions.
12 changes: 10 additions & 2 deletions fuchsia/engine/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ repack("web_engine_pak") {
component("web_engine_core") {
deps = [
":mojom",
":switches",
":web_engine_pak",
"//base",
"//base:base_static",
Expand Down Expand Up @@ -152,8 +153,6 @@ component("web_engine_core") {
"renderer/on_load_script_injector.h",
"renderer/web_engine_content_renderer_client.cc",
"renderer/web_engine_content_renderer_client.h",
"switches.cc",
"switches.h",
"web_engine_content_client.cc",
"web_engine_content_client.h",
"web_engine_export.h",
Expand All @@ -166,6 +165,13 @@ component("web_engine_core") {
]
}

source_set("switches") {
sources = [
"switches.cc",
"switches.h",
]
}

executable("web_engine_exe") {
deps = [
":web_engine_core",
Expand Down Expand Up @@ -221,13 +227,15 @@ test("web_engine_browsertests") {
"browser/content_directory_browsertest.cc",
"browser/context_impl_browsertest.cc",
"browser/frame_impl_browsertest.cc",
"browser/media_browsertest.cc",
]
defines = [ "HAS_OUT_OF_PROC_TEST_RUNNER" ]
data = [
"test/data",
]
deps = [
":browsertest_core",
":switches",
":web_engine_core",
"//base/test:test_support",
"//content/public/browser",
Expand Down
125 changes: 125 additions & 0 deletions fuchsia/engine/browser/media_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// Copyright 2019 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 "fuchsia/engine/test/web_engine_browser_test.h"

#include "base/files/file_path.h"
#include "base/test/bind_test_util.h"
#include "base/test/test_timeouts.h"
#include "fuchsia/base/frame_test_util.h"
#include "fuchsia/base/test_navigation_listener.h"
#include "fuchsia/engine/common.h"
#include "fuchsia/engine/switches.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace {
const char kTestServerRoot[] = FILE_PATH_LITERAL("fuchsia/engine/test/data");

class MediaTest : public cr_fuchsia::WebEngineBrowserTest {
public:
MediaTest()
: run_timeout_(TestTimeouts::action_timeout(),
base::MakeExpectedNotRunClosure(FROM_HERE)) {
set_test_server_root(base::FilePath(kTestServerRoot));
}
~MediaTest() override = default;

void SetUpOnMainThread() override {
CHECK(embedded_test_server()->Start());
cr_fuchsia::WebEngineBrowserTest::SetUpOnMainThread();
}

protected:
// Creates a Frame with |navigation_listener_| attached.
fuchsia::web::FramePtr CreateFrame() {
return WebEngineBrowserTest::CreateFrame(&navigation_listener_);
}

cr_fuchsia::TestNavigationListener navigation_listener_;

private:
const base::RunLoop::ScopedRunTimeoutForTest run_timeout_;

DISALLOW_COPY_AND_ASSIGN(MediaTest);
};

// VP8 can presently only be decoded in software.
// Verify that the --disable-software-video-decoders flag results in VP8
// media being reported as unplayable.
class SoftwareDecoderDisabledTest : public MediaTest {
public:
SoftwareDecoderDisabledTest() = default;
~SoftwareDecoderDisabledTest() override = default;

void SetUp() override {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kDisableSoftwareVideoDecoders);
cr_fuchsia::WebEngineBrowserTest::SetUp();
}

private:
DISALLOW_COPY_AND_ASSIGN(SoftwareDecoderDisabledTest);
};

IN_PROC_BROWSER_TEST_F(SoftwareDecoderDisabledTest, VP8IsTypeSupported) {
const GURL kUrl(embedded_test_server()->GetURL("/can_play_vp8.html"));

fuchsia::web::FramePtr frame = CreateFrame();

fuchsia::web::NavigationControllerPtr controller;
frame->GetNavigationController(controller.NewRequest());

EXPECT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
controller.get(), fuchsia::web::LoadUrlParams(), kUrl.spec()));
navigation_listener_.RunUntilUrlAndTitleEquals(kUrl, "can play vp8: false");
}

using SoftwareDecoderEnabledTest = MediaTest;

// Verify that VP8 is reported as playable if --disable-software-video-decoders
// is unset.
IN_PROC_BROWSER_TEST_F(SoftwareDecoderEnabledTest, VP8IsTypeSupported) {
const GURL kUrl(embedded_test_server()->GetURL("/can_play_vp8.html"));

fuchsia::web::FramePtr frame = CreateFrame();

fuchsia::web::NavigationControllerPtr controller;
frame->GetNavigationController(controller.NewRequest());

EXPECT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
controller.get(), fuchsia::web::LoadUrlParams(), kUrl.spec()));
navigation_listener_.RunUntilUrlAndTitleEquals(kUrl, "can play vp8: true");
}

// Verify that a VP8 video is loaded if --disable-software-video-decoders is
// unset.
IN_PROC_BROWSER_TEST_F(SoftwareDecoderEnabledTest, PlayVP8) {
const GURL kUrl(embedded_test_server()->GetURL("/play_vp8.html"));

fuchsia::web::FramePtr frame = CreateFrame();

fuchsia::web::NavigationControllerPtr controller;
frame->GetNavigationController(controller.NewRequest());

EXPECT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
controller.get(), fuchsia::web::LoadUrlParams(), kUrl.spec()));
navigation_listener_.RunUntilUrlAndTitleEquals(kUrl, "loaded");
}

// Verifies that VP8 videos won't play if --disable-software-video-decoders is
// set.
IN_PROC_BROWSER_TEST_F(SoftwareDecoderDisabledTest, PlayVP8Disabled) {
const GURL kUrl(embedded_test_server()->GetURL("/play_vp8.html"));

fuchsia::web::FramePtr frame = CreateFrame();

fuchsia::web::NavigationControllerPtr controller;
frame->GetNavigationController(controller.NewRequest());

EXPECT_TRUE(cr_fuchsia::LoadUrlAndExpectResponse(
controller.get(), fuchsia::web::LoadUrlParams(), kUrl.spec()));
navigation_listener_.RunUntilUrlAndTitleEquals(kUrl, "error");
}

} // namespace
6 changes: 6 additions & 0 deletions fuchsia/engine/browser/web_engine_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "fuchsia/engine/browser/web_engine_browser_main_parts.h"
#include "fuchsia/engine/browser/web_engine_devtools_manager_delegate.h"
#include "fuchsia/engine/common.h"
#include "fuchsia/engine/switches.h"

WebEngineContentBrowserClient::WebEngineContentBrowserClient(
fidl::InterfaceRequest<fuchsia::web::Context> request)
Expand Down Expand Up @@ -97,4 +98,9 @@ void WebEngineContentBrowserClient::AppendExtraCommandLineSwitches(
int child_process_id) {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(kContentDirectories))
command_line->AppendSwitch(kContentDirectories);

if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableSoftwareVideoDecoders)) {
command_line->AppendSwitch(switches::kDisableSoftwareVideoDecoders);
}
}
18 changes: 18 additions & 0 deletions fuchsia/engine/context_provider_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,24 @@ void ContextProviderImpl::Create(
key_system);
}

bool disable_software_video_decoder =
(features &
fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER_ONLY) ==
fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER_ONLY;
bool enable_hardware_video_decoder =
(features & fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER) ==
fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER;
if (disable_software_video_decoder) {
if (!enable_hardware_video_decoder) {
LOG(ERROR) << "Software video decoding may only be disabled if hardware "
"video decoding is enabled.";
context_request.Close(ZX_ERR_INVALID_ARGS);
return;
}

launch_command.AppendSwitch(switches::kDisableSoftwareVideoDecoders);
}

// Validate embedder-supplied product, and optional version, and pass it to
// the Context to include in the UserAgent.
if (params.has_user_agent_product()) {
Expand Down
22 changes: 22 additions & 0 deletions fuchsia/engine/renderer/web_engine_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
#include "components/cdm/renderer/widevine_key_system_properties.h"
#include "content/public/renderer/render_frame.h"
#include "fuchsia/engine/renderer/on_load_script_injector.h"
#include "fuchsia/engine/switches.h"
#include "media/base/eme_constants.h"
#include "media/base/video_codecs.h"
#include "services/service_manager/public/cpp/binder_registry.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_registry.h"
#include "third_party/widevine/cdm/widevine_cdm_common.h"
Expand Down Expand Up @@ -51,3 +53,23 @@ void WebEngineContentRendererClient::AddSupportedKeySystems(
media::EmeFeatureSupport::ALWAYS_ENABLED, // persistent state
media::EmeFeatureSupport::ALWAYS_ENABLED)); // distinctive identifier
}

bool WebEngineContentRendererClient::IsSupportedVideoType(
const media::VideoType& type) {
// Fall back to default codec querying logic if software codecs aren't
// disabled.
if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableSoftwareVideoDecoders)) {
return ContentRendererClient::IsSupportedVideoType(type);
}

// TODO(fxb/36000): Replace these hardcoded checks with a query to the
// fuchsia.mediacodec FIDL service.
if (type.codec == media::kCodecH264 && type.level <= 41)
return true;

if (type.codec == media::kCodecVP9 && type.level <= 40)
return true;

return false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class WebEngineContentRendererClient : public content::ContentRendererClient {
void AddSupportedKeySystems(
std::vector<std::unique_ptr<media::KeySystemProperties>>* key_systems)
override;
bool IsSupportedVideoType(const media::VideoType& type) override;

private:
DISALLOW_COPY_AND_ASSIGN(WebEngineContentRendererClient);
Expand Down
1 change: 1 addition & 0 deletions fuchsia/engine/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@ namespace switches {

const char kEnableWidevine[] = "enable-widevine";
const char kPlayreadyKeySystem[] = "playready-key-system";
const char kDisableSoftwareVideoDecoders[] = "disable-software-video-decoders";

} // namespace switches
6 changes: 5 additions & 1 deletion fuchsia/engine/switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ extern const char kEnableWidevine[];
// Enables PlayReady CDM and specifies the corresponding key system string.
extern const char kPlayreadyKeySystem[];

// Prevents the use of video codecs that are not hardware-accelerated.
extern const char
kDisableSoftwareVideoDecoders[]; // "disable-software-video-decoders"

} // namespace switches

#endif // FUCHSIA_ENGINE_SWITCHES_H_
#endif // FUCHSIA_ENGINE_SWITCHES_H_
Binary file added fuchsia/engine/test/data/bear-vp8a.webm
Binary file not shown.
9 changes: 9 additions & 0 deletions fuchsia/engine/test/data/can_play_vp8.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<html>
<head></head>
<body>
<script>
document.title = 'can play vp8: ' +
MediaSource.isTypeSupported('video/webm;codecs="vp8"');
</script>
</body>
</html>
10 changes: 10 additions & 0 deletions fuchsia/engine/test/data/play_vp8.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<html>
<body>
<script>
var bear = document.createElement('video');
bear.onerror = function() { document.title = 'error'; }
bear.onloadeddata = function() { document.title = 'loaded'; }
bear.src = 'bear-vp8a.webm';
</script>
</body>
</html>
11 changes: 10 additions & 1 deletion fuchsia/runners/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,19 @@ declare_args() {
# Enable remote debugging for WebContentRunner instances on this port.
# Set to 0 to disable.
enable_remote_debugging_on_port = 0

# Allow the Cast Runner to use software decoders for rendering video.
# TODO(crbug.com/1000858): Revisit this value once hardware playback is fixed
# (fxb/13659).
enable_software_video_decoders = true
}

buildflag_header("buildflags") {
header = "buildflags.h"
flags = [ "ENABLE_REMOTE_DEBUGGING_ON_PORT=$enable_remote_debugging_on_port" ]
flags = [
"ENABLE_REMOTE_DEBUGGING_ON_PORT=$enable_remote_debugging_on_port",
"ENABLE_SOFTWARE_VIDEO_DECODERS=$enable_software_video_decoders",
]
visibility = [ ":*" ]
}

Expand Down Expand Up @@ -89,6 +97,7 @@ executable("cast_runner_exe") {
"cast/main.cc",
]
deps = [
":buildflags",
":cast_runner_core",
":common",
"//base",
Expand Down
10 changes: 8 additions & 2 deletions fuchsia/runners/cast/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,31 @@

#include <lib/sys/cpp/component_context.h>

#include "base/command_line.h"
#include "base/fuchsia/default_context.h"
#include "base/message_loop/message_pump_type.h"
#include "base/run_loop.h"
#include "base/task/single_thread_task_executor.h"
#include "build/buildflag.h"
#include "fuchsia/runners/buildflags.h"
#include "fuchsia/runners/cast/cast_runner.h"

int main(int argc, char** argv) {
base::SingleThreadTaskExecutor io_task_executor(base::MessagePumpType::IO);
base::RunLoop run_loop;

constexpr fuchsia::web::ContextFeatureFlags kCastRunnerFeatures =
fuchsia::web::ContextFeatureFlags features =
fuchsia::web::ContextFeatureFlags::NETWORK |
fuchsia::web::ContextFeatureFlags::AUDIO |
fuchsia::web::ContextFeatureFlags::VULKAN |
fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER;

if (!BUILDFLAG(ENABLE_SOFTWARE_VIDEO_DECODERS))
features |= fuchsia::web::ContextFeatureFlags::HARDWARE_VIDEO_DECODER_ONLY;

CastRunner runner(
base::fuchsia::ComponentContextForCurrentProcess()->outgoing().get(),
WebContentRunner::CreateIncognitoWebContext(kCastRunnerFeatures));
WebContentRunner::CreateIncognitoWebContext(features));

base::fuchsia::ComponentContextForCurrentProcess()
->outgoing()
Expand Down
4 changes: 4 additions & 0 deletions media/renderers/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ source_set("renderers") {
"//ui/gl",
]

if (is_fuchsia) {
deps += [ "//fuchsia/engine:switches" ]
}

configs += [
"//media:subcomponent_config",

Expand Down
1 change: 1 addition & 0 deletions media/renderers/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
include_rules = [
"+components/viz/client",
"+components/viz/common",
"+fuchsia/engine/switches.h",
"+third_party/khronos/GLES2",
"+ul/gl/gl_enums.h",
]
Expand Down
Loading

0 comments on commit 4732c7a

Please sign in to comment.