-
Notifications
You must be signed in to change notification settings - Fork 3k
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
console.lua: add script-opt --max-msg-level #11846
base: master
Are you sure you want to change the base?
Conversation
default value: |
Does the most recent commit address this sufficiently? |
Sry i might misunderstand this option previously. I thought it generates logs independently. if it uses the same lv from option --msg-level, actually it did nothing. We just need set it trace to share the same output. |
I'm not sure that trace as the default is optimal. The original reasoning for discarding them is still valid: Lines 828 to 832 in effc680
If someone were to use |
But the current commit is the same as set it "trace" |
Lol, true. I'll swap back to my first commit tomorrow. It didn't bother trying to detect anything from --msg-level. Initially I used info as the default. Does this sound good, or should it be raised to verbose? |
As a win-user, I always use console instead of cmd. |
ae1856a
to
8dd2f8d
Compare
--max-msg-level controls the maximum message level for the console. It filters messages, discarding any at a higher level than its value. It does not affect the terminal or log file outputs. It can be set to any of --msg-level's levels (excluding status). The default is v. Previously, the console would always discard trace messages, but show all other messages. Now, if mpv is generating trace messages (controlled by --msg-level), they can be shown in the console using --max-msg-level=trace, as expected. DOCS/console.rst has been updated to reflect this addition. Also: * Remove a variable that duplicated the error message style within the help command. * Add a missing newline to an error message. closes mpv-player#11761
Please excuse the mess above! v (verbose) is now the default for --max_msg_level and the --msg-level detection has been removed! Let me know if any if there are any other issues. |
I'm not sure if it was caused from mpv itself or this commit. |
I think it might be |
I don't think the new changes are at fault. A minimally modified (to output the messages it receives to a file) console.lua (from mpv/master) also results in many dropped messages at levels higher than info (compared to the number of messages at any level from diff of mpv/master console.lua and min-modified-console.lua--- mpv-master-console.lua
+++ min-modified-console.lua
@@ -813,6 +813,8 @@
-- until enable_messages is called again without the silent: prefix.
mp.enable_messages('silent:terminal-default')
+local console_log_file = io.open('min-modified-console.log', 'w')
+
mp.register_event('log-message', function(e)
-- Ignore log messages from the OSD because of paranoia, since writing them
-- to the OSD could generate more messages in an infinite loop.
@@ -829,7 +831,7 @@
-- level includes them. These aren't too useful for an on-screen display
-- without scrollback and they include messages that are generated from the
-- OSD display itself.
- if e.level == 'trace' then return end
+ if e.level == 'trace' or e.level == 'debug' then return end
-- Use color for debug/v/warn/error/fatal messages. Colors are stolen from
-- base16 Eighties by Chris Kempson.
@@ -847,6 +849,12 @@
end
log_add(style, '[' .. e.prefix .. '] ' .. e.text)
+ console_log_file:write(e.level .. '\t\t' .. e.prefix .. ': ' .. e.text)
+ console_log_file:flush() -- Preserve the message order.
end)
+mp.register_event('shutdown', function()
+ console_log_file:close()
+end)
+
collectgarbage() |
Using:
files:
mpv log lines <335 were presumably pushed out of the console's buffer. 1.log-file.log I'll try to look into this more, particularly at how the console falls behind when mpv generates more messages than it can handle. |
@catcake: please rebase @guidocella: do you have opinion on that? |
Logging code is now very different with the addition of suggestion and terminal styles so it's up to the author if he's willing to update this. |
I have been writing scripts to extend mpv for a few months and eventually grew annoyed that the console did not allow trace messages, so I made an option to control this (mentioned in #11761).
If you aren't interested in using the console for trace messages (which is probably more sane), the option can still be useful for nearly the opposite reason. The console can be restricted to only important messages, while the terminal shows more granularity.
These commits are functional & already implement what's mentioned above, but I have a couple related ideas and questions:
debug and trace message styles/colors
I think they're identical, at least in the terminal on Windows (and trace wasn't relevant to the console before this). Looking at the theme that the console pulls from (here), would gui05 (#747369) be a good color for trace to differentiate it from debug messages?
a non-static default for
--max-msg-level
Rather than
info
, when--max-msg-level
is not explicitly set, maybe determine the most noisy level set in--msg-level
and use that? I actually wrote this already, but I wasn't sure if it was desirable to anyone other than myself.I've tried to be thorough, but please let me know if there's anything I should correct! Thanks!