mpv-player / mpv

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

loadfile: update the format of terminal track information #14405

Closed guidocella closed 3 months ago

guidocella commented 3 months ago

Stop making unselected tracks and editions grey because they can be hard to read over a dark background (\033[2m would be hard to differentiate from regular text with a light theme instead), and because there is no way to not print the escape sequences in --log-file.

Just use the same circles as the OSD and OSC. We need to print the empty circles for alignment on mlterm with East Asian fonts (we could also make them invisible with \033[8m but it would still get added to log files).

Add back the space before tracks and editions so they look like a sub-section of the "Playing..." line printed with playlists and of "Track switched", consistently with the metadata which starts with space which makes it look like a sub-section of the "File tags" line.

Also stop converting Hz to kHz for consistency with other log messages, e.g. AO: [pipewire] 48000Hz stereo 2ch floatp

sfan5 commented 3 months ago

relevant PR https://github.com/mpv-player/mpv/pull/14192#event-13085241215

github-actions[bot] commented 3 months ago

Download the artifacts for this pull request:

Windows * [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1631881165.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1631887492.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1631901239.zip)
macOS * [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1631879796.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1631882603.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1631877788.zip)
guidocella commented 3 months ago

Do we want to merge the track data in one set of parentheses, or at least hls-bitrate and external?

rach-md commented 3 months ago

Could you make it more consistent? samplerate on ao does not use space but track does. Also codec profile only show up after track switched, so show them from very first or just remove it.

kasper93 commented 3 months ago
[  0.035624]    cplayer: ● Video  --vid=1  --vlang=eng  Salt (2010)  (*) (h264 1920x800 23.976 fps)
[  0.035679]    cplayer: ● Audio  --aid=1  --alang=eng  AC3 5.1 @ 640 Kbps  (*) (ac3 6ch 48000 Hz)
[  0.035722]    cplayer: ○ Audio  --aid=2  --alang=pol  AC3 5.1 @ 640 Kbps  (ac3 6ch 48000 Hz)
[  0.035761]    cplayer: ○ Audio  --aid=3  --alang=eng  Commentary  (aac 2ch 48000 Hz)
[  0.035810]    cplayer: ○ Subs   --sid=1  --slang=pol  (*) (subrip)
[  0.035846]    cplayer: ● Subs   --sid=2  --slang=eng  (subrip)
[  0.035881]    cplayer: ○ Subs   --sid=3  --slang=eng  forced  (subrip)
[  0.035914]    cplayer: ●  --edition=0 'Director's Cut' (*)
[  0.035947]    cplayer: ○  --edition=1 'Extended Cut'
guidocella commented 3 months ago

Updated. Editions had issues because I've been modifying them blindly since I can't find any file with them so I don't think it's worth adding an option alias just for aligning them with tracks when they're so rare.

kasper93 commented 3 months ago

Looks like this now

[  0.035891]    cplayer: ● --edition=0  'Director's Cut'  (*)
[  0.035935]    cplayer: ○ --edition=1  'Extended Cut'
[  0.035994]    cplayer: ● Video  --vid=1  --vlang=eng  'Salt (2010)' (h264 1920x800 23.976 fps) (*)
[  0.036032]    cplayer: ● Audio  --aid=1  --alang=eng  'AC3 5.1 @ 640 Kbps' (ac3 6ch 48000 Hz) (*)
[  0.036073]    cplayer: ○ Audio  --aid=2  --alang=pol  'AC3 5.1 @ 640 Kbps' (ac3 6ch 48000 Hz)
[  0.036106]    cplayer: ○ Audio  --aid=3  --alang=eng  'Commentary' (aac 2ch 48000 Hz)
[  0.036139]    cplayer: ○ Subs   --sid=1  --slang=pol  (subrip) (*)
[  0.036172]    cplayer: ● Subs   --sid=2  --slang=eng  (subrip)
[  0.036204]    cplayer: ○ Subs   --sid=3  --slang=eng  'forced' (subrip)

I'm fine with this. Let see what @sfan5 thinks though.

guidocella commented 3 months ago

Maybe use single spaces for editions?

kasper93 commented 3 months ago

Maybe use single spaces for editions?

Ok for me.

sfan5 commented 3 months ago

can we maybe not make this into one mega-commit that changes everything all over again?

there are clear problems with the current code that I would prefer to see fixed before the next round of bikesheddings. (by current I mean what's currently in master since d49879f1f7c3bc6fada32856a368352a09359a6c)

kasper93 commented 3 months ago

there are clear problems with the current code

Which ones specifically? I though we are bikeshedding it from the beginning.

guidocella commented 3 months ago

Which ones specifically? I though we are bikeshedding it from the beginning.

Same, and the changes are already done.

kasper93 commented 3 months ago

LGTM. I'm stopping here. I promise, I won't comment anymore.

sfan5 commented 3 months ago

Which ones specifically? I though we are bikeshedding it from the beginning.

The color handling regarding log file and depending on stdout state is objectively broken.

mlindner commented 2 months ago

The [default] getting added to every track name really inflates the size, as well as all the pointless padding. Please consider the real estate of people's windows and 80 character limits.