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

Thomas Guillem thomas at gllm.fr
Thu May 9 12:53:23 CEST 2019


On Thu, May 9, 2019, at 12:39, Rémi Denis-Courmont wrote:
> There was never a rule of sending specifically "core" changes for review outside code freeze. This is just a bad faith excuse to not admit that you are picking on me.

OK, It was never a written rule. But after last discussions about moving to gitlab, it's common-sense to don't push directly core changes to vlc.git.

> 
> Le 9 mai 2019 13:15:34 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>> 
>> 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
>> 
> 
> -- 
> 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/b6766de3/attachment.html>


More information about the vlc-devel mailing list