[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