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

Rémi Denis-Courmont remi at remlab.net
Thu May 9 13:00:14 CEST 2019


I think common sense says not to knowingly break the Qt vout window and not to keep resubmitting patches ignoring review comments (audio gap-less, vout life cycle).

I don't think it says anything about core vs other parts.

Le 9 mai 2019 13:53:23 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>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

-- 
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/73dc60f1/attachment.html>


More information about the vlc-devel mailing list