Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GStreamer default media backend #30420

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ports/libsimpleservo/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ vergen = { version = "8.0.0", features = ["git", "gitcl"] }

[features]
debugmozjs = ["libservo/debugmozjs"]
default = ["max_log_level", "native-bluetooth", "webdriver"]
default = ["max_log_level", "native-bluetooth", "webdriver", "media-gstreamer"]
googlevr = ["libservo/googlevr"]
jitspew = ["libservo/jitspew"]
js_backtrace = ["libservo/js_backtrace"]
Expand Down
2 changes: 1 addition & 1 deletion ports/libsimpleservo/capi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ cbindgen = "0.26"

[features]
debugmozjs = ["simpleservo/debugmozjs"]
default = ["webdriver", "max_log_level"]
default = ["webdriver", "max_log_level", "media-gstreamer"]
googlevr = ["simpleservo/googlevr"]
jitspew = ["simpleservo/jitspew"]
js_backtrace = ["simpleservo/js_backtrace"]
Expand Down
2 changes: 1 addition & 1 deletion ports/libsimpleservo/jniapi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ cc = "1.0"

[features]
debugmozjs = ["simpleservo/debugmozjs"]
default = ["max_log_level", "native-bluetooth", "webdriver"]
default = ["max_log_level", "native-bluetooth", "webdriver", "media-gstreamer"]
googlevr = ["simpleservo/googlevr"]
js_backtrace = ["simpleservo/js_backtrace"]
max_log_level = ["simpleservo/max_log_level"]
Expand Down
2 changes: 1 addition & 1 deletion ports/servoshell/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ ProductName = "Servo"

[features]
debugmozjs = ["libservo/debugmozjs"]
default = ["max_log_level", "native-bluetooth", "webdriver"]
default = ["max_log_level", "native-bluetooth", "webdriver", "media-gstreamer"]
jitspew = ["libservo/jitspew"]
js_backtrace = ["libservo/js_backtrace"]
max_log_level = ["log/release_max_level_info"]
Expand Down
5 changes: 2 additions & 3 deletions python/servo/build_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ class MachCommands(CommandBase):
def build(self, build_type: BuildType, jobs=None, params=None, no_package=False,
verbose=False, very_verbose=False, libsimpleservo=False, **kwargs):
opts = params or []
has_media_stack = "media-gstreamer" in self.features

if build_type == BuildType.RELEASE:
opts += ["--release"]
Expand Down Expand Up @@ -171,7 +170,7 @@ def package_generated_shared_libraries(libs, build_path, servo_exe_dir):
status = 1

# copy needed gstreamer DLLs in to servo.exe dir
if has_media_stack:
if self.media_stack.is_gstreamer():
print("Packaging gstreamer DLLs")
if not package_gstreamer_dlls(env, servo_exe_dir, target_triple):
status = 1
Expand All @@ -187,7 +186,7 @@ def package_generated_shared_libraries(libs, build_path, servo_exe_dir):
servo_bin_dir = os.path.dirname(servo_path)
assert os.path.exists(servo_bin_dir)

if has_media_stack:
if self.media_stack.is_gstreamer():
print("Packaging gstreamer dylibs")
if not package_gstreamer_dylibs(self.cross_compile_target, servo_path):
return 1
Expand Down
56 changes: 42 additions & 14 deletions python/servo/command_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,28 @@ class BuildType(Enum):
RELEASE = 2


class MediaStack(Enum):
""" Media stack """
DUMMY = 1
GSTREAMER = 2

def is_dummy(self) -> bool:
return self == MediaStack.DUMMY

def is_gstreamer(self) -> bool:
return self == MediaStack.GSTREAMER

@staticmethod
def from_str(s: str):
s = s.lower()
if s == "gstreamer":
return MediaStack.GSTREAMER
elif s == "dummy":
return MediaStack.DUMMY
else:
raise RuntimeError(f"Got unknown media stack {s}")


@contextlib.contextmanager
def cd(new_path):
"""Context manager for changing the current working directory"""
Expand Down Expand Up @@ -211,6 +233,7 @@ class CommandBase(object):
def __init__(self, context):
self.context = context
self.features = []
self.media_stack: Optional[MediaStack] = None
self.cross_compile_target = None
self.is_android_build = False
self.target_path = util.get_target_dir()
Expand Down Expand Up @@ -832,20 +855,21 @@ def configure_media_stack(self, media_stack: Optional[str]):
"""Determine what media stack to use based on the value of the build target
platform and the value of the '--media-stack' command-line argument.
The chosen media stack is written into the `features` instance variable."""
if not media_stack:
if self.config["build"]["media-stack"] != "auto":
media_stack = self.config["build"]["media-stack"]
assert media_stack
elif (
not self.cross_compile_target
or ("armv7" in self.cross_compile_target and self.is_android_build)
or "x86_64" in self.cross_compile_target
):
media_stack = "gstreamer"
else:
media_stack = "dummy"
if media_stack != "dummy":
self.features += ["media-" + media_stack]

if media_stack: # In this case, the media stack is already configured.
self.media_stack = MediaStack.from_str(media_stack.lower())
return

if self.config["build"]["media-stack"] != "auto":
self.media_stack = MediaStack.from_str(self.config["build"]["media-stack"].lower())
elif (
not self.cross_compile_target
or ("armv7" in self.cross_compile_target and self.is_android_build)
or "x86_64" in self.cross_compile_target
):
self.media_stack = MediaStack.GSTREAMER
else:
self.media_stack = MediaStack.DUMMY

def run_cargo_build_like_command(
self, command: str, cargo_args: List[str],
Expand Down Expand Up @@ -888,6 +912,10 @@ def run_cargo_build_like_command(
features.append("webgl-backtrace")
if self.config["build"]["dom-backtrace"]:
features.append("dom-backtrace")

if self.media_stack.is_dummy() and "--no-default-features" not in cargo_args:
args += ["--no-default-features"]
Comment on lines +915 to +917
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm. There is an issue here and it's that using the dummy media stack disables the "max_log_level", "native-bluetooth", and "webdriver" features. Perhaps we should make these features non-configurable? I'm not sure what the right thing to do here is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's why I wanted ports so I could detect which features to enable. Webdriver should probably be hidden behind pref.

We could also move gstreamer enabling into cargo per target, then mach would only need to handle cross and forced dummy cases.
inspiration: https://stackoverflow.com/questions/57972355/enable-a-cargo-feature-by-default-when-the-target-arch-is-wasm


args += ["--features", " ".join(features)]

if with_debug_assertions or self.config["build"]["debug-assertions"]:
Expand Down