feat: add sub_margins, osd_margins and dynamic_margins options#523
feat: add sub_margins, osd_margins and dynamic_margins options#523
Conversation
|
Wow that was quick. Will try it out very soon. For subtitles, how much margin does it know to offset by? Specifically what I'm wondering is would it still overlap with title or chapter title if enabled? I personally don't use either of these, so it doesn't bother me if they overlap though. Why sub-pos and sub-margin-y both exist but accomplish the same thing? Now I'm just thinking out loud at this point lol. |
It should never overlap. This method uses the margins set by OSC height. similar to how select menu (long playlist) does not overlap with OSC.
We were joking about it on irc at one point, that mpv has so many options, a student can write their thesis on them and pass. 😆 |
|
Also, on an unrelated note, this PR has the phrase |
You're not wrong lol. Scrolling through the documentation and you can tell it's more than bloated. What is that shutdown event? Does it reset the script-adjusted sub pos on shutdown? |
Ensures that they're reset back to default/user config. In essence, |
|
I'll update docs and I need a break from PRs lol. |
|
If WC is visible and you scroll up/down on the volume bar (which triggers OSD) it seems to be preventing the WC from timing out. However I don't know if that's expected so I'll point it out anyway. Subtitle works well too. Maybe it raises a bit high if the title and/or chapter title is disabled. Should those options be taken into account?
I'm not sure if this is working as intended. When doing a quit-water-later it seems they're permanently raised on next startup. Why not enable osd_margin by default? It's useful. |
You mean while hovering WC scroll up/down? hovering WC will keep it showing, regardless of mouse actions.
True, we might need to document, add in watch-later-options-remove=sub-pos,osd-margin-y
Fear of what you just mentioned, that especially should only be enabled by user when he's aware they should adjust watch later options. |
|
Ah, it's actually seems unrelated. But I was noticing that WC hideout was not working while scrolling the volume bar, like this:
Result is the WC stays on screen until you stop changing the volume. Anyway,
What happens if we lower the value of |
|
Maybe both
It does, yes. OSC margins use osc_height and fade_alpha value. Whichever is highest, is the OSC boundary. osc_param.video_margins.b = math.max(user_opts.osc_height, user_opts.fade_alpha) / osc_param.playresy
Nice catch. |
Unless I messed something up, this option doesn't fix the issue. They are still raised on startup.
|
With Without My watch-later-dir="~~/cache/watch_later"
write-filename-in-watch-later-config=yes
watch-later-options-remove=sub-pos |
Then it's working as intended. This would be good to document about the fade_alpha. I personally found the lower limit for osc_height is around 115 otherwise it messed up clicking on OSC elements.
Ok, sorry, it works with just this line. However if you add
|
True, turns out watch-later-options-remove=sub-pos
watch-later-options-remove=osd-margin-yBut, from my testing, |
|
By the way, the whole But at least now we should document and tell users what to do.
I will document this as well. |
|
Can we force enable that option? Or just rely on the user to set up their mpv.conf properly. Either way, the PR works really well. I'm impressed. Thanks for getting it in so quick. |
I thought about it, but I don't know if we should. We'd be altering the user's configuration without their knowledge. Even if we tie it to an option that could be disabled. I'm not saying no, I just don't know if yes is ok lol. |
|
That's fair. Side note, is user_opts.md even needed? Since modernz.conf has the same content and updating so many files gets a bit too much. |
It is a pain in the ass and probably is read by 1% of users. However, it makes readability easier and we're able to add more detailed notes like the ones I just added about OSC boundary, watch later and so on. |
|
All done. I think this PR is ready to merge, let me know if I missed something please. If it's not a bother.
I'll try to fix this in another PR. |
|
Looks good to me too. |
Forgot to reply sorry. I agree they should be disabled by default to avoid issues or just disable |
|
I was discussing it on irc, and guido suggested a great solution: What do you think? At least then we can enable it all by default. Edit: |
|
Yeah that seems like a good solution! |
Changes:
sub_marginsoption: Raises subtitles above OSC if it's visible, then restores it when OSC is hiddenosd_marginsoption: Adjusts displayed OSD position to not overlap with OSC/WC when it is visibledynamic_marginsoption: Allows subtitles and OSD to be adjusted when visibility mode is set to auto (default)raise_subtitlesraise_subtitleswas used that bothraise_subtitlesandraise_subtitles_amountare deprecated, to usesub_marginsanddynamic_marginsinstead.Based on mpv-player/mpv#17544
Key difference:
sub-posto adjust subtitles, they usesub-margin-ysub-margin-ydoes not work with ASS subtitles unlesssub-ass-override=stripis used, which defeats the purpose of ASS subtitlessub-margin-ymight be affected bysub-use-marginscc @Keith94 as I know you wanted
osd_margins