overlay:update() is ignored during pause events #8350
Comments
|
Can reproduce with this smaller test case: mp.observe_property("pause", "bool", function(n, v)
mp.osd_message("pause: " .. (v and "yes" or "no"))
end)It seems that when the ass overlay is updated from the pause observer right after it was changed from playing to paused doesn't get displayed. However, if the display is further delayed by some ms, then it does seem to work, e.g. mp.observe_property("pause", "bool", function(n, v)
mp.add_timeout(50/1000, function()
mp.osd_message("pause: " .. (v and "yes" or "no"))
end)
end)On my system I found out that a delay of 50ms (as above) seem to always work, while 10ms usually is not enough. This does suggest a possible race condition in mpv when a request to update the ASS overlay happens right after/when mpv is entering paused state. |
|
@darsain can you confirm that you only see the issue when you try to update the overlay from the pause observer? or does it also fail to update from other triggers? for instance I tried to update it from a 500ms repeating interval timer, and it always worked. |
|
I've not run into this in any other place yet, but I wasn't hunting for it either. |
|
Thanks, so for now we only know that it happens when trying to update the overlay right after/when the player gets paused. |
|
According to tomasklaen/uosc#44 (comment) this might not be windows only. |
|
Following up on tomasklaen/uosc#44 (comment) Attached are logs both with and without --no-config |
|
@po5 thanks. Does it get "fixed" if you use this instead at mp.observe_property("pause", "bool", function(n, v)
mp.add_timeout(100/1000, function()
mp.osd_message("pause: " .. (v and "yes" or "no"))
end)
end) |
|
It does "fix" it. Pausing no longer randomly skips "pause: yes", and there's only one case I've found where the message doesn't get displayed: |
Is that with the additional 100/1000 delay? or with the original script? |
|
Dang, had the words "With this script" in the original edit but removed it for whatever reason. |
|
:) thanks. I don't know when it will be looked at, but I'm trying to collect useful info if someone tries to tackle it. Obviously the additional delay is a workaround hack, but it also serves as an indication to the conditions which affect the issue. Thanks again. |
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
Maybe the title of this issue should be changed then? |
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
changed the title
|
I was typing up a big long post, but I discovered something unexpected while doing so I'll update on this later. But this is not windows-only. I can trivially reproduce it on linux. I only looked at the osd_msg case but overlays is probably similar. #9735 appears to fix this, but I think that's just an accident/band-aid. This absolutely is limited to only pause events though. I can say that for certain. When playback is active, the playloop will always force another redraw (duh), so this kind of race can only happen with pause. |
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.
|
Fixed by #11395. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Assuming:
Updating the
ov.dataand callingov:update()during pause event on windows will be ignored and not update the overlay.Reproduction steps
I've isolated the bug into this script: https://0x0.st/i7mf.lua
It renders a pause indicator when playback is paused.
Most of the time, the indicator won't be rendered when video is paused.
In my tests, at least 40ms delay is needed before calling
ov:update()for it to render consistently, but that might differ for others.Log file
https://0x0.st/i7mW.txt
The text was updated successfully, but these errors were encountered: