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

mpv.net breaks scripts that rely on mouse-up events #567

Closed
Sneakpeakcss opened this issue Oct 3, 2023 · 15 comments
Closed

mpv.net breaks scripts that rely on mouse-up events #567

Sneakpeakcss opened this issue Oct 3, 2023 · 15 comments
Assignees

Comments

@Sneakpeakcss
Copy link
Collaborator

Describe the bug
Initial discussion about this issue was created on uosc repo tomasklaen/uosc#615.

No matter the configuration, mpv.net seems to be blocking(?) mouse-up events and breaking scripts that rely on them, the mentioned uosc discussion has a video showcasing it in that case, but same issues happen in other scripts like crop.lua where the cropping action doesn't properly execute unless mpv.net is in fullscreen.

To Reproduce

Steps to reproduce the behavior:

  1. Install either uosc or crop.lua script
  2. Try to change volume by clicking on uosc's volume bar or initialize cropping(with hotkey) by clicking on video.

Expected behavior
In case of uosc: The volume bar shouldn't be locked to the mouse cursor like LMB is still being held after clicking on it.
In case of crop: The cropping overlay should initialize after using the hotkey+LMB rather than locking it in mouse-down state.

Screenshots

uosc.mpv.net.mp4

Additional context

All of those examples work without any issues when mpv.net is in fullscreen/maximized state, so it might be connected to dragging action, but i can't be sure since --no-window-dragging doesn't seem to do anything in mpv.net. It might also be connected to previously reported #533 issue

  1. mpv.net version v6.0.4.0-stable
  2. Windows version WIN10 21H2
@stax76
Copy link
Collaborator

stax76 commented Oct 5, 2023

Thanks for the detailed bug report and sorry about the late reaction. I hope it's not too hard to fix, I understand that uosc is very popular.

@ThomasEricB
Copy link

@stax76 I have the same issue, always had in fact.

@stax76
Copy link
Collaborator

stax76 commented Oct 20, 2023

I worked on this today and think it was successful.

@Sneakpeakcss
Copy link
Collaborator Author

I worked on this today and think it was successful.

I'll gladly test it if there's a test version ready to download.

@ThomasEricB
Copy link

I worked on this today and think it was successful.

Thanks!

@Sneakpeakcss
Copy link
Collaborator Author

Sneakpeakcss commented Oct 24, 2023

I worked on this today and think it was successful.

I've checked the fix using the build provided in this PR, and crop.lua works without issues, unless this version is used, but that's because switching set window-dragging no doesn't do anything in mpv.net.

On the other hand, uosc no longer has issues when clicking on the volume bar/uosc menu. However, trying to hold and drag any of them still holds mpv.net in dragging state. While hold+drag isn't expected for uosc menu (at least i can't think of any reasons to do that, so the issue is minimal), it still creates the same problem when it comes to the volume bar. Uosc doesn't seem to rely on switching off window-dragging like crop.lua, so this probably is a different problem.


Also, somewhat unrelated… I'm not sure if this was intentional, but changes in the newest version fixed 2 major issues i had with mpv:

mpv-player/mpv#12214 and mpv-player/mpv#1921

Previously, doubleclick on mpv.net behaved differently than in mpv by going fullscreen on mousedown compared to mouseup. This allowed me to do some janky workaround using AHK to send mouseup whenever i doubleclicked. However, in the upcoming mpv.net update, this has changed.

While mpv.net now uses mouseup for fullscreen, the two mentioned above issues do not exist at all when input-doubleclick-time is set to a very low value (input-doubleclick-time=1). It seems that in mpv.net this command works independently from the 'default' doubleclick behaviour, since if you set it very low in mpv, it stops responding because it expects impossibly quick doubleclick, but in mpv.net, it simply gets rid of 2 big problems without losing doubleclick functionality. So this is simply a cherry on top.

@stax76
Copy link
Collaborator

stax76 commented Oct 26, 2023

I added support for window-dragging=no

https://github.com/mpvnet-player/mpv.net/actions/runs/6656296808

What's now the biggest problem I can try to solve?

I do my best to get great compatibility and mpv devs always help, but in some cases it's difficult.

@Sneakpeakcss
Copy link
Collaborator Author

Sneakpeakcss commented Oct 26, 2023

I added support for window-dragging=no

https://github.com/mpvnet-player/mpv.net/actions/runs/6656296808

And from what i see, that PR version of crop.lua works perfectly now!

What's now the biggest problem I can try to solve?

I'm personally bothered by few, but i can't think of many big ones that are exclusive to mpv.net. However, the newest update introduced some, and depending how we look at it, they might be considered as 'compatibility breaking'.


  • There's this that i've mentioned in the passing, but this whole thing sounds like some Windows/AMD driver issues, so even if you can manage to reproduce that it still seems very out of scope for mpv.net.

  • There's an issue when seeking forward very fast (basically holding down keybind) on short videos (30s-3min~). Where suddenly the player exits/closes playback like it encountered the end of playlist, while not being close to it at all. This seems to happen before it even reaches end-of-file, or maybe when the 'next' seeking action would go past the available remaining time in the current video.
    This however, from what i remember also happens very very rarely in mpv, and for some reason happenes more often in mpv.net, so this might be completely upstream issue, and i haven't worked with the new update too much to say if it's any better now. In case of mpv it happenes so rarely that i was reluctant to even try opening an issue about it there.

  • When it comes to little things, it would be nice if mpv.net context window(RMB) followed the typical behaviour, where rightclicking again reopens the menu in new position rather than cycling it ON/OFF like it does currently, and also close it on ESC keypress.

  • Playlist Random was deprecated in this update, and now redirects to misc.lua script, but the script has an issue where it has a significant delay on keypress. It requires to wait around 1 second after keypress to go to another random video when you press it too fast. Previously the action was instant no matter the keypress speed.
     
    Reverting it back to:

    public static void PlaylistRandom()
    {
    int count = Core.GetPropertyInt("playlist-count");
    Core.SetPropertyInt("playlist-pos", new Random().Next(count));
    }
    seems to work like previously without any additional changes.
     
    I'm not sure if other functions from misc.lua also have that delay, but depending on the action it might be insignificant, because you don't expect them to be used in consecutive manner. Though, maybe this is fixable within the lua script itself, so reverting might be completely unnecessary.

  • mpv.net floods logfile (log-file=~~/mpvnetLogFile.txt) with hundreds/thousands of lines containing:

[ 2.467][d][cplayer] Run command: mouse, flags=73, args=[x="495", y="331", button="-1", mode="single"]


 

Few issues after the new Redesigned bindings and context menu.

In changelog it's mentioned that "Defining the context menu in input.conf is still supported, but discouraged and undocumented.", this created few issues/incompatibilities:

From what i understand, it's expected now to use Show input editor to define bindings or build context menu.

  • You can't use more than one modifier key when using Input editor, i.e. Ctrl+Alt+A is impossible to make.
  • Learn input popout only shows up when either command was already written, or you first activate name or command columns, and then jump back to it.
  • It would also be good if the popout had the typical "press any key" placeholder so it doesn't look like an empty box, but that's purely visual.
  • Input editor only creates the command properly if it's defined from right to left, trying to create a name first just bugs out after pressing enter, showing no changes in editor, but still creating an 'empty binding' at the end of `input.conf'.
2023-10-26_21-48-48.mp4

       And this is what shows up in input.conf when it's not defined in right->left order:

_           ignore
_           ignore
_           test
h           ignore
h           test
  • Another new(?) issue is simmilar to #538 . When any new bind is added through Input editor it removes any commented-out and empty lines, so not only it deletes users formating, it also removes any "turned off" bindings. Basically deleting anything that starts with # without any notification to the user. This makes input.conf file mostly exclusive to mpv.net, and has potential config loss for the user.
     
    image

       It even removes old separators in main context menu, but not submenus:

image

       also changes old #menu: syntax and replaces it with # menu: which potentailly adds up to another issue:

  • Uosc menu syntax is now broken for both uosc's compatibility #menu: and their native #! menu defining prefixes. Right now uosc doesn't even recognize it's own #! syntax, and outputs only it's own hardcoded default menu. This happens no matter if using old input.conf before Input editor goes through it removing stuff.

I'm also not 100% sure if all those Input editor issues are new, i've never used it before because i always did every editing manually in input.conf


 

And i'm not certain, but while mpv.net still has Customizable context menu defined in the same file as the key bindings in description, there's also this in changelog

Redesigned bindings and context menu, the default bindings and context menu are now defined internally, no longer is a default input.conf file generated. It means mpv.net no longer loses control over the default bindings and menu

does this mean that creating personal context menu functionality is mostly unsupported now, even through editor ? No matter what i've tried i couldn't create anything using the editor, since it just deletes anything in Name column after reopening:

image

After reopening:

image

Name column doesn't seem to do anything, even if you successfully create a keybind the name value doesn't exist in any form in input.conf. It's being stripped down to H cycle fullscreen


 

I do my best to get great compatibility and mpv devs always help, but in some cases it's difficult.

As a end user i can only imagine, and simply thank you for all that time spent on development.

@ThomasEricB
Copy link

@stax76 Thank you for fixing the bug with UOSC... I love that I can finally use it now... However, can you bring an option to enable mpv.net context menu with UOSC? I actually prefer using it over UOSC's menu!

@stax76
Copy link
Collaborator

stax76 commented Oct 28, 2023

@ThomasEricB

mpv.net in new versions uses normal input bindings to show the context menu:

MBTN_Right script-message-to mpvnet show-menu

I create isolated test configurations for every mpv script I want to test, so I don't have to modify my main mpv configuration when I want to test a script. I can come back any time to any test configuration to continue testing.

PowerShell code for my uosc test environment:

cd $PSScriptRoot
$Env:MPV_HOME = "$PSScriptRoot"
mpvnet --quiet ../video.mkv

This uosc configuration does not have an input.conf or mpv.conf file, the configuration is really minimal.

When I right-click it shows the mpv.net context menu and does nothing else, so it appears to be an issue on your side.

@ThomasEricB
Copy link

ThomasEricB commented Oct 28, 2023

so it appears to be an issue on your side.

Yep. I had to delete my old input.conf (it was acting up) and generate a new one to fix it

@stax76
Copy link
Collaborator

stax76 commented Oct 28, 2023

@Sneakpeakcss

typical behaviour, where rightclicking again reopens the menu in new position

mpv.net mixes two different UI frameworks, WinForms for the main window and WPF for the main window context menu, this design still makes sense. The problem is probably caused by mixing the UI frameworks, the window and UI systems are very complex, so it's very hard to figure out why it happens and how to fix it. The problem seems to be minor, so I don't spent a lot of time trying to fix it.

and also close it on ESC keypress

That was relatively easy to fix.

Playlist Random was deprecated

I've put it back, not in the manual or menu, just the code handling the command, for backward compatibility.

mpv.net floods logfile (log-file=~~/mpvnetLogFile.txt) with hundreds/thousands of lines containing:

I tried forwarding mouse window messages to mpv, this caused the osc to flicker and instantly hide when ever it is showed, I assumed that it is very hard to find out why it happens, so instead of forwarding mouse window messages, mpv.net sends input commands, this is the code:

            case 0x200: // WM_MOUSEMOVE
                if (Environment.TickCount - _lastCycleFullscreen > 500)
                {
                    Point pos = PointToClient(Cursor.Position);
                    Player.Command($"mouse {pos.X} {pos.Y}");
                }

Here is the documentation for the mouse input command:

https://mpv.io/manual/master/#command-interface-[%3Cmode%3E]]

For mpv frontends, such operations are probably normal, don't seem to cause problems, mpv probably logs any input command, mouse move generates a lot of commands. The reason why I remember this is I just recently did work in this area. Sometimes I look at the code of other frontends to see how they work.

For the bindings and context menu issues, I created a dedicated issue:

#572

@stax76
Copy link
Collaborator

stax76 commented Oct 31, 2023

I close it since it mostly works now, for the remaining issues I made a feature request here.

@stax76 stax76 closed this as completed Oct 31, 2023
@Sneakpeakcss
Copy link
Collaborator Author

I close it since it mostly works now, for the remaining issues I made a feature request here.

I only now noticed that the dragging deadzone to fix volume bar was already implemented, and that's because in uosc you can change its position from right side to left with:

# Where to display volume controls: none, left, right
volume=left

So this one is still having an issue. I'm not certain if it's possible to check that setting through mpv.net and switch the deadzone based on it, so this might require another chunk 'cut' on the left.

@stax76
Copy link
Collaborator

stax76 commented Nov 4, 2023

In the next build (hopefully available in 1-2 days), there will also be a dead zone on the left side, regardless of installed scripts.

Values are:

Left: 10%
Top: 10%, but only if border=no
Right: 10%
Bottom: 22%

I made a feature request regarding the problem on the mpv tracker here, and I explained it in the manual.

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

3 participants