[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