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

[RFC] options: convert some guarded options to new warning mechanism #14624

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Dudemanguy
Copy link
Member

@Dudemanguy Dudemanguy commented Aug 1, 2024

Here's my attempt at implementing what I suggested in #14618 (comment). The mechanism works fine. I implemented both per option and for sub options.

  1. I had to add a bunch of defines because of windows junk. It's not really a big deal but it's kind of ugly imo.
  2. Not really sure how we want to handle suboptions that are conditional and private somewhere. They could just be moved to options.h for example, but then that makes them not-private obviously. Not a blocker for the feature but something to think about.

It makes sense to only descend into the subdirectory meson build file on
windows, but we should always make sure HAVE_WIN32_SMTC is 0 for other
platforms. Otherwise, it gets unneccesarily awkward to work with.
Copy link

github-actions bot commented Aug 1, 2024

Download the artifacts for this pull request:

Windows
macOS

options/options.c Outdated Show resolved Hide resolved
options/options.c Outdated Show resolved Hide resolved
options/options.c Outdated Show resolved Hide resolved
mpv has an internal inconsistency where some options are guarded at
compile time and others are not. One advantage of guarding certain
options is that it prevents users from using something that can't
possibly work (e.g. windows-specific options on a linux machine).
However, this hurts the portability of mpv config files and means it's
possible for configs to break depending on a machine. This is not so
nice.

Attempt to have our cake and eat it too by introducing a new
not_available boolean which can optionally be set when defining options.
Naturally, you just set this to the negation of the preprocessor that
applies. Instead of doing a hard error, mpv will print out a generic
warning message that you are trying to use an option that cannot ever
work with the binary. As a downside of this new approach, a bunch of
windows-specific defines have to be explicitly handled which is ugly.
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.

2 participants