osc.lua: add osd-margin option#17544
Conversation
kasper93
force-pushed
the
osc-margin
branch
3 times, most recently
from
6cae843 to
2fa38c2
Compare
The OSC (with mpv-player#17544) and uosc increase --osd-margin-y along with user-data/osc/margins to not overlap with OSD messages, which positions console far apart form the bottom. Fix this by using the initial values of --osd-margin-{x,y} without updating them at runtime. Fixes mpv-player#15334 (comment)
The OSC (with mpv-player#17544) and uosc increase --osd-margin-y along with user-data/osc/margins to not overlap with OSD messages, which positions console far apart form the bottom. Fix this by using the default --osd-margin-{x,y} when OSC margins are set. Fixes mpv-player#15334 (comment)
| ``sub_margins`` | ||
| Default: yes | ||
|
|
||
| Whether to adjust ``--sub-margin-y`` so that subtitles do not overlap | ||
| with the OSC. The offset is derived from the bottom OSC margin and added | ||
| on top of the current ``--sub-margin-y`` value. Requires | ||
| ``dynamic_margins`` or ``visibility=always`` to take effect. |
There was a problem hiding this comment.
I think the default should be no. This will mess with watch-later and any kind of "persistent config" scripts, and I think mpv should not do that out of box.
The boxvideo option does similar thing, but it is disabled by default and points out the caveats in the documentation:
If this option is set, the osc may overwrite the --video-margin-ratio-* options, even if the user has set them.
There was a problem hiding this comment.
Could disable them by default, and document it, to add in mpv.conf:
watch-later-options-remove=sub-pos
# osd-margin-y is not saved, but just in case
watch-later-options-remove=osd-margin-y At least that way no one is bothered by defaults since they're disabled, and those that enable it are aware of what could happen and how to circumvent it.
There was a problem hiding this comment.
I think the default should be no. This will mess with watch-later and any kind of "persistent config" scripts, and I think mpv should not do that out of box.
The impact is minimal, user need to change option while the OSC is visible and before the original value is restored. Also note that on shutdown the margins are restored to original state always. Though now that I think about it, it is probably too late in some cases.
This will mess with watch-later
osd/sub margins are not in default watch later options. So only if user explicitly opt-in to saving those, it could be a problem. And since those properties are relating to player state, it is probably very uncommon to save the on per-file watch later config.
Of course we will monitor the user feedback and revert if needed.
There was a problem hiding this comment.
The impact is minimal, user need to change option while the OSC is visible and before the original value is restored. Also note that on shutdown the margins are restored to original state always. Though now that I think about it, it is probably too late in some cases.
Watch later files are written before any scripts are aware of shutdown. If a script saves these option values while running, osc.lua has no way to know about that. And if a script saves these option values on quit, the values saved can depend on the order the scripts quit.
osd/sub margins are not in default watch later options. So only if user explicitly opt-in to saving those, it could be a problem. >And since those properties are relating to player state, it is probably very uncommon to save the on per-file watch later config.
The users have no way to know about this if they do not read the documentation of osc.lua, which is an unnatural place to watch when they are looking at watch later options, and they should have no reason to worry about when choosing which option are changed by mpv.
osc.lua has never done something like this before by default, so this breaks the general expectation that built-in player scripts do not modify these options.
There was a problem hiding this comment.
Usability improvement for vast majority of users outweighs theoretical case of 1 user who wants to save osd margin in his watch later config or sub margin, same thing.
If you think we should prioritize theoretical usecases for single user, we can revert.
EDIT: We can add private internal property dedicated for osc to use if this issue is critical.
There was a problem hiding this comment.
I just found out a critical issue caused by this: this affects subtitle position on screenshots, which should not happen because osc/osd are not rendered on screenshots by default, so on the screenshots the margin will be too large.
If you think we should prioritize theoretical usecases for single user, we can revert.
The use case is not theoretical. This config persists sub-margin-y and has 32 stars.
There was a problem hiding this comment.
Are you saying that both should not be enabled by default or osd is ok? I initially had subtitles disabled, because I was aware it would cause issues like that, but guido said it should be fine, so not sure.
I just found out a critical issue caused by this: this affects subtitle position on screenshots
It will render at the same position as currently displayed, no? Seems expected behavior. And remember for text subtitles, there is no "correct" position really.
EDIT: Also this is only limited to dynamic_margins=yes or visibility=always, both of which are not default. Nothing changes by default.
There was a problem hiding this comment.
Are you saying that both should not be enabled by default or osd is ok? I initially had subtitles disabled, because I was aware it would cause issues like that, but guido said it should be fine, so not sure.
I think there are many practical issues with sub-margin that it should be reverted. For osc-margin, I still think it should not be enabled by default (e.g. scripts like this that set and reset osd-margin-y dynamically), although uosc does set osd-margin-y (but not sub-margin-y) in a similar fashion, so it may be less problematic.
It will render at the same position as currently displayed, no? Seems expected behavior. And remember for text subtitles, there is no "correct" position really.
In the context of screenshot, it results in too large margin, which is only there in the context of osc occupying a part of window. And it does nothing for image and ASS subtitles, so it also causes inconsistent behavior.
EDIT: Also this is only limited to
dynamic_margins=yesorvisibility=always, both of which are not default.
visibility is bound by a key by default, and a user can switch the option at runtime without an intention to change the default config. It is like saying mpv is free to do something strange when volume is not 100 because it is "not default".
There was a problem hiding this comment.
osc can just disable these options if --watch-later-options contains --osd/sub-margin-y (which is an edge case anyway).
There was a problem hiding this comment.
@na-na-hi @guidocella: Does #17555 resolve this topic?
4 participants
No description provided.