search

mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.78k stars 2.93k forks source link

Allow frontends/GUIs to write log messages #14551

Open stax76 opened 4 months ago

stax76 commented 4 months ago

Expected behavior of the wanted feature

Allow frontends/GUIs to write log messages, support a new input command with three parameters:

  1. Log level
  2. Module name
  3. Log message

This way errors from mpv.net and mpv.net extensions can be seen directly in the on-screen console, no need to have a terminal attached.

Alternative behavior of the wanted feature

No response

Log File

No response

Sample Files

No response

kasper93 commented 4 months ago

What do you mean by frontend? You can use mp_client_get_log to get log object and use that to log.

stax76 commented 4 months ago

With frontend, I mean a mpv GUI like mpv.net, which uses the client API.

kasper93 commented 4 months ago

So, use mp_client_get_log

stax76 commented 4 months ago

I cannot find mp_client_get_log here:

https://github.com/mpv-player/mpv/blob/master/libmpv/client.h

avih commented 4 months ago

I cannot find mp_client_get_log here:

Good catch. I also assumed there's a libmpv api for logging, but seems there isn't.

kasper93 commented 4 months ago

Oh, my bad. Our public api is prefixed with mpv_. So it seems like lua/javascript have own wrappers to expose logging api, but C API doesn't have any. We could expose mpv_client_get_log and msg.h in public headers.

na-na-hi commented 4 months ago

libmpv clients are expected to be responsible to logging themselves with mpv_event_log_message. If you write your own on-screen console instead of console.lua you can display mpv and client logs.

kasper93 commented 4 months ago

libmpv clients are expected to be responsible to logging themselves with mpv_event_log_message. If you write your own on-screen console instead of console.lua you can display mpv and client logs.

That's also true. There is possibility to log directly to terminal form libmpv with terminal=yes, but maybe we should force libmpv users to rely on mpv_event_log_message if they want to modify/add logs.

stax76 commented 4 months ago

libmpv clients are expected to be responsible to logging themselves with mpv_event_log_message.

With that, I can only receive log messages, but I want to write log messages.

avih commented 4 months ago

libmpv clients are expected to be responsible to logging themselves with mpv_event_log_message. If you write your own on-screen console instead of console.lua you can display mpv and client logs.

If I understand you correctly, you mean that clients should capture the log messages, and then output/write those to wherever they want, and while at it, also insert their own messages in between as much as they like?

If yes, then I think it's not what OP requested.

The use case here, I'd imagine, is that the mpv logging backend is used normally, for instance logging to the console or to a file, and the libmpv client wants to add log messages which clearly came from that client and not from other sources in mpv, but there's no api available to them other than running a script and then using mp.log* api from the script - which is very awkward.

I think it's reasonable to expect that libmpv should have logging api.

na-na-hi commented 4 months ago

If yes, then I think it's not what OP requested.

Correct. The same callback-based logging mechanism is also used by some mpv dependencies like libass. mpv captures the logs from these libraries and displays them on its own. I'm showing that this feature request isn't strictly necessary for this reason.

That said, scripts already have access to the logging API, so I think this feature request is reasonable.

low-batt commented 4 months ago

Previously I ran an experiment to see if IINA could use one combined log file. I experimented with IINA capturing all mpv log messages with mpv_event_log_message. I found it to be too problematic to use as it overloaded the event system and log messages were dropped unless it was restricted to just capturing warning and error level messages.

This is not the only log related issue for clients. See issue #12250. Every once in a while the mpv log file ends up containing a large block of NUL characters.

kasper93 commented 4 months ago

I found it to be too problematic to use as it overloaded the event system and log messages were dropped unless it was restricted to just capturing warning and error level messages.

Wasn't that fixed by @sfan5 in https://github.com/mpv-player/mpv/pull/13363?

low-batt commented 4 months ago

I don't know the current status. Been quite a while since I ran that experiment. Possibly that was with mpv 0.35.1. PR #13363 was merged in January.

The documentation for mpv_wait_event says;

The internal event queue has a limited size (per client handle). If you don't empty the event queue quickly enough with mpv_wait_event(), it will overflow and silently discard further events

Events are critical to the operation of the client. Bad client behavior happens if an event is dropped. I decided it was unwise to risk overflowing the event queue just to combine log files and did not look further into this.

The problem with blocks of NUL characters in the mpv log file is very rare. I had to develop a stress test to reproduce it. It was on my mind as I recently found NUL characters in a log file again. I'd have to try reproducing it against master to know for sure if it is still a problem. At the moment I'm very busy as IINA is in the process of upgrading to mpv 0.38.0.

sfan5 commented 4 months ago

Wasn't that fixed by @sfan5 in #13363?

that's a very specific case but could very well be what @low-batt was seeing.