[vlc-devel] [vlc-commits] vout: cancel the thread and simplify

[Rémi Denis-Courmont]

It does simplify the code on objective complexity metrics. And that ship has sailed about ten years ago.

And I don't think it's okay to pick on one developer. I was not the first to push code without review lately. And I am also not the only person to disagree with systematic review.

Le 9 mai 2019 10:35:47 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>Hello,
>
>This doesn't simplify at all, thread_cancel is very complicated, I wold
>have preferred a cond + bool.
>
>You should propose all your patches changing the core to the ML. Since
>they always have to be reviewed anyway when we switch to gitlab soon.
>
>On Wed, May 8, 2019, at 20:21, Rémi Denis-Courmont wrote:
>> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Wed
>May  
>> 8 19:20:53 2019 +0300| [949204f27498bd21e8d269b5aa9b80c7ec84a435] | 
>> committer: Rémi Denis-Courmont
>> 
>> vout: cancel the thread and simplify
>> 
>> >
>http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=949204f27498bd21e8d269b5aa9b80c7ec84a435
>> ---
>> 
>>  src/video_output/control.h      |  7 -------
>>  src/video_output/video_output.c | 24 ++++++++++++------------
>>  2 files changed, 12 insertions(+), 19 deletions(-)
>> 
>> diff --git a/src/video_output/control.h b/src/video_output/control.h
>> index 46b78122e4..b189b6478a 100644
>> --- a/src/video_output/control.h
>> +++ b/src/video_output/control.h
>> @@ -27,13 +27,6 @@
>>  
>>  /* */
>>  enum {
>> -    VOUT_CONTROL_CLEAN,
>> -
>> -#if 0
>> -    /* */
>> -    VOUT_CONTROL_START,
>> -    VOUT_CONTROL_STOP,
>> -#endif
>>      VOUT_CONTROL_CHANGE_FILTERS,        /* string */
>>      VOUT_CONTROL_CHANGE_INTERLACE,      /* boolean */
>>  
>> diff --git a/src/video_output/video_output.c
>b/src/video_output/video_output.c
>> index 2a76ad5c00..b84336ac23 100644
>> --- a/src/video_output/video_output.c
>> +++ b/src/video_output/video_output.c
>> @@ -33,6 +33,8 @@
>>  # include "config.h"
>>  #endif
>>  
>> +#include <stdnoreturn.h>
>> +
>>  #include <vlc_common.h>
>>  
>>  #include <math.h>
>> @@ -1527,11 +1529,9 @@ void vout_Cancel(vout_thread_t *vout, bool
>canceled)
>>      vout_control_Release(&sys->control);
>>  }
>>  
>> -static int ThreadControl(vout_thread_t *vout, vout_control_cmd_t
>cmd)
>> +static void ThreadControl(vout_thread_t *vout, vout_control_cmd_t
>cmd)
>>  {
>>      switch(cmd.type) {
>> -    case VOUT_CONTROL_CLEAN:
>> -        return 1;
>>      case VOUT_CONTROL_CHANGE_FILTERS:
>>          ThreadChangeFilters(vout, NULL,
>>                              cmd.string != NULL ?
>> @@ -1579,7 +1579,6 @@ static int ThreadControl(vout_thread_t *vout, 
>> vout_control_cmd_t cmd)
>>          break;
>>      }
>>      vout_control_cmd_Clean(&cmd);
>> -    return 0;
>>  }
>>  
>>  
>>
>/*****************************************************************************
>> @@ -1589,7 +1588,7 @@ static int ThreadControl(vout_thread_t *vout, 
>> vout_control_cmd_t cmd)
>>   * terminated. It handles the pictures arriving in the video heap
>and 
>> the
>>   * display device events.
>>   
>>
>*****************************************************************************/
>> -static void *Thread(void *object)
>> +noreturn static void *Thread(void *object)
>>  {
>>      vout_thread_t *vout = object;
>>      vout_thread_sys_t *sys = vout->p;
>> @@ -1607,20 +1606,21 @@ static void *Thread(void *object)
>>          } else {
>>              deadline = VLC_TICK_INVALID;
>>          }
>> -        while (!vout_control_Pop(&sys->control, &cmd, deadline))
>> -            if (ThreadControl(vout, cmd))
>> -                goto out;
>> +        while (!vout_control_Pop(&sys->control, &cmd, deadline)) {
>> +            int canc = vlc_savecancel();
>> +            ThreadControl(vout, cmd);
>> +            vlc_restorecancel(canc);
>> +        }
>>  
>> +        int canc = vlc_savecancel();
>>          deadline = VLC_TICK_INVALID;
>>          wait = ThreadDisplayPicture(vout, &deadline) != VLC_SUCCESS;
>>  
>>          const bool picture_interlaced =
>sys->displayed.is_interlaced;
>>  
>>          vout_SetInterlacingState(vout, picture_interlaced);
>> +        vlc_restorecancel(canc);
>>      }
>> -
>> -out:
>> -    return NULL;
>>  }
>>  
>>  static void vout_StopDisplay(vout_thread_t *vout)
>> @@ -1628,7 +1628,7 @@ static void vout_StopDisplay(vout_thread_t
>*vout)
>>      vout_thread_sys_t *sys = vout->p;
>>  
>>      assert(sys->original.i_chroma != 0);
>> -    vout_control_PushVoid(&sys->control, VOUT_CONTROL_CLEAN);
>> +    vlc_cancel(sys->thread);
>>      vlc_join(sys->thread, NULL);
>>  
>>      if (sys->spu_blend != NULL)
>> 
>> _______________________________________________
>> vlc-commits mailing list
>> vlc-commits at videolan.org
>> https://mailman.videolan.org/listinfo/vlc-commits
>>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190509/d0f7e3fb/attachment.html>