Skip to content
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

Allow control over OSC logo #10201

Closed
stax76 opened this issue May 17, 2022 · 19 comments
Closed

Allow control over OSC logo #10201

stax76 opened this issue May 17, 2022 · 19 comments

Comments

@stax76
Copy link
Contributor

stax76 commented May 17, 2022

1. Completely hide the logo

Allow to completely hide the logo and logo caption via showlogo option.

Use cases:

  1. mpv.net covers the OSC logo with its own logo via overlay-add, which doesn't always work correctly.
  2. some users might not like the logo design.

2. Hide and show the logo via script message

Use cases:

There are many menu scripts that conflict with the logo, personally I use file-browser (excellent script) and ogm (on screen menu), but there must be many like various bookmark scripts etc. Disabling the osc temporarily altogether would not work because the initial state (always, never, auto) cannot be accessed and thus be restored.

3. Allow replacing the logo with an image file

Allow replacing the logo with an image file and scale option, default is 0.5 which is half the height of the window. mpc has such a feature (without scale option).

@Dudemanguy
Copy link
Member

Disabling the osc temporarily altogether would not work because the initial state (always, never, auto) cannot be accessed and thus be restored.

Hmm, how come? Can't you just save whatever the value of the osc property is first somewhere, turn it off, and then restore back the old value? Number 3 is not likely to happen. The mpv logo is actually just ASS that's compiled into the executable so it's not really setup for "user-friendly" tinkering. overlay-add is supposed to be good enough for this usecase though.

@stax76
Copy link
Contributor Author

stax76 commented May 18, 2022

Hmm, how come? Can't you just save whatever the value of the osc property is first somewhere, turn it off, and then restore back the old value?

From the client API I think I can only get the state after startup by reading the conf file and also the script-opts property, so it's a bit inconvenient, slow and don't cover the use case that the user might change the state after startup.

I proposed an okay solution here:

CogentRedTester/mpv-file-browser#55

There are probably many scripts with that issue, one of mine here:

https://github.com/stax76/mpv-scripts/blob/main/src/osm.lua

For 1., if there are no objections, I can build it and send a PR when I have time.

@CogentRedTester
Copy link
Contributor

CogentRedTester commented May 18, 2022

It seems like the OSC stores the current visibility state in the shared-script-properties property:

image

I tested and it gets updated when you cycle the visibility mode with a keybind.

Edit: yes, it does so in this line

@stax76
Copy link
Contributor Author

stax76 commented May 18, 2022

I didn't really know about shared-script-properties.

Your screenshot is very interesting, I didn't know it can be used in such ways. I have similar functionality in mpv.net, properties can be searched with the command palette and when the selected property is invoked the value is shown, and it can be selected in order to copy it to the clipboard.

aaa

bbb

@Dudemanguy
Copy link
Member

Dudemanguy commented May 18, 2022

Oh my mistake, I mixed up osc with osc-visibility (different things). Anyways, it looks like shared-script-properties (which I also didn't know about; the documentation is worded to be very negative for some reason) does what you need from this discussion? You can get the value and then write to if if you need to using script-message.

@stax76
Copy link
Contributor Author

stax76 commented May 18, 2022

shared-script-properties was helpful for the solution provided by CogentRedTester:

CogentRedTester/mpv-file-browser#55 (comment)

Still helpful would be osc options showlogo and also showdropmsg.

@Dudemanguy
Copy link
Member

How so? I mean it's not hard of course, but is there value in letting osc-visibility be on (always or auto) and having a hypothetical showlogo and showdropmsg both be false? Seems like that would be the same thing as putting osc-visibility to never.

@dyphire
Copy link
Contributor

dyphire commented May 19, 2022

How so? I mean it's not hard of course, but is there value in letting osc-visibility be on (always or auto) and having a hypothetical showlogo and showdropmsg both be false? Seems like that would be the same thing as putting osc-visibility to never.

The option of adding showlogo seems to be a good choice. It will allow users to customize simple logo scripts instead of modifying OSC scripts for this purpose.
At present, only opaque logo can be used for coverage osc logo.

@CogentRedTester
Copy link
Contributor

CogentRedTester commented May 19, 2022

From a quick look in the code and some brief testing it would probably be as simple as modifying the following code (and adding the option obviously):

Converting this:

mpv/player/lua/osc.lua

Lines 2575 to 2605 in f20dbcd

-- render idle message
msg.trace("idle message")
local icon_x, icon_y = 320 - 26, 140
local line_prefix = ("{\\rDefault\\an7\\1a&H00&\\bord0\\shad0\\pos(%f,%f)}"):format(icon_x, icon_y)
local ass = assdraw.ass_new()
-- mpv logo
for i, line in ipairs(logo_lines) do
ass:new_event()
ass:append(line_prefix .. line)
end
-- Santa hat
if is_december and not user_opts.greenandgrumpy then
for i, line in ipairs(santa_hat_lines) do
ass:new_event()
ass:append(line_prefix .. line)
end
end
ass:new_event()
ass:pos(320, icon_y+65)
ass:an(8)
ass:append("Drop files or URLs to play here.")
set_osd(640, 360, ass.text)
if state.showhide_enabled then
mp.disable_key_bindings("showhide")
mp.disable_key_bindings("showhide_wc")
state.showhide_enabled = false
end

To this:

        if (user_opts.hide_logo) then
            render_wipe()
        else
            -- render idle message
            msg.trace("idle message")
            local icon_x, icon_y = 320 - 26, 140
            local line_prefix = ("{\\rDefault\\an7\\1a&H00&\\bord0\\shad0\\pos(%f,%f)}"):format(icon_x, icon_y)

            local ass = assdraw.ass_new()
            -- mpv logo
            for i, line in ipairs(logo_lines) do
                ass:new_event()
                ass:append(line_prefix .. line)
            end

            -- Santa hat
            if is_december and not user_opts.greenandgrumpy then
                for i, line in ipairs(santa_hat_lines) do
                    ass:new_event()
                    ass:append(line_prefix .. line)
                end
            end

            ass:new_event()
            ass:pos(320, icon_y+65)
            ass:an(8)
            ass:append("Drop files or URLs to play here.")
            set_osd(640, 360, ass.text)
        end

        -- I admit I don't know what this is for, so I'm not sure if it can be disabled too
        if state.showhide_enabled then
            mp.disable_key_bindings("showhide")
            mp.disable_key_bindings("showhide_wc")
            state.showhide_enabled = false
        end

Edit: I used a hide_logo option but showlogo might fit the naming scheme better

@Dudemanguy
Copy link
Member

The option of adding showlogo seems to be a good choice. It will allow users to customize simple logo scripts instead of modifying OSC scripts for this purpose.

But you can just send a script message to the osc to turn off the visibility and then show your logo. I don't see what the difference is in practice. Am I just missing something here?

@CogentRedTester
Copy link
Contributor

But you can just send a script message to the osc to turn off the visibility and then show your logo. I don't see what the difference is in practice. Am I just missing something here?

Users who don't want a logo at all (maybe the same people who want to disable the Santa hat).

I guess the advantage is that you don't have to worry about restoring the OSC visibility to the same value or about overwriting other script-opts, you can just enable/disable the logo and no other OSC behaviour would be disabled. See my comment here for an analysis of what can go wrong when using the visibility option: CogentRedTester/mpv-file-browser#55 (comment).

@Dudemanguy
Copy link
Member

I see, that was informative. We don't like touching the osc much because it's complicated but this part is thankfully trivial.

@dyphire
Copy link
Contributor

dyphire commented May 19, 2022

But you can just send a script message to the osc to turn off the visibility and then show your logo. I don't see what the difference is in practice. Am I just missing something here?

I just don't think should control the visibility of osc in other scripts, because users may use other osc scripts. It is better to implement the corresponding options by osc script(Including other user's osc script).

In fact, after seeing this issue, I added corresponding options of showlogo and showtext to my osc script.
https://github.com/dyphire/mpv-config/blob/b0ca34347f30871cfddf3a710e6039e2571d7070/scripts/thumbnailer_osc.lua#L3422-L3444

Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue May 19, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. While you can use script-message and
shared-script-properties (highly discouraged in the documentation for
some reason) to work around this, there's still some brittle edge cases
and complicated logic that can pop up. Adding an option for these two
things (the logo and the "drop files..." text) is really trivial and
would make things easier for scripts. Discussed in a couple of issues
below:
mpv-player#10201
CogentRedTester/mpv-file-browser#55
@Dudemanguy
Copy link
Member

Made a PR for this. I made the actual logo and the message text (Drop files...) separate options since the santa hat is already a separate option.

Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue May 19, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. While you can use script-message and
shared-script-properties (highly discouraged in the documentation for
some reason) to work around this, there's still some brittle edge cases
and complicated logic that can pop up. Adding an option for these two
things (the logo and the "drop files..." text) is really trivial and
would make things easier for scripts. Discussed in a couple of issues
below:
mpv-player#10201
CogentRedTester/mpv-file-browser#55
@stax76
Copy link
Contributor Author

stax76 commented May 19, 2022

Made a PR for this. I made the actual logo and the message text (Drop files...) separate options since the santa hat is already a separate option.

With that, I can add the mpv.net overlay logo without having to worry that the osc logo is still sometimes visible, thanks!

Regarding the overlap problem, the auto profile solution works well and is interesting as it shows advanced mpv concepts, the downside is that it requires user configuration. Here is another idea without user configuration:

mpv would provide a new property screen-owner, if a client wants to show a large and persistent overlay, he should set this to his client name before showing the overlay, and clear it after closing the overlay. Clients should observe this property and hide or exit if somebody else takes ownership.

If this could work, maybe provide this too, if it's not difficult to build, script authors have then a choice to decide which model they prefer and support.

@stax76
Copy link
Contributor Author

stax76 commented May 19, 2022

My last idea tried to solve a problem that doesn't even exist, the problem at hand is really only that an overlay script can overlap with the osc logo. So here is the next idea:

A new osc message called osc-hidelogo:

script-message osc-hidelogo

Hides the osc logo until next time times the player enters idle state.
This is useful for scripts that show large and permanent overlays.

This way, scripts that show overlays only have to send the message before adding the overlay, no action is needed closing the overlay. mpv.net and other osc scripts can easily receive the message and act upon.

@Dudemanguy
Copy link
Member

Dudemanguy commented May 19, 2022

For something like that, I think we could follow the script-message osc-visibility model which does alter the value of user_opts.visibility. I think this one would just be a straight on/off. And osc-logovisibility (or something like that) would hide everything unconditionally (mpv logo, text and santa hat). The only thing is, though, if I add that to my PR above, I'd be tempted to just collapse showlogo and showidlemsg into one single option that turns it all off and on for simplicity's sake. I'll think about it.

Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue May 24, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlelogo option
disables all logo related things (including the santa hat) if it is set
to "no". A new script message (osc-idlelogo) is also added so users and
scripts can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found on the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue May 25, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlelogo option
disables all logo related things (including the santa hat) if it is set
to "no". A new script message (osc-idlelogo) is also added so users and
scripts can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found on the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue May 29, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlelogo option
disables all logo related things (including the santa hat) if it is set
to "no". A new script message (osc-idlelogo) is also added so users and
scripts can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found on the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue May 30, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlelogo option
disables all logo related things (including the santa hat) if it is set
to "no". A new script message (osc-idlelogo) is also added so users and
scripts can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found on the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue Jun 1, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlelogo option
disables all logo related things (including the santa hat) if it is set
to "no". A new script message (osc-idlelogo) is also added so users and
scripts can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found on the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue Jun 3, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlescreen option
disables all idlescreen related things (including the santa hat) if it
is set to "no". A new script message (osc-idlescreen) is also added so
users can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found in the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue Jun 4, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlescreen option
disables all idlescreen related things (including the santa hat) if it
is set to "no". A new script message (osc-idlescreen) is also added so
users can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found in the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Dudemanguy added a commit that referenced this issue Jun 4, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlescreen option
disables all idlescreen related things (including the santa hat) if it
is set to "no". A new script message (osc-idlescreen) is also added so
users can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found in the
below github issues.
#10201
CogentRedTester/mpv-file-browser#55
@Dudemanguy
Copy link
Member

@stax76: Is it okay to close this one now? I know the third request isn't technically fulfilled.

@stax76
Copy link
Contributor Author

stax76 commented Jun 7, 2022

Yes, it's fine, thank you for improving the osc, I close it.

@stax76 stax76 closed this as completed Jun 7, 2022
po5 pushed a commit to po5/thumbfast that referenced this issue Sep 18, 2022
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlescreen option
disables all idlescreen related things (including the santa hat) if it
is set to "no". A new script message (osc-idlescreen) is also added so
users can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found in the
below github issues.
mpv-player/mpv#10201
CogentRedTester/mpv-file-browser#55
dyphire pushed a commit to dyphire/mpv that referenced this issue Feb 22, 2023
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlescreen option
disables all idlescreen related things (including the santa hat) if it
is set to "no". A new script message (osc-idlescreen) is also added so
users can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found in the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
dyphire pushed a commit to dyphire/mpv that referenced this issue Feb 22, 2023
This is mainly for other user scripts that may conflict with the osc
logo in some way. Although it is possible to listen for
shared-script-properties, this has many edge cases that could easily pop
up. A user could want other OSC things to happen at the same time (say
osc-message). They just don't want the logo. The idlescreen option
disables all idlescreen related things (including the santa hat) if it
is set to "no". A new script message (osc-idlescreen) is also added so
users can easily toggle the value (passing "cycle" or just explictly
setting "yes" or "no"). Some more discussion on this is found in the
below github issues.
mpv-player#10201
CogentRedTester/mpv-file-browser#55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants