player: integrate stats.lua #4918
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the script would throw garbage (ASS tags) at the terminal when the bound key was pressed. This changes the behaviour to _not_ print any ASS tags (and replace those which can be interpreted by the terminal) if there's no video. I cleaned the patch up since you mentioned you were busy. As I said before, there is absolutely no problem with calling mpv to display strings to the OSD without any video. They'll just go straight to the terminal just as they would with an active VO.
Also add a few convenience functions and remove the unused italic and underlined formatting functions.
Previously we unnecessarily added newline characters at the end. Only noticeable when printed on terminal, though.
Some properties were renamed recently. Of course this requires a recent mpv built (> f9507f) now.
print a warning for properties without value
and comment nitpicking
Unify both append_property* functions and greatly refactor them. Instead of thousands of arguments we now use a table. While this is in theory cleaner it does not exactly look like it. However, it's way more flexible and extendable this way. Also, since the new append_property() might look a bit confusing I felt the need to add a comment.
this one slipped through my "tests"
feels a bit better that way
This simply prints ASCII codes to display any text marked as bold in the terminal. Supported by every sane terminal since 1986. For those insane, there's a check. The check has been copied from the ansicolors.lua script floating around and it checks if the directory path uses "\" instead of "/", and in case it does, it checks whether ANSICON env variable has been set (which is used to indicate the Windows terminal supports ACII escape sequences).
These spaces were not displayed on screen, however they were taken into account for line wrapping.
It's getting a bit cramped
Fallback to hwdec-active if not available. This one will be removed in mpv 0.19.0.
You can now either show the stats once or toggle their display. Both are using different key bindings which are additionally configurable now. Please bear in mind that "toggling" means "redraw every x seconds for x seconds". Therefore, this approach is prone to problems especially when something else is printing text to the OSD as well as every of these calls will overwrite each other. This is currently a limitation of mpv. Fixes #18
More useful names
Print it to console and especially OSD upon each invocation
Previously the delay was slightly shortened but a user explicitly setting a specific delay most likely expects the stats to refresh in exactly the frequency he desired.
Please note that the latest version of this script needs a very recent version of mpv (from yesterday, to be precise, see the readme). For older versions, please go to "releases". HOW IT WORKS: While the stats are visible (i.e. text is printed to the OSD) a subsequent click on a numeric key (1, 2, ...) will display the corresponding "page". This works no matter if the stats are toggled or just shown as a single invocation. In case of a single invocation, the newly displayed page will be shown for the full duration again. The selected page will be remembered (not persistantly though). So far, only 3 pages are available. 1: the default page, stats as they used to be 2: extensive VO performance stats (to be redesigned/changed soon) 3: dummy In the future, many more pages are possible. Implementation is likely to change again (functionality will stay the same). A new timer had to be introduced to remove the forced keybindings in the oneshot case. The toggle case can remove them without a timer. Ensuring that each mode won't remove timers of the other mode didn't really turn out neat. Therefore, I intend to change this again, maybe by merging the oneshot case into the toggle case.
A keybinding in input.conf like: e script-binding stats/display-page-2 can be used to directly show the respective page (2, in this case)
Previously I could trigger a bug with intense button mashing, however, was unable to reproduce it and therefore debug it. This change now seems to be resilient against button mashing, let's hope it really is.
Almost cosmetic change. This tables-in-table was done back when we actually processed strings but that's long ago now and no longer needed nor useful.
There's no reason it's not.
There is enough space now
There's no point in disabling it anyway
There was a superfluous newline and some indentation
Due to popular demand
It's apparently already stated as part of the upload pass name. This reverts commit ec837f6.
Clarified the relationship between `Dropped` and `VO`, and also merged the DS-exclusive stats into the DS line.
Fixes #44 (well, partially, I'm still not sure about the time)
It used a bad heuristic that got even worse/less reliable with recent changes in mpv. In fact, it's not reliable at all. Watch out for dropped frames instead. That's a useful indicator.
Previously multiple timers were used to realize oneshot, toggling (redrawing) and page keybindings. The oneshot case in particular also relied on mp.osd_message to display text only for a given duration. This was changed to only use one timer in total now. Because now each case has a defined "start" and "end" point (including oneshot) mp.set_osd_ass() can be used to print stats as well. This is currently optional and has to be activated using the config option persistent_overlay=true. One shortcoming: oneshot and toggling are mutual exclusive right now. Previously you could enter toggling while oneshot stats were shown, this is not possible anymore to reduce the number of cases to be considered. This can be added later on if desired.
avih
reviewed
Sep 24, 2017
|
|
||
| -- Text style | ||
| font = "Source Sans Pro", | ||
| font_mono = "Source Sans Pro", -- monospaced digits are sufficient |
There was a problem hiding this comment.
What happens if the user doesn't have this font installed?
There was a problem hiding this comment.
Fallback as performed by libass or whatever it uses to lookup fonts.
There was a problem hiding this comment.
Is there some generic way to ask any monospace font?
On a system which doesn't have this font (and arguably most systems don't have it), it'll probably choose a non monospace one, which will look ugly, even if the user does have other mono fonts installed, wouldn't it?
| and type(package.config) == 'string' | ||
| and package.config:sub(1, 1) == '\\' | ||
| if is_windows then | ||
| return os.getenv("ANSICON") |
There was a problem hiding this comment.
Even without ansicon (or conemu) mpv knows to translate some ansi escape sequences on windows too. IIRC at the very least basic colors, clear line, and move cursor to the beginning of the line.
If you need a sequence which mpv doesn't support on windows, it could be simple to add.
There was a problem hiding this comment.
This was added by @atomnuker IIRC. I have no way to test on Windows.
|
|
||
| -- Toggling key binding | ||
| mp.add_key_binding(o.key_toggle, "display-stats-toggle", function() process_key_binding(false) end, | ||
| {repeatable=false}) |
There was a problem hiding this comment.
Not that it matters much IMO, but why do you disable repeat here?
There was a problem hiding this comment.
There's no point. There's no advantage to gain from key repeat here but there are plenty of counterintuitive disadvantages.
|
Force-pushed a new version of this PR, now the history of the stats.lua file is preserved (and rewritten to follow mpv's guidelines). |
|
Relicensed the script to LPGL 2.1 (https://github.com/Argon-/mpv-stats/issues/47) |
stats.rst is heavily based on osc.rst
|
Merged, with a small change mentioned on IRC. |
ghost
closed this
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Well then, let's try this.
Naturally, I'd continue maintaining/extending the script.
In case this would get accepted, there are a few things to do before merging:
Should I add averdict: not neededstats=nohere? https://github.com/mpv-player/mpv/blob/master/etc/builtin.conf#L20Regarding default options: any specific wishes? Doesn't collide with default bindings right now, as far as I can see. One more thing: the script can do both, print using
mp.osd_message()(can get overwritten by other printed text) andmp.set_osd_ass()(persistent but might overlap with other printed text). What would be a sane default (as in: best user experience) for that?