[vlc-devel] [vlc-commits] vout: cancel the thread and simplify
Thomas Guillem
thomas at gllm.fr
Thu May 9 13:18:19 CEST 2019
On Thu, May 9, 2019, at 13:00, Rémi Denis-Courmont wrote:
> I think common sense says not to knowingly break the Qt vout window
I knowingly broke it, but tell every-one before. The Qt needed to be pushed before Gsoc (we have 4 student working on it).
You knowingly break vout recycling, it happens.
> and not to keep resubmitting patches ignoring review comments (audio gap-less,
For gapless, cf https://code.videolan.org/tguillem/vlc/commits/gapless/15 (still WIP)
I agreed with your reviews. The player now handle 2 inputs simultaneously in order to don't have the aout stream waiting for a possible but never commit audio track.
> vout life cycle).
For vout-life, I just didn't understand your reviews, cf. https://code.videolan.org/tguillem/vlc/commit/1d4d6cbc54b87ac7cf4d40876f2c0a80e60f3c2c#note_34282 for the very long exchange. I didn't know vout recycling was broken by you in the first place, that's why It confused me a lot.
cf. my last patches, I finally fixed all comments that you addressed to me.
>
> 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é.
> _______________________________________________
> 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/b4b5eda5/attachment.html>
More information about the vlc-devel
mailing list