[vlc-devel] [PATCH 5/7] vout/control: keep the lock instead of setting a held flag

[Thomas Guillem]

Why is it not needed anymore?

vout_control_Push() will now have to wait that controls are processed. Are you really sure we want that? For example, vout_MouseState() will wait until a flush command is processed.


On Thu, Aug 13, 2020, at 14:12, Steve Lhomme wrote:
> The code calling vout_control_Hold holds the lock until vout_control_Release is
> called. vout_control_Pop() doesn't need to wait for it. If it has the lock no
> other code is "holding" it.
> 
> Now vout_control_Release() doesn't wake up the vout_control_Pop() directly.
> Either commands were added and wait_request was signaled. Or nothing was done
> and it doesn't need to wake up vout_control_Pop(). It will be unblocked by new
> commands, a vout_control_Wake call or a deadline timeout.
> ---
>  src/video_output/control.c | 17 -----------------
>  src/video_output/control.h |  3 ---
>  2 files changed, 20 deletions(-)
> 
> diff --git a/src/video_output/control.c b/src/video_output/control.c
> index 675953f2c89..93bf7c3f92a 100644
> --- a/src/video_output/control.c
> +++ b/src/video_output/control.c
> @@ -51,10 +51,7 @@ void vout_control_Init(vout_control_t *ctrl)
>  {
>      vlc_mutex_init(&ctrl->lock);
>      vlc_cond_init(&ctrl->wait_request);
> -    vlc_cond_init(&ctrl->wait_available);
>  
> -    ctrl->is_held = false;
> -    ctrl->is_waiting = false;
>      ctrl->is_dead = false;
>      ctrl->can_sleep = true;
>      ARRAY_INIT(ctrl->cmd);
> @@ -125,18 +122,10 @@ void vout_control_PushString(vout_control_t 
> *ctrl, int type, const char *string)
>  void vout_control_Hold(vout_control_t *ctrl)
>  {
>      vlc_mutex_lock(&ctrl->lock);
> -    while (ctrl->is_held || !ctrl->is_waiting)
> -        vlc_cond_wait(&ctrl->wait_available, &ctrl->lock);
> -    ctrl->is_held = true;
> -    vlc_mutex_unlock(&ctrl->lock);
>  }
>  
>  void vout_control_Release(vout_control_t *ctrl)
>  {
> -    vlc_mutex_lock(&ctrl->lock);
> -    assert(ctrl->is_held);
> -    ctrl->is_held = false;
> -    vlc_cond_signal(&ctrl->wait_available);
>      vlc_mutex_unlock(&ctrl->lock);
>  }
>  
> @@ -148,16 +137,10 @@ int vout_control_Pop(vout_control_t *ctrl, 
> vout_control_cmd_t *cmd,
>      if (ctrl->cmd.i_size <= 0) {
>          /* Spurious wakeups are perfectly fine */
>          if (deadline != INVALID_DEADLINE && ctrl->can_sleep) {
> -            ctrl->is_waiting = true;
> -            vlc_cond_signal(&ctrl->wait_available);
>              vlc_cond_timedwait(&ctrl->wait_request, &ctrl->lock, 
> deadline);
> -            ctrl->is_waiting = false;
>          }
>      }
>  
> -    while (ctrl->is_held)
> -        vlc_cond_wait(&ctrl->wait_available, &ctrl->lock);
> -
>      int ret;
>      if (ctrl->cmd.i_size > 0) {
>          ret = VLC_SUCCESS;
> diff --git a/src/video_output/control.h b/src/video_output/control.h
> index 6161c5ba3db..af13cc8fede 100644
> --- a/src/video_output/control.h
> +++ b/src/video_output/control.h
> @@ -50,13 +50,10 @@ void vout_control_cmd_Clean(vout_control_cmd_t *);
>  typedef struct {
>      vlc_mutex_t lock;
>      vlc_cond_t  wait_request;
> -    vlc_cond_t  wait_available;
>  
>      /* */
>      bool is_dead;
>      bool can_sleep;
> -    bool is_waiting;
> -    bool is_held;
>      DECL_ARRAY(vout_control_cmd_t) cmd;
>  } vout_control_t;
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel