[vlc-devel] [PATCH v4 16/23] vout/control: move the lock outside of the control API

Steve Lhomme robux4 at ycbcr.xyz
Fri Aug 21 11:59:34 CEST 2020


The lock still needs to wait for incoming controls before the deadline.

The picture_fifo is always called under the control_lock for consistency with
the conditional variable state.
---
 src/video_output/control.c      | 24 ++--------------------
 src/video_output/control.h      |  5 +----
 src/video_output/video_output.c | 35 ++++++++++++++++++++++-----------
 3 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/src/video_output/control.c b/src/video_output/control.c
index 55e43cbfa47..0d2912dee6d 100644
--- a/src/video_output/control.c
+++ b/src/video_output/control.c
@@ -39,7 +39,6 @@ void vout_control_cmd_Init(vout_control_cmd_t *cmd, int type)
 /* */
 void vout_control_Init(vout_control_t *ctrl)
 {
-    vlc_mutex_init(&ctrl->lock);
     vlc_cond_init(&ctrl->wait_request);
 
     ctrl->is_dead = false;
@@ -55,27 +54,21 @@ void vout_control_Clean(vout_control_t *ctrl)
 
 void vout_control_Dead(vout_control_t *ctrl)
 {
-    vlc_mutex_lock(&ctrl->lock);
     ctrl->is_dead = true;
-    vlc_mutex_unlock(&ctrl->lock);
 }
 
 void vout_control_Push(vout_control_t *ctrl, vout_control_cmd_t *cmd)
 {
-    vlc_mutex_lock(&ctrl->lock);
     if (!ctrl->is_dead) {
         ARRAY_APPEND(ctrl->cmd, *cmd);
         vlc_cond_signal(&ctrl->wait_request);
     }
-    vlc_mutex_unlock(&ctrl->lock);
 }
 
 void vout_control_Wake(vout_control_t *ctrl)
 {
-    vlc_mutex_lock(&ctrl->lock);
     ctrl->can_sleep = false;
     vlc_cond_signal(&ctrl->wait_request);
-    vlc_mutex_unlock(&ctrl->lock);
 }
 
 void vout_control_PushVoid(vout_control_t *ctrl, int type)
@@ -86,25 +79,13 @@ void vout_control_PushVoid(vout_control_t *ctrl, int type)
     vout_control_Push(ctrl, &cmd);
 }
 
-void vout_control_Hold(vout_control_t *ctrl)
-{
-    vlc_mutex_lock(&ctrl->lock);
-}
-
-void vout_control_Release(vout_control_t *ctrl)
-{
-    vlc_mutex_unlock(&ctrl->lock);
-}
-
 int vout_control_Pop(vout_control_t *ctrl, vout_control_cmd_t *cmd,
-                     vlc_tick_t deadline)
+                     vlc_tick_t deadline, vlc_mutex_t *lock)
 {
-    vlc_mutex_lock(&ctrl->lock);
-
     if (ctrl->cmd.i_size <= 0) {
         /* Spurious wakeups are perfectly fine */
         if (deadline != INVALID_DEADLINE && ctrl->can_sleep) {
-            vlc_cond_timedwait(&ctrl->wait_request, &ctrl->lock, deadline);
+            vlc_cond_timedwait(&ctrl->wait_request, lock, deadline);
         }
     }
 
@@ -117,7 +98,6 @@ int vout_control_Pop(vout_control_t *ctrl, vout_control_cmd_t *cmd,
         has_cmd = false;
         ctrl->can_sleep = true;
     }
-    vlc_mutex_unlock(&ctrl->lock);
 
     return has_cmd ? VLC_SUCCESS : VLC_EGENERIC;
 }
diff --git a/src/video_output/control.h b/src/video_output/control.h
index a635be29e50..e06f4739c3b 100644
--- a/src/video_output/control.h
+++ b/src/video_output/control.h
@@ -43,7 +43,6 @@ typedef struct {
 void vout_control_cmd_Init(vout_control_cmd_t *, int type);
 
 typedef struct {
-    vlc_mutex_t lock;
     vlc_cond_t  wait_request;
 
     /* */
@@ -62,13 +61,11 @@ void vout_control_WaitEmpty(vout_control_t *);
 void vout_control_Push(vout_control_t *, vout_control_cmd_t *);
 void vout_control_PushVoid(vout_control_t *, int type);
 void vout_control_Wake(vout_control_t *);
-void vout_control_Hold(vout_control_t *);
-void vout_control_Release(vout_control_t *);
 
 /* control inside of the vout thread */
 #define INVALID_DEADLINE   ((vlc_tick_t) INT64_MAX)
 
-int vout_control_Pop(vout_control_t *, vout_control_cmd_t *, vlc_tick_t deadline);
+int vout_control_Pop(vout_control_t *, vout_control_cmd_t *, vlc_tick_t deadline, vlc_mutex_t *lock);
 void vout_control_Dead(vout_control_t *);
 
 #endif
diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
index 0a32a2d461d..61bdc12bd05 100644
--- a/src/video_output/video_output.c
+++ b/src/video_output/video_output.c
@@ -120,6 +120,7 @@ typedef struct vout_thread_sys_t
     vlc_blender_t   *spu_blend;
 
     /* Thread & synchronization */
+    vlc_mutex_t     control_lock;
     vout_control_t  control;
     vlc_thread_t    thread;
 
@@ -404,7 +405,9 @@ void vout_MouseState(vout_thread_t *vout, const vlc_mouse_t *mouse)
     sys->mouse = cmd.mouse;
     vlc_mutex_unlock(&sys->display_lock);
 
+    vlc_mutex_lock(&sys->control_lock);
     vout_control_Push(&sys->control, &cmd);
+    vlc_mutex_unlock(&sys->control_lock);
 
     if (moved)
         var_SetCoords(vout, "mouse-moved", cmd.mouse.i_x, cmd.mouse.i_y);
@@ -498,8 +501,10 @@ void vout_PutPicture(vout_thread_t *vout, picture_t *picture)
     vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
     assert(!sys->dummy);
     picture->p_next = NULL;
+    vlc_mutex_lock(&sys->control_lock);
     picture_fifo_Push(sys->decoder_fifo, picture);
     vout_control_Wake(&sys->control);
+    vlc_mutex_unlock(&sys->control_lock);
 }
 
 /* */
@@ -1534,7 +1539,7 @@ void vout_ChangePause(vout_thread_t *vout, bool is_paused, vlc_tick_t date)
     vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
     assert(!sys->dummy);
 
-    vout_control_Hold(&sys->control);
+    vlc_mutex_lock(&sys->control_lock);
     assert(!sys->pause.is_on || !is_paused);
 
     if (sys->pause.is_on)
@@ -1545,7 +1550,7 @@ void vout_ChangePause(vout_thread_t *vout, bool is_paused, vlc_tick_t date)
     }
     sys->pause.is_on = is_paused;
     sys->pause.date  = date;
-    vout_control_Release(&sys->control);
+    vlc_mutex_unlock(&sys->control_lock);
 
     vlc_mutex_lock(&sys->window_lock);
     vout_window_SetInhibition(sys->display_cfg.window, !is_paused);
@@ -1594,9 +1599,9 @@ void vout_Flush(vout_thread_t *vout, vlc_tick_t date)
     vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
     assert(!sys->dummy);
 
-    vout_control_Hold(&sys->control);
+    vlc_mutex_lock(&sys->control_lock);
     vout_FlushUnlocked(sys, false, date);
-    vout_control_Release(&sys->control);
+    vlc_mutex_unlock(&sys->control_lock);
 }
 
 void vout_NextPicture(vout_thread_t *vout, vlc_tick_t *duration)
@@ -1605,7 +1610,7 @@ void vout_NextPicture(vout_thread_t *vout, vlc_tick_t *duration)
     assert(!sys->dummy);
     *duration = 0;
 
-    vout_control_Hold(&sys->control);
+    vlc_mutex_lock(&sys->control_lock);
     if (sys->step.last == VLC_TICK_INVALID)
         sys->step.last = sys->displayed.timestamp;
 
@@ -1619,7 +1624,7 @@ void vout_NextPicture(vout_thread_t *vout, vlc_tick_t *duration)
             /* TODO advance subpicture by the duration ... */
         }
     }
-    vout_control_Release(&sys->control);
+    vlc_mutex_unlock(&sys->control_lock);
 }
 
 void vout_ChangeDelay(vout_thread_t *vout, vlc_tick_t delay)
@@ -1628,10 +1633,10 @@ void vout_ChangeDelay(vout_thread_t *vout, vlc_tick_t delay)
     assert(!sys->dummy);
     assert(sys->display);
 
-    vout_control_Hold(&sys->control);
+    vlc_mutex_lock(&sys->control_lock);
     vlc_clock_SetDelay(sys->clock, delay);
     sys->delay = delay;
-    vout_control_Release(&sys->control);
+    vlc_mutex_unlock(&sys->control_lock);
 }
 
 void vout_ChangeRate(vout_thread_t *vout, float rate)
@@ -1639,9 +1644,9 @@ void vout_ChangeRate(vout_thread_t *vout, float rate)
     vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
     assert(!sys->dummy);
 
-    vout_control_Hold(&sys->control);
+    vlc_mutex_lock(&sys->control_lock);
     sys->rate = rate;
-    vout_control_Release(&sys->control);
+    vlc_mutex_unlock(&sys->control_lock);
 }
 
 void vout_ChangeSpuDelay(vout_thread_t *vout, size_t channel_id,
@@ -1843,15 +1848,18 @@ static void *Thread(void *object)
             deadline = INVALID_DEADLINE;
         }
 
-        while (!vout_control_Pop(&sys->control, &cmd, deadline)) {
+        vlc_mutex_lock(&sys->control_lock);
+        while (!vout_control_Pop(&sys->control, &cmd, deadline, &sys->control_lock)) {
             switch(cmd.type) {
                 case VOUT_CONTROL_TERMINATE:
+                    vlc_mutex_unlock(&sys->control_lock);
                     return NULL; /* no need to clean &cmd */
                 case VOUT_CONTROL_MOUSE_STATE:
                     ThreadProcessMouseState(vout, &cmd.mouse);
                     break;
             }
         }
+        vlc_mutex_unlock(&sys->control_lock);
 
         wait = ThreadDisplayPicture(vout, &deadline) != VLC_SUCCESS;
 
@@ -1912,7 +1920,9 @@ void vout_StopDisplay(vout_thread_t *vout)
 {
     vout_thread_sys_t *sys = VOUT_THREAD_TO_SYS(vout);
 
+    vlc_mutex_lock(&sys->control_lock);
     vout_control_PushVoid(&sys->control, VOUT_CONTROL_TERMINATE);
+    vlc_mutex_unlock(&sys->control_lock);
     vlc_join(sys->thread, NULL);
 
     vout_ReleaseDisplay(sys);
@@ -1951,7 +1961,9 @@ void vout_Close(vout_thread_t *vout)
 
     vout_IntfDeinit(VLC_OBJECT(vout));
     vout_snapshot_End(sys->snapshot);
+    vlc_mutex_lock(&sys->control_lock);
     vout_control_Dead(&sys->control);
+    vlc_mutex_unlock(&sys->control_lock);
     vout_chrono_Clean(&sys->render);
 
     if (sys->spu)
@@ -2057,6 +2069,7 @@ vout_thread_t *vout_Create(vlc_object_t *object)
     sys->spu = var_InheritBool(vout, "spu") || var_InheritBool(vout, "osd") ?
                spu_Create(vout, vout) : NULL;
 
+    vlc_mutex_init(&sys->control_lock);
     vout_control_Init(&sys->control);
 
     sys->title.show     = var_InheritBool(vout, "video-title-show");
-- 
2.26.2



More information about the vlc-devel mailing list