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

[Thomas Guillem]

On Thu, May 9, 2019, at 11:48, Rémi Denis-Courmont wrote:
> 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.

I don't think someone pushed in VLC core without a review since we decided to move to gitlab.

>From the vote poll result, it seems the vast majority of people agreed to use gitalb and systematic reviews.

> 
> 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é. 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190509/31733e3a/attachment.html>