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

Conversation

sagudev
Copy link
Member

@sagudev sagudev commented Sep 25, 2023

Try run: https://github.com/sagudev/servo/actions/runs/6289052004


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because ___

@sagudev sagudev changed the title Makes gstreamer default media backend GStreamer default media backend Sep 25, 2023
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Great work. I really like the use of an enum to store the configuration value. Just a couple changes requested. We should also run this by the community to make sure the change is acceptable to everyone.

python/servo/command_base.py Outdated Show resolved Hide resolved
python/servo/command_base.py Outdated Show resolved Hide resolved
python/servo/command_base.py Outdated Show resolved Hide resolved
Comment on lines +915 to +917

if self.media_stack.is_dummy() and "--no-default-features" not in cargo_args:
args += ["--no-default-features"]
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants