From 44c398c3e133379e01f06c89fd78b6692694849c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Michaj=C5=82ow?= Date: Fri, 6 Oct 2023 20:32:45 +0200 Subject: [PATCH] Revert "video: remove another redundant wakeup" vo_still_displaying() is racey with vo_request_wakeup_on_done() and above that it doesn't work as expected. VO can run out of work and go to sleep for 1000s, while the play thread still returns on vo_still_displaying() check, because of a check `now < frame_end` so it never advances and go to sleep itself. This fixes dead lock that we have when image parameters changes during playback. This reverts commit 0c9ac5835be70ae26e4aa875e833fe2c7b3b3bf3. Fixes: #12575 --- player/video.c | 4 +--- video/out/vo.c | 60 ++++++++++++++++---------------------------------- video/out/vo.h | 1 - 3 files changed, 20 insertions(+), 45 deletions(-) diff --git a/player/video.c b/player/video.c index 4f68a97515..390cc26a9c 100644 --- a/player/video.c +++ b/player/video.c @@ -1160,10 +1160,8 @@ void write_video(struct MPContext *mpctx) struct mp_image_params *p = &mpctx->next_frames[0]->params; if (!vo->params || !mp_image_params_equal(p, vo->params)) { // Changing config deletes the current frame; wait until it's finished. - if (vo_still_displaying(vo)) { - vo_request_wakeup_on_done(vo); + if (vo_still_displaying(vo)) return; - } const struct vo_driver *info = mpctx->video_out->driver; char extra[20] = {0}; diff --git a/video/out/vo.c b/video/out/vo.c index 7b2e0976fe..0130ae4387 100644 --- a/video/out/vo.c +++ b/video/out/vo.c @@ -141,7 +141,6 @@ struct vo_internal { bool want_redraw; // redraw request from VO to player bool send_reset; // send VOCTRL_RESET bool paused; - bool wakeup_on_done; int queued_events; // event mask for the user int internal_events; // event mask for us @@ -760,41 +759,6 @@ void vo_wakeup(struct vo *vo) pthread_mutex_unlock(&in->lock); } -static bool still_displaying(struct vo *vo) -{ - struct vo_internal *in = vo->in; - int64_t now = mp_time_us(); - int64_t frame_end = 0; - if (in->current_frame) { - frame_end = in->current_frame->pts + MPMAX(in->current_frame->duration, 0); - if (in->current_frame->display_synced) - frame_end = in->current_frame->num_vsyncs > 0 ? INT64_MAX : 0; - } - return (now < frame_end || in->rendering || in->frame_queued) && in->hasframe; -} - -// Return true if there is still a frame being displayed (or queued). -bool vo_still_displaying(struct vo *vo) -{ - pthread_mutex_lock(&vo->in->lock); - bool res = still_displaying(vo); - pthread_mutex_unlock(&vo->in->lock); - return res; -} - -// Make vo issue a wakeup once vo_still_displaying() becomes true. -void vo_request_wakeup_on_done(struct vo *vo) -{ - struct vo_internal *in = vo->in; - pthread_mutex_lock(&vo->in->lock); - if (still_displaying(vo)) { - in->wakeup_on_done = true; - } else { - wakeup_core(vo); - } - pthread_mutex_unlock(&vo->in->lock); -} - // Whether vo_queue_frame() can be called. If the VO is not ready yet, the // function will return false, and the VO will call the wakeup callback once // it's ready. @@ -951,7 +915,6 @@ static bool render_frame(struct vo *vo) if (in->dropped_frame) { in->drop_count += 1; - wakeup_core(vo); } else { in->rendering = true; in->hasframe_rendered = true; @@ -1023,14 +986,11 @@ static bool render_frame(struct vo *vo) more_frames = true; pthread_cond_broadcast(&in->wakeup); // for vo_wait_frame() + wakeup_core(vo); done: if (!vo->driver->frame_owner) talloc_free(frame); - if (in->wakeup_on_done && !still_displaying(vo)) { - in->wakeup_on_done = false; - wakeup_core(vo); - } pthread_mutex_unlock(&in->lock); return more_frames; @@ -1217,6 +1177,24 @@ void vo_seek_reset(struct vo *vo) pthread_mutex_unlock(&in->lock); } +// Return true if there is still a frame being displayed (or queued). +// If this returns true, a wakeup some time in the future is guaranteed. +bool vo_still_displaying(struct vo *vo) +{ + struct vo_internal *in = vo->in; + pthread_mutex_lock(&vo->in->lock); + int64_t now = mp_time_us(); + int64_t frame_end = 0; + if (in->current_frame) { + frame_end = in->current_frame->pts + MPMAX(in->current_frame->duration, 0); + if (in->current_frame->display_synced) + frame_end = in->current_frame->num_vsyncs > 0 ? INT64_MAX : 0; + } + bool working = now < frame_end || in->rendering || in->frame_queued; + pthread_mutex_unlock(&vo->in->lock); + return working && in->hasframe; +} + // Whether at least 1 frame was queued or rendered since last seek or reconfig. bool vo_has_frame(struct vo *vo) { diff --git a/video/out/vo.h b/video/out/vo.h index e0b24d4cf7..ca120672f6 100644 --- a/video/out/vo.h +++ b/video/out/vo.h @@ -495,7 +495,6 @@ bool vo_is_ready_for_frame(struct vo *vo, int64_t next_pts); void vo_queue_frame(struct vo *vo, struct vo_frame *frame); void vo_wait_frame(struct vo *vo); bool vo_still_displaying(struct vo *vo); -void vo_request_wakeup_on_done(struct vo *vo); bool vo_has_frame(struct vo *vo); void vo_redraw(struct vo *vo); bool vo_want_redraw(struct vo *vo);