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

frame-step still flashes icon under certain conditions #44

Closed
musjj opened this issue Nov 26, 2020 · 19 comments
Closed

frame-step still flashes icon under certain conditions #44

musjj opened this issue Nov 26, 2020 · 19 comments

Comments

@musjj
Copy link

musjj commented Nov 26, 2020

On the latest version, frame-step can cause the pause/play icon to flash, if you hold down the corresponding shortcut for a long enough period. As you hold it down, the 'play' icon will eventually flash. Once you let it go, the 'pause' icon will then flash.
Is this intended behavior?
Strangely, this does not happen with frame-step-back.
Also, on certain short videos, frame-step will flash the pause/play icon even if you simply tap on the key once. (I can provide the video, if you want to investigate it further. But I believe these two issues are probably intertwined.)

@po5
Copy link
Contributor

po5 commented Nov 26, 2020

This was my intention behind #40, to move pause icon logic to a script binding much like flash-timeline, flash-volume and flash-speed so the user can choose when it flashes and when it doesn't (here, everywhere but frame seeks).

@darsain
Copy link
Contributor

darsain commented Nov 26, 2020

The icon is flashing because mpv is sending play/pause events on frame seek. But only sometimes, which is just weird...

It might be fixable by maybe waiting a couple milliseconds and flash the indicator only if the state didn't change back. Would need to research how fast are these events triggering.

But no promises, dunno when I'll have time to look into it.

@darsain
Copy link
Contributor

darsain commented Nov 27, 2020

All right, I've figured out why mpv sends those short play->pause events. When you hold the frame step key, mpv resumes playback for the duration it is pressed, which is a pretty nice feature, but it does fuck up pause indicator.

So I added a filter for short play/pause changes. Try it out and let me know if it works fine for you:

https://raw.githubusercontent.com/darsain/uosc/dev/uosc.lua

@musjj
Copy link
Author

musjj commented Nov 27, 2020

When you hold the frame step key, mpv resumes playback for the duration it is pressed, which is a pretty nice feature, but it does fuck up pause indicator.

I see, so that's why the same issue doesn't happen with frame-step-back.
Thanks I tried out the new version, but I still experience the same flash. I'm guessing this commit makes it so that it doesn't flash if you hold down the frame-step key for less than 0.08 seconds?

@darsain
Copy link
Contributor

darsain commented Nov 27, 2020

I'm guessing this commit makes it so that it doesn't flash if you hold down the frame-step key for less than 0.08 seconds?

Yes. Do you experience it always, or only sometimes? Because it's pretty much nonexistent for me on all videos I've tried. Even on my slow linux laptop.

@oe-d
Copy link

oe-d commented Nov 27, 2020

When you hold frame-step key, you unpause the video. That's why it flashes.
https://github.com/oe-d/control got a frame-step feature with customizable delay before unpausing. Maybe this will help?

@darsain
Copy link
Contributor

darsain commented Nov 27, 2020

When you hold frame-step key, you unpause the video. That's why it flashes.

Well of course, that just resumes the video, there is no way how to filter that out. I was asking about performing actual framesteps. But I agree that flashing the indicator when holding framestep button is undesirable.

I'm definitely not adding custom frame stepping commands. That just feels like I'm re-implementing mpv here.
And I also don't like how the filtering adds a delay on the indicator, making it feel less responsive.

I guess following the @po5 suggestion and adding flash-pause-indicator command as a secondary command to play/pause bindings is the only way how to get the ideal behavior. I wanted to solve this without increasing configuration surface but oh well.

darsain added a commit that referenced this issue Nov 27, 2020
Usage:

Set option `pause_indicator=manual`.

For flashing the indicator, add to bindings:

```
space  cycle pause; script-binding uosc/flash-pause-indicator
```

Or this for the static one:

```
space  cycle pause; script-binding uosc/decide-pause-indicator
```

ref #44, fixes #40
@darsain
Copy link
Contributor

darsain commented Nov 27, 2020

You can try it: https://raw.githubusercontent.com/darsain/uosc/dev/uosc.lua

Usage:

Set option pause_indicator=manual.

For flashing the indicator, add to bindings:

space  cycle pause; script-binding uosc/flash-pause-indicator

Or this for the static one:

space  cycle pause; script-binding uosc/decide-pause-indicator

@po5
Copy link
Contributor

po5 commented Nov 27, 2020

Works great, except that with uosc/decide-pause-indicator the osc doesn't always render when paused, requiring a mouse hover to update.

@darsain
Copy link
Contributor

darsain commented Nov 27, 2020

@po5 O.O can't replicate. I even added request_render() at the end of the decide command to prevent this

@musjj
Copy link
Author

musjj commented Nov 28, 2020

Thanks, the manual flash indicator really works well! This should solve all my original issues for now.
But I have the same problem with po5 for the static indicator. I need to move my mouse until any of the proximity-based GUI animates, only then will the static pause icon show up/disappear.

@darsain
Copy link
Contributor

darsain commented Nov 28, 2020

Can you give me steps/config to reproduce? This is not happening to me on any system or video file I've tried.

Also make sure you are on latest dev: https://raw.githubusercontent.com/darsain/uosc/dev/uosc.lua

@musjj
Copy link
Author

musjj commented Nov 28, 2020

I updated mpv and tried again with a new config from a scratch.
I grabbed a fresh uosc.lua and uosc.conf from the dev branch. Changed pause_indicator in uosc.conf to manual.
In my mpv.conf:

osc=no
osd-bar=no
border=no

In my input.conf:

space  cycle pause; script-binding uosc/decide-pause-indicator

That's it, basically.
I tried it on a whole bunch of different videos and they all exhibit the same behavior.
I'm on Windows btw, not sure if that changes anything.

Actually, a correction to my previous post. The static pause indicator can go away without any mouse movements. The mouse movement is only necessary for making it show up.

@darsain
Copy link
Contributor

darsain commented Nov 28, 2020

Oh, was able to replicate after I removed my ontop-when-playing script. I'll look into it later.

@darsain
Copy link
Contributor

darsain commented Nov 30, 2020

So after half an hour of head scratching and frustration I hopped into #mpv on freenode and a kind soul avih told me this is a know race condition bug on windows that happens during pause, where overlay update requests are sometimes being ignored (mpv-player/mpv#8350).

So the only thing I could do is add a 50ms timeout that asks mpv to update the overlay again. 30ms was still having issues, 40ms was fine, but to be safe I made it 50ms. Tell me if it works for you or if it needs to be increased.

https://raw.githubusercontent.com/darsain/uosc/dev/uosc.lua

@po5
Copy link
Contributor

po5 commented Dec 1, 2020

I was experiencing this on Linux so definitely not Windows-only.
May be a different vo/gpu-api combo.
This does fix it for me.

I don't know whether this is within the scope of this issue but frame-stepping forwards with pause_indicator=static sometimes flashes the video without a pause indicator, could be the same problem.
Here's a video of me frame-stepping: https://i.fiery.me/8aQrq.webm

@avih
Copy link

avih commented Dec 1, 2020

I was experiencing this on Linux so definitely not Windows-only.

Would you mind trying this:

  • Save this as pause.lua:
mp.observe_property("pause", "bool", function(n, v)
    mp.osd_message("pause: " .. (v and "yes" or "no"))
end)
  • run mpv --script=path/to/pause.lua someclip.mp4 (replace path and the clip file with the actual values)
  • Toggle pause with space several times without touching the mouse.

Report whether you only see the "pause: no" messages in addition to the "pause: yes" ones?

If you only see "pause: yes", would you mind posting a log of it (add --log-file=mylog.txt to the mpv command).

Thanks.

EDIT: I think it would be more appropriate to post your result at mpv-player/mpv#8350 . Thanks.

darsain added a commit that referenced this issue Dec 3, 2020
Usage:

Set option `pause_indicator=manual`.

For flashing the indicator, add to bindings:

```
space  cycle pause; script-binding uosc/flash-pause-indicator
```

Or this for the static one:

```
space  cycle pause; script-binding uosc/decide-pause-indicator
```

ref #44, fixes #40
@darsain darsain closed this as completed Dec 3, 2020
@hooke007
Copy link
Contributor

@tomasklaen I notice the workaround doesn't work any more.

@tomasklaen
Copy link
Owner

Is there anything more we can do apart of adding more time to that the timeout?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants