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

OSD tends not to update for a short period of time after a pause. #8172

Closed
Solarunit opened this issue Oct 13, 2020 · 8 comments · Fixed by #11395
Closed

OSD tends not to update for a short period of time after a pause. #8172

Solarunit opened this issue Oct 13, 2020 · 8 comments · Fixed by #11395

Comments

@Solarunit
Copy link

  • mpv, version: 0.32.0
  • Solus Budgie
  • Source of the mpv binary: Solus Software Center

Reproduction steps

When you add osd-msg for the Pause key in input.conf, e.g.:

SPACE osd-msg cycle pause

OSD tends not to update for a short period of time after a pause.

The osd-msg will show when you drag the mouse and the bottom control panel will appear. (see gif)

mpv

The issue was found here #8155

@oe-d
Copy link

oe-d commented Oct 14, 2020

Related osd rendering issue at #8107

@avih
Copy link
Member

avih commented Oct 14, 2020

You didn't provide a log as the template requested, so it's hard to tell, but by default the OSC mouse-movement-detection-area is the bottom half of the screen, and in your animated gif the OSC seems to show up exactly when the mouse goes to the bottom half of the window.

Next time, please follow the issue template even if you think it's not required. It's required at the template exactly because many people think, wrongly, that it's not required.

@Akemi
Copy link
Member

Akemi commented Apr 29, 2021

closing this for now. if the issue still persists please open a new issue with all the info we ask for.

@Akemi Akemi closed this as completed Apr 29, 2021
@avih
Copy link
Member

avih commented Apr 29, 2021

SPACE osd-msg cycle pause
OSD tends not to update for a short period of time after a pause.

This is still reproducible by me with mpv-master, at least on Windows.

Doesn't happen in idle (when the the "Drop files or URLs..." message shows), but it does happen to me when a video is loaded.

This issue has existed for a while on Windows, but I wasn't aware it's reproducible on linux too. We suspect some race condition, but it hasn't been pinpointed so far.

@Akemi
Copy link
Member

Akemi commented Apr 29, 2021

some extra info. i wasn't able to reproduce this issue on macOS.

@Akemi
Copy link
Member

Akemi commented Apr 29, 2021

seems like the script from this comment #8155 (comment) worked around the issue. it might be an indication for the source of the problem.

@Akemi Akemi added the core:osd label Apr 29, 2021
@avih
Copy link
Member

avih commented Apr 29, 2021

The script, or anything which updates the OSD in any way makes it show. mpv does take the string, but simply doesn't display it right away, but does display it once anything changes at the OSD - like a script which adds a new overlay few ms later (like moving the mouse which shows the OSC - and now the earlier pause OSD message finally shows up too).

That's probably because OSD updates are throttled differently when pauses/playing, and also in sync with the video frames, and changing the OSD output at exactly the same time as toggling pause seem to hit some race condition.

dexeonify added a commit to dexeonify/mpv-config that referenced this issue Dec 29, 2021
If you pause or resume the video too quickly, either by clicking the
pause/play button or through keypress, the OSC does not update itself
immediately. This workaround adds a small delay after pausing so that
the OSC can update itself promptly.

Original commit from: cyl0/ModernX@8cd17a3
Code taken from here: mpv-player/mpv#8155 (comment)
Related issues:
mpv-player/mpv#8172
mpv-player/mpv#8350
chengsun added a commit to chengsun/mpv that referenced this issue Jan 21, 2022
This manifested most frequently as a bug where the OSD fails to update
after pausing or single-stepping forwards (issues mpv-player#8172, mpv-player#8350).

In `render_frame`, the vo thread takes the lock once before rendering
the frame, and once after. The lock is relinquished whilst the frame is
actually rendering.

The redraw_request flag was being cleared whilst holding the lock
*after* the video frame has been rendered; however this frame was
rendered according to the request fetched whilst holding the lock the
first time. If the OSD was updated during the rendering of this frame,
the redraw request is cleared once the frame finishes rendering even
though the updated OSD is not included in the rendered frame.
chengsun added a commit to chengsun/mpv that referenced this issue Jan 21, 2022
This manifested most frequently as a bug where the OSD fails to update
after pausing or single-stepping forwards (issues mpv-player#8172, mpv-player#8350).

In `render_frame`, the vo thread takes the lock once before rendering
the frame, and once after. The lock is relinquished whilst the frame is
actually rendering.

The `redraw_request` flag was being cleared whilst holding the lock
*after* the video frame has been rendered; however this frame was
rendered according to the request fetched whilst holding the lock the
first time. If the OSD was updated during the rendering of this frame,
the redraw request is cleared once the frame finishes rendering even
though the updated OSD is not included in the rendered frame.

This patch fixes the race condition by clearing the `redraw_request`
flag *before* the video frame rendering starts. That way, a redraw
request that comes in during frame rendering will not be cleared without
another call to `render_frame` or `do_redraw`.
chengsun added a commit to chengsun/mpv that referenced this issue Jan 21, 2022
This manifested most frequently as a bug where the OSD fails to update
after pausing or single-stepping forwards (issues mpv-player#8172, mpv-player#8350).

In `render_frame`, the vo thread takes the lock once before rendering
the frame, and once after. The lock is relinquished whilst the frame is
actually rendering.

The `redraw_request` flag was being cleared whilst holding the lock
*after* the video frame has been rendered; however this frame was
rendered according to the request fetched whilst holding the lock the
first time. If the OSD was updated during the rendering of this frame,
the redraw request is cleared once the frame finishes rendering even
though the updated OSD is not included in the rendered frame.

This patch fixes the race condition by clearing the `redraw_request`
flag *before* the video frame rendering starts. That way, a redraw
request that comes in during frame rendering will not be cleared without
another call to `render_frame` or `do_redraw`. However, we need to
ensure that if the frame is dropped, we set `redraw_request` again if it
was previously set. See the newly added comment in the code.
chengsun added a commit to chengsun/mpv that referenced this issue Jan 21, 2022
This manifested most frequently as a bug where the OSD fails to update
after pausing or single-stepping forwards (issues mpv-player#8172, mpv-player#8350).

In `render_frame`, the vo thread takes the lock once before rendering
the frame, and once after. The lock is relinquished whilst the frame is
actually rendering.

The `redraw_request` flag was being cleared whilst holding the lock
*after* the video frame has been rendered; however this frame was
rendered according to the request fetched whilst holding the lock the
first time. If the OSD was updated during the rendering of this frame,
the redraw request is cleared once the frame finishes rendering even
though the updated OSD is not included in the rendered frame.

This patch fixes the race condition by clearing the `redraw_request`
flag *before* the video frame rendering starts. That way, a redraw
request that comes in during frame rendering will not be cleared without
another call to `render_frame` or `do_redraw`. However, we need to
ensure that if the frame is dropped, we set `redraw_request` again if it
was previously set. See the newly added comment in the code.
chengsun added a commit to chengsun/mpv that referenced this issue Jan 21, 2022
This manifested most frequently as a bug where the OSD fails to update
after pausing or single-stepping forwards (issues mpv-player#8172, mpv-player#8350).

In `render_frame`, the vo thread takes the lock once before rendering
the frame, and once after. The lock is relinquished whilst the frame is
actually rendering.

The `redraw_request` flag was being cleared whilst holding the lock
*after* the video frame has been rendered; however this frame was
rendered according to the request fetched whilst holding the lock the
first time. If the OSD was updated during the rendering of this frame,
the redraw request is cleared once the frame finishes rendering even
though the updated OSD is not included in the rendered frame.

This patch fixes the race condition by clearing the `redraw_request`
flag *before* the video frame rendering starts. That way, a redraw
request that comes in during frame rendering will not be cleared without
another call to `render_frame` or `do_redraw`. However, we need to
ensure that if the frame is dropped, we set `redraw_request` again if it
was previously set. See the newly added comment in the code.
avih pushed a commit to avih/mpv that referenced this issue Jan 25, 2022
This manifested most frequently as a bug where the OSD fails to update
after pausing or single-stepping forwards (issues mpv-player#8172, mpv-player#8350).

In `render_frame`, the vo thread takes the lock once before rendering
the frame, and once after. The lock is relinquished whilst the frame is
actually rendering.

The redraw_request flag was being cleared whilst holding the lock
*after* the video frame has been rendered; however this frame was
rendered according to the request fetched whilst holding the lock the
first time. If the OSD was updated during the rendering of this frame,
the redraw request is cleared once the frame finishes rendering even
though the updated OSD is not included in the rendered frame.
@Dudemanguy
Copy link
Member

Dudemanguy commented Mar 1, 2023

Just to throw some potential weirdness on this one, I can reproduce the issue in wayland but not in x11. Nothing springs to mind why that would happen.

Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue Mar 2, 2023
There is a very subtle race in vo that can manifest itself on pause
events. In the renderloop, render_frame, unsurprisingly, does the heavy
lifting of actually queuing up and flipping the frames. This is called
during normal playback. Sometimes various parts of the player can make a
redraw request which will latter trigger another render of the frame
later down in the loop (do_redraw). Because these requests can happen at
essentially anytime, sometimes the redraw request will happen *before*
do_redraw and it'll be caught in render_frame. When this happens,
the existing render_frame run works perfectly fine as a redraw so it
clears out the request which is sensible. Normally this is all locked of
course, but there's one catch. render_frame has to unlock itself when
propagating down into specific VOs/backends. That's what causes this
bug.

While render_frame is unlocked, other parts of the player can send
redraw requests which will cause in->request_redraw to become true. The
logic in the code always clears out this parameter after a successful
render, but this isn't correct. When in->request_become becomes true in
the middle of render_frame, there needs to be one more draw afterwards
to reflect whatever actually changed (usually the OSD). Instead, this
gets simply discarded. If you rapidly spam pause while rendering things
to the OSD at the same time, it's possible to for the last render to
behind a frame and appear as if your osd event was ignored.

Once you realize what is happening, the fix is quite simple. Just store
the initial value of in->request_redraw before the unlock step. After we
do the render step and unlock again, only set in->request_redraw to
false if there was an initial redraw request. We just finished doing a
redraw, so it is safe to clear. Otherwise, leave in->request_redraw
alone. If it is initially false, then it will still be false and nothing
changes. However if it updated to true in the middle of the rendering,
this value is now preserved so we can go and call do_redraw later and
show what that last frame was meant to be when you pause. One
unfortunate thing about this design is that it is technically possible
for other internal things in vo to update during that unlocked period.
Hopefully, that doesn't actually happen and only redraw requests work
like this.

Fixes mpv-player#8172 and mpv-player#9735.
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue Mar 2, 2023
There is a very subtle race in vo that can manifest itself on pause
events. In the renderloop, render_frame, unsurprisingly, does the heavy
lifting of actually queuing up and flipping the frames. This is called
during normal playback. Sometimes various parts of the player can make a
redraw request which will latter trigger another render of the frame
later down in the loop (do_redraw). Because these requests can happen at
essentially anytime, sometimes the redraw request will happen *before*
do_redraw and it'll be caught in render_frame. When this happens,
the existing render_frame run works perfectly fine as a redraw so it
clears out the request which is sensible. Normally this is all locked of
course, but there's one catch. render_frame has to unlock itself when
propagating down into specific VOs/backends. That's what causes this
bug.

While render_frame is unlocked, other parts of the player can send
redraw requests which will cause in->request_redraw to become true. The
logic in the code always clears out this parameter after a successful
render, but this isn't correct. When in->request_become becomes true in
the middle of render_frame, there needs to be one more draw afterwards
to reflect whatever actually changed (usually the OSD). Instead, this
gets simply discarded. If you rapidly spam pause while rendering things
to the OSD at the same time, it's possible to for the last render to
behind a frame and appear as if your osd event was ignored.

Once you realize what is happening, the fix is quite simple. Just store
the initial value of in->request_redraw before the unlock step. After we
do the render step and unlock again, only set in->request_redraw to
false if there was an initial redraw request. We just finished doing a
redraw, so it is safe to clear. Otherwise, leave in->request_redraw
alone. If it is initially false, then it will still be false and nothing
changes. However if it updated to true in the middle of the rendering,
this value is now preserved so we can go and call do_redraw later and
show what that last frame was meant to be when you pause. One
unfortunate thing about this design is that it is technically possible
for other internal things in vo to update during that unlocked period.
Hopefully, that doesn't actually happen and only redraw requests work
like this.

Fixes mpv-player#8172 and mpv-player#8350.
Dudemanguy added a commit to Dudemanguy/mpv that referenced this issue Mar 2, 2023
There is a very subtle race in vo that can manifest itself on pause
events. In the renderloop, render_frame, unsurprisingly, does the heavy
lifting of actually queuing up and flipping the frames. This is called
during normal playback. Sometimes various parts of the player can make a
redraw request which will latter trigger another render of the frame
later down in the loop (do_redraw). Because these requests can happen at
essentially anytime, sometimes the redraw request will happen *before*
do_redraw and it'll be caught in render_frame. When this happens,
the existing render_frame run works perfectly fine as a redraw so it
clears out the request which is sensible. Normally this is all locked of
course, but there's one catch. render_frame has to unlock itself when
propagating down into specific VOs/backends. That's what causes this
bug.

While render_frame is unlocked, other parts of the player can send
redraw requests which will cause in->request_redraw to become true. The
logic in the code always clears out this parameter after a successful
render, but this isn't correct. When in->request_become becomes true in
the middle of render_frame, there needs to be one more draw afterwards
to reflect whatever actually changed (usually the OSD). Instead, this
gets simply discarded. If you rapidly spam pause while rendering things
to the OSD at the same time, it's possible to for the last render to
be behind a frame and appear as if your osd event was ignored.

Once you realize what is happening, the fix is quite simple. Just store
the initial value of in->request_redraw before the unlock step. After we
do the render step and unlock again, only set in->request_redraw to
false if there was an initial redraw request. We just finished doing a
redraw, so it is safe to clear. Otherwise, leave in->request_redraw
alone. If it is initially false, then it will still be false and nothing
changes. However if it updated to true in the middle of the rendering,
this value is now preserved so we can go and call do_redraw later and
show what that last frame was meant to be when you pause. One
unfortunate thing about this design is that it is technically possible
for other internal things in vo to update during that unlocked period.
Hopefully, that doesn't actually happen and only redraw requests work
like this.

Fixes mpv-player#8172 and mpv-player#8350.
Dudemanguy added a commit that referenced this issue Mar 2, 2023
There is a very subtle race in vo that can manifest itself on pause
events. In the renderloop, render_frame, unsurprisingly, does the heavy
lifting of actually queuing up and flipping the frames. This is called
during normal playback. Sometimes various parts of the player can make a
redraw request which will latter trigger another render of the frame
later down in the loop (do_redraw). Because these requests can happen at
essentially anytime, sometimes the redraw request will happen *before*
do_redraw and it'll be caught in render_frame. When this happens,
the existing render_frame run works perfectly fine as a redraw so it
clears out the request which is sensible. Normally this is all locked of
course, but there's one catch. render_frame has to unlock itself when
propagating down into specific VOs/backends. That's what causes this
bug.

While render_frame is unlocked, other parts of the player can send
redraw requests which will cause in->request_redraw to become true. The
logic in the code always clears out this parameter after a successful
render, but this isn't correct. When in->request_become becomes true in
the middle of render_frame, there needs to be one more draw afterwards
to reflect whatever actually changed (usually the OSD). Instead, this
gets simply discarded. If you rapidly spam pause while rendering things
to the OSD at the same time, it's possible to for the last render to
be behind a frame and appear as if your osd event was ignored.

Once you realize what is happening, the fix is quite simple. Just store
the initial value of in->request_redraw before the unlock step. After we
do the render step and unlock again, only set in->request_redraw to
false if there was an initial redraw request. We just finished doing a
redraw, so it is safe to clear. Otherwise, leave in->request_redraw
alone. If it is initially false, then it will still be false and nothing
changes. However if it updated to true in the middle of the rendering,
this value is now preserved so we can go and call do_redraw later and
show what that last frame was meant to be when you pause. One
unfortunate thing about this design is that it is technically possible
for other internal things in vo to update during that unlocked period.
Hopefully, that doesn't actually happen and only redraw requests work
like this.

Fixes #8172 and #8350.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants