[vlc-devel] [PATCH 2/4] input: add a new variable storing which ES type was last modified

[Rémi Denis-Courmont]

Le 2014-07-21 15:23, Felix Abecassis a écrit :
> 2014-07-19 8:30 GMT+02:00 Rémi Denis-Courmont <remi at remlab.net>:
>> Le vendredi 18 juillet 2014, 19:40:50 Felix Abecassis a écrit :
>>> ---
>>>  src/input/event.c | 9 +++++++++
>>>  src/input/var.c   | 2 ++
>>>  2 files changed, 11 insertions(+)
>>
>>
>> I still do not see how that ensures serialization. As far as I can 
>> tell, the
>> ES lock is *not* mandatory at that point.
>>
>
> I am probably missing something here, I do not see what the problem 
> is.
> For instance, in input_SendEventPosition (src/input/event.c), we set
> the variables "position" and "time" and set "intf-event" with
> INPUT_EVENT_POSITION.

Again, existing bad code is not an excuse for new bad code. Also again, 
an input has only one position, so concurrent updates are much less of a 
potential issue, than for ES.

> Do you mean that functions input_SendEventEs* can be called
> concurrently?

I don't see anything preventing it.

> Calls to these functions seems to be protected by a lock
> in the EsOut functions (src/input/es_out.c).

Again, it does not seem that "the" ES lock is taken on all code paths. 
The ES output is supposed to be (mostly) reentrant, and indeed the 
decoders have one thread each.

>> And even if that worked, eventually people would break it, as it's 
>> too easy to
>> access the variable and make incorrect assumptions.

> The variable name suggested by Francois sounds better, there should 
> be
> less confusion about the usage of this variable.

I'm not concerned about the variable name. I'm concerned about people 
doing var_Get/var_Inherit; as things stand, I think the variables are 
only usable with var_AddCallback.

-- 
Rémi Denis-Courmont