[vlc-devel] [PATCH 4/5] video_output: handle the terminate state internally in the control FIFO

[Steve Lhomme]

On 2020-12-17 17:44, Thomas Guillem wrote:
> Are you sure about this one?
> Before your patch, TERMINATE was a pseudo drain, the vout thread was stopped once it had processed all controls. What about remaining pictures?

There is no picture involved in control.c.

In current master TERMINATE is handled this way:

         while (!vout_control_Pop(&sys->control, &cmd, deadline)) {
             switch(cmd.type) {
                 case VOUT_CONTROL_TERMINATE:
                     return NULL; /* no need to clean &cmd */

It exits immediately the thread.

     vout_control_PushVoid(&sys->control, VOUT_CONTROL_TERMINATE);
     vlc_join(sys->thread, NULL);

The join continues the code once this thread is done.

The new code does the same by forcing the control_Pop to exit ASAP with 
the terminate value set. Then the thread exits immediately.

I think it's possible to simplify one step further by handling the 
terminate outside of control.c and just forcing a control_Wake to make 
it exit ASAP as well.

> On Thu, Dec 17, 2020, at 16:39, Steve Lhomme wrote:
>> It doesn't push a dummy command anymore but sets an internal flag.
>>
>> If the control FIFO is terminated we don't wait for new commands before
>> returning. There might be some pending mouse commands before the TERMINATED
>> command (that is now removed).
>> ---
>>   src/video_output/control.c | 18 ++++++++++--------
>>   src/video_output/control.h |  1 +
>>   2 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/video_output/control.c b/src/video_output/control.c
>> index 38026bf31a9..15b3e5d2adc 100644
>> --- a/src/video_output/control.c
>> +++ b/src/video_output/control.c
>> @@ -39,6 +39,7 @@ void vout_control_Init(vout_control_t *ctrl)
>>       ctrl->is_held = false;
>>       ctrl->is_waiting = false;
>>       ctrl->can_sleep = true;
>> +    ctrl->is_terminated = false;
>>       ARRAY_INIT(ctrl->cmd);
>>   }
>>   
>> @@ -70,12 +71,8 @@ void vout_control_Wake(vout_control_t *ctrl)
>>   
>>   void vout_control_PushTerminate(vout_control_t *ctrl)
>>   {
>> -    vout_control_cmd_t cmd = {
>> -        VOUT_CONTROL_TERMINATE, {0},
>> -    };
>> -
>>       vlc_mutex_lock(&ctrl->lock);
>> -    ARRAY_APPEND(ctrl->cmd, cmd);
>> +    ctrl->is_terminated = true;
>>       vlc_cond_signal(&ctrl->wait_request);
>>       vlc_mutex_unlock(&ctrl->lock);
>>   }
>> @@ -101,9 +98,10 @@ void vout_control_Release(vout_control_t *ctrl)
>>   int vout_control_Pop(vout_control_t *ctrl, vout_control_cmd_t *cmd,
>>                        vlc_tick_t deadline)
>>   {
>> +    bool has_cmd = false;
>>       vlc_mutex_lock(&ctrl->lock);
>>   
>> -    if (ctrl->cmd.i_size <= 0) {
>> +    if (ctrl->cmd.i_size <= 0 && !ctrl->is_terminated) {
>>           /* Spurious wakeups are perfectly fine */
>>           if (deadline != VLC_TICK_INVALID && ctrl->can_sleep) {
>>               ctrl->is_waiting = true;
>> @@ -116,15 +114,19 @@ int vout_control_Pop(vout_control_t *ctrl,
>> vout_control_cmd_t *cmd,
>>       while (ctrl->is_held)
>>           vlc_cond_wait(&ctrl->wait_available, &ctrl->lock);
>>   
>> -    bool has_cmd;
>> +    if (ctrl->is_terminated) {
>> +        *cmd = (vout_control_cmd_t) { VOUT_CONTROL_TERMINATE, {0} };
>> +        goto done;
>> +    }
>> +
>>       if (ctrl->cmd.i_size > 0) {
>>           has_cmd = true;
>>           *cmd = ARRAY_VAL(ctrl->cmd, 0);
>>           ARRAY_REMOVE(ctrl->cmd, 0);
>>       } else {
>> -        has_cmd = false;
>>           ctrl->can_sleep = true;
>>       }
>> +done:
>>       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 bb0ba999ea1..8f8ebb966e3 100644
>> --- a/src/video_output/control.h
>> +++ b/src/video_output/control.h
>> @@ -46,6 +46,7 @@ typedef struct {
>>       bool can_sleep;
>>       bool is_waiting;
>>       bool is_held;
>> +    bool is_terminated;
>>       DECL_ARRAY(vout_control_cmd_t) cmd;
>>   } vout_control_t;
>>   
>> -- 
>> 2.29.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
>