Romain Vimont authored 3 years ago and Romain Vimont committed 3 years ago

The deadline computation was shared between the vout thread function and
DisplayPicture(), both by hacking the return value (VLC_EGENERIC meaning
"wait") and using an output parameter, to handle 3 possible cases:
 - VLC_SUCCESS: do not wait
 - VLC_EGENERIC and deadline set: wait for deadline
 - VLC_EGENERIC and deadline not set: wait but no deadline is known

In case of VLC_EGENERIC, the vout thread function computed the max
between the returned value and 100ms.

Simplify by computing the expected deadline from DisplayPicture()
directly.

This code is already handled in vout_Request().

Fixes #26394

Refs #26394

This is not a failure. This is a perfectly normal and expected
situation. If PCM also fails, we will then print an error message.

The display cannot be started if the window is not enabled first.
If enabling the window failed, we can infer that the display was not
started, so there is no point trying to stop it.

Lyndon Brown authored 3 years ago and Jean-Baptiste Kempf committed 3 years ago

caused a big block of warnings.

Lyndon Brown authored 6 years ago and Hugo Beauzée-Luyssen committed 3 years ago

see e967f81f.

Lyndon Brown authored 6 years ago and Hugo Beauzée-Luyssen committed 3 years ago

see e967f81f.

note, this does **not** affect cat-based module selection items
(of which there are just three in use by the core), since that
mechanism uses subcats not cats.

Lyndon Brown authored 6 years ago and Hugo Beauzée-Luyssen committed 3 years ago

see e967f81f.

Lyndon Brown authored 3 years ago and Jean-Baptiste Kempf committed 3 years ago

i presume this would be problematic.

Alexandre Janniaux authored 3 years ago and Rémi Denis-Courmont committed 3 years ago

Currently, when encoding an ES category, the encoders had to check
whether they supported the ES category and return an error, forcing many
modules to be loaded and probed for nothing.

By splitting by ES category, we ensure that most encoder that would have
not supported the encoding anyway won't be probed. As a side effect, it
improves the documentation in the help output by only listing encoders
that will actually be able to encode the format, for instance in
aenc/venc/senc options in the transcode module. It will also provide a
better way for the UI to list the available encoders.

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

to be clear about what we do for them.

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

i don't believe we have any short options assigned to float options
currently, but we should have this case here prepared to handle it.

Lyndon Brown authored 3 years ago and Jean-Baptiste Kempf committed 3 years ago

creating the option label with `asprintf()` up front allows us to tidy
things up quite a bit, removing most notably the duplication of strings
that differ only between char and string format items.

Lyndon Brown authored 3 years ago and Jean-Baptiste Kempf committed 3 years ago

experimenting with this i decided that it is cleaner to not use them, for
valid option names at least. they are still used for unknown option quoting
for instance, and where we are quoting multiple CLI arguments like
`--foo <VAL>` or `vlc --foo`.

Lyndon Brown authored 3 years ago and Jean-Baptiste Kempf committed 3 years ago

inspired by Rust's cargo output, IMO this adds a nice little
enhancement to UX along with the colour error/warn labels.

Lyndon Brown authored 3 years ago and Jean-Baptiste Kempf committed 3 years ago

old: "Try `vlc --help' for more information."
new: "For more information try `--help`"

in experiments, the new text proved much cleaner. it becomes yet more
clean, having dropped "vlc" from `vlc --help`, when we then drop the
quotes around the remaining `--help` in the commit after next.

note, we now no longer have "vlc" in any of the error output. this
is not a problem for the vlc app itself obviously; i hope it is not
a problem for any other libvlc use cases.

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

i.e. if a user tries a long option which does not exist, if it is a close
enough match to one or more available options, the error message can
include the best match as a helpful suggestion.

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

to be used with suggestion matching.

this implementation is based upon the implementation from the `strsim` Rust
crate, authored by Danny Guo, available at [1]; more specifically the (as
yet un-merged) optimised copy authored by myself, available at [2]. the
code is available under the MIT license.

one implementation difference is that we use floats rather than doubles
since we only intend to use this for suggestion matching in unknown option
error messages and we don't need the extra precision.

[1]: https://github.com/dguo/strsim-rs
[2]: https://github.com/dguo/strsim-rs/pull/31

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

for subsequent use with suggestion matching.

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

improves UX by helping to highlight problems in terminal output.

for now, as with the rest of the codebase, we rely upon only a
simple `isatty()` check for automatic colouring decisions, which
does not work for Windows. implementing `isatty()` for Windows to
could perhaps be tackled later.

---

note that user control over use of colour in terminal output via the
use of `--color`/`--no-color` options is deliberately ignored with
respect to the use of colour within error and warning output in
response to any problems encountered whilst processing the supplied
command-line arguments themselves (unknown option error; missing option
value error; obsolete option warning), having decided that this was the
best choice. this was questioned in review of this work, so i have
expanded/documented my thoughts on it here below.

 1. default behaviour depends upon whether or not a detection routine
    determines that output is connected to a tty. if so then we choose
    to use colour in the hope that most users will appreciate the
    highlighting of problems. if not, i.e. output is thus being
    redirected into a log file or another application, we choose to
    not use colour because embeddeding terminal sequences into the
    output in such situations risks causing confusion due to the
    likely possibility of them not being consumed as intended.

    the only situations where users may want or need to change this
    default behaviour, are when either:

     a. they happen to dislike the use of colour.
     b. they want to force use of colour on Windows, due to current
        lack of a tty detection mechanism.

    the latter of course becomes redundant as soon as we implement
    an `isatty()` for Windows.

    it is reasonable in my opinion to fail to honour such a user
    preference for the limited circumstances of encountering a problem
    with the very data-set in which the user communicates that
    preference.

    never-the-less, let's briefly explore the possible solutions
    that could in theory be implemented to try to honour such user
    preference for such output.

 2. solution 1: "pre-scanning"

    the concept was raised of "pre-scanning" the set of arguments to
    try to learn the user's indicated preference ahead of properly
    processing them, such that we can ensure that it is obeyed if/when
    outputting such an error/warning for a problem encountered in
    undertaking such processing. i.e. essentially walking through the
    argument list looking out for arguments matching a few fixed
    strings like "--color", and thus taking note of the preference
    before we begin the proper argument processing work.

    it should be understood that the rules of proper argument processing
    dictate that you cannot with certainty know the nature of an
    argument until you have processed fully all of those preceeding it.
    options that take data values can be used with the value given
    within the same argument or supplied within the next one, and so you
    never know if an arbitrarily given argument must be consumed as a
    data value rather than treated as a possible option until you've
    processed all proceeding arguments; a special "early terminator"
    (`--`) argument indicates that all subsequent arguments must be
    treated as generic arguments (that must not be interpretted as
    options); and an unknown option argument brings processing to a halt
    since it is not possible to correctly interpret the nature of
    anything subsequently given.

    despite this, practically speaking we could get away with
    pre-scanning - there are no "short" option forms of these toggle
    options to complicate things; and making simple assumptions that
    arguments matching "--color" or "--no-color" (or "--nocolor") are
    actual uses of those options is pretty reasonable along with assuming
    that a "--" argument is an actual early terminator, since such
    assumptions are almost always likely to be correct except in certain
    very unusual circumstances of broken CLI use or from users
    deliberately looking for flaws.

    however, i do not feel that it is worth adding a chunk of code
    just to essentially ensure that a hypothetical and presumably rare
    CLI user who dislikes colour will not see it should they happen to
    make a mistake in their CLI usage.

    (it might be noted that the vlc binaries do similarly flawed scans
    to steal one or two options, but that's not readily avoidable).

 3. solution 2: "updating as we go":

    as an alternative concept, we could, upon successfully processing
    each option, check if the option is `--color` or `--no-color` and if
    so, update the `color` boolean accordingly, such that if we run into
    a problem and thus need to output an optionally coloured error or
    warning label, we might thus obey the user's preference when doing so.

    however, we have equal chance that we do the opposite since there
    could be a later argument that we did not yet get to that flips the
    preference the other way (e.g. `vlc --no-color --foo --color`). in
    a worst case scenario the user might end up with a mix of coloured
    and non-coloured output that's messy and possibly confusing.

    again, i do not feel that it is worth implementing code to do this,
    to introduce an inefficiency into the option processing for such
    little benefit, especially with the risk of this creating a mess
    of inconsistency in some cases.

we (i) should endeavour to introduce `isatty()` support for Windows such
that Windows CLI users get the benefits of colour highlighting, but i
feel that it is perfectly reasonable to take a stance of not bothering to
try to enforce user no-color preference upon argument processing
errors/warnings.

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

 - replaced use of `GRAY` and `WHITE` for resetting with `RESET` and
   `RESET_BOLD` respectively.
 - addressed hidden use of bold by creating separate colour and colour+bold
   defines.
 - added a `TS_` prefix to help clarify that the defines, where they are
   used, are terminal sequences.

Lyndon Brown authored 6 years ago and Jean-Baptiste Kempf committed 3 years ago

the terminal control code stuff is relevant to (and wanted by me for)
cmdline.c