Skip to content

Commit

Permalink
Fix gn mojo_media arg overrides in //media/media_options.gni
Browse files Browse the repository at this point in the history
https://chromium-review.googlesource.com/959446 broke the ability
to reliably override mojo_media_services or mojo_media_host with
gn args. Fix by calculating the default values before declare_args
in helper variables, having args overrides apply, and asserting
the constraints on defaults only.

Fixes gn gen with args such as
  target_os="android"
  mojo_media_services=["cdm"]
which would fail with a "Replacing nonempty list" gn error.

Change-Id: Ia6aeaedfb4c40ba681e9c1500545ddd55cbdb0f1
Reviewed-on: https://chromium-review.googlesource.com/1092731
Commit-Queue: Tomasz Śniatowski <tsniatowski@vewd.com>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567888}
  • Loading branch information
ilor authored and Commit Bot committed Jun 16, 2018
1 parent b915340 commit e41a59d
Showing 1 changed file with 41 additions and 38 deletions.
79 changes: 41 additions & 38 deletions media/media_options.gni
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,45 @@ assert(
assert(!enable_cdm_host_verification || is_mac || is_win,
"CDM host verification is only supported on Mac and Windows.")

_default_mojo_media_services = []
_default_mojo_media_host = "none"

# Default mojo_media_services and mojo_media_host on various platforms.
# Can be overridden by gn build arguments from the --args command line flag
# for local testing.
if (enable_mojo_media) {
if (is_chromecast && is_cast_using_cma_backend) {
_default_mojo_media_services = [
"cdm",
"renderer",
]
_default_mojo_media_host = "browser"
} else if (is_android) {
# Both chrome for Android and cast for ATV belongs to this case
_default_mojo_media_services = [
"cdm",
"audio_decoder",
"video_decoder",
]
_default_mojo_media_host = "gpu"
} else if (is_mac || is_win) {
_default_mojo_media_services = [ "video_decoder" ]
_default_mojo_media_host = "gpu"
}

if (enable_library_cdms) {
_default_mojo_media_services += [ "cdm" ]

# Having a CDM running means it could require a CdmProxy running in the GPU
# process.
assert(
_default_mojo_media_host == "none" || _default_mojo_media_host == "gpu",
"For now, mojo_media_host should not overwrite it with a different " +
"value if it has been set.")
_default_mojo_media_host = "gpu"
}
}

# Use another declare_args() to pick up possible overrides of enable_mojo_media
# from --args command line flags. See "gn help declare_args".
declare_args() {
Expand All @@ -182,7 +221,7 @@ declare_args() {
# Renderer. Cannot be used with the mojo Renderer above.
# - "video_decoder": Use mojo-based video decoder in the default media
# Renderer. Cannot be used with the mojo Renderer above.
mojo_media_services = []
mojo_media_services = _default_mojo_media_services

# The process that the mojo MediaService runs in. By default, all services
# registered in |mojo_media_services| are hosted in the MediaService, with the
Expand All @@ -195,43 +234,7 @@ declare_args() {
# - "browser": Use mojo media service hosted in the browser process.
# - "gpu": Use mojo media service hosted in the gpu process.
# - "utility": Use mojo media service hosted in the utility process.
mojo_media_host = "none"
}

# Default mojo_media_services and mojo_media_host on various platforms.
# Can be overridden by gn build arguments from the --args command line flag
# for local testing.
if (enable_mojo_media) {
if (is_chromecast && is_cast_using_cma_backend) {
mojo_media_services = [
"cdm",
"renderer",
]
mojo_media_host = "browser"
} else if (is_android) {
# Both chrome for Android and cast for ATV belongs to this case
mojo_media_services = [
"cdm",
"audio_decoder",
"video_decoder",
]
mojo_media_host = "gpu"
} else if (is_mac || is_win) {
mojo_media_services += [ "video_decoder" ]
mojo_media_host = "gpu"
}

if (enable_library_cdms) {
mojo_media_services += [ "cdm" ]
assert(
mojo_media_host == "none" || mojo_media_host == "gpu",
"For now, mojo_media_host should not overwrite it with a different " +
"value if it has been set.")

# Having a CDM running means it could require a CdmProxy running in the GPU
# process.
mojo_media_host = "gpu"
}
mojo_media_host = _default_mojo_media_host
}

declare_args() {
Expand Down

0 comments on commit e41a59d

Please sign in to comment.