[vlc-devel] [PATCH v2 09/13] vout/control: move the lock outside of the control API

Steve Lhomme robux4 at ycbcr.xyz
Mon Aug 17 16:42:20 CEST 2020


On 2020-08-17 16:33, Thomas Guillem wrote:
> On Mon, Aug 17, 2020, at 15:15, Steve Lhomme wrote:
>> The lock is still need to wait for incoming controls before the deadline.
> 
> *is still needed*

~is~ still needs

After the patchset the lock remains, it's used wherever 
vout_control_Hold was used.

>>
>> 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 | 33 ++++++++++++++++++++++-----------
>>   3 files changed, 25 insertions(+), 37 deletions(-)
>>
>> diff --git a/src/video_output/control.c b/src/video_output/control.c
>> index 6454461b068..cf974f4e29c 100644
>> --- a/src/video_output/control.c
>> +++ b/src/video_output/control.c
>> @@ -38,7 +38,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;
>> @@ -54,27 +53,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)
>> @@ -85,25 +78,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);
>>           }
>>       }
>>   
>> @@ -116,7 +97,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 d7b5f1f4aac..5e73d2fc3ab 100644
>> --- a/src/video_output/control.h
>> +++ b/src/video_output/control.h
>> @@ -37,7 +37,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;
>>   
>>       /* */
>> @@ -56,13 +55,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 be7b2185fe8..3cdc849e507 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;
>>   
>> @@ -497,8 +498,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);
>>   }
>>   
>>   /* */
>> @@ -1558,7 +1561,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)
>> @@ -1569,7 +1572,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);
>> @@ -1618,9 +1621,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)
>> @@ -1629,7 +1632,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;
>>   
>> @@ -1643,7 +1646,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)
>> @@ -1652,10 +1655,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)
>> @@ -1663,9 +1666,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,
>> @@ -1895,12 +1898,15 @@ 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 */
>>               }
>>           }
>> +        vlc_mutex_unlock(&sys->control_lock);
>>   
>>           wait = ThreadDisplayPicture(vout, &deadline) != VLC_SUCCESS;
>>   
>> @@ -1961,7 +1967,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);
>> @@ -2000,7 +2008,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)
>> @@ -2106,6 +2116,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
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 


More information about the vlc-devel mailing list