[vlc-devel] [PATCH 3/4] sout: lock sout_stream_t individually

[Rémi Denis-Courmont]

Hi,

It's needed in two cases, the duplicate module and the core (what the next patch removes). We can add it in duplicate, with a lot of boilerplate. But given that the ES output has a similar internal locking for essentially the same motivation (concurrent ES processed in different threads), it seems logical to move it from sout instance to sout stream instead and share the code.

In the common non-contended case, vlc_mutex_lock is just one atomic op extra.

Le 12 octobre 2020 09:57:20 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>This lock looks like in belongs in the duplicate module rather that do
>a 
>lock for all sout_stream_t users.
>
>On 2020-10-11 16:30, Rémi Denis-Courmont wrote:
>> The stream output is called from multiple threads without
>> synchronisation, notably at least one thread per ES decoder.
>Normally,
>> the stream output instance (sout_instance_t) lock will serialise all
>> calls to the first stream output module instance (sout_stream_t) in
>the
>> stream output chain.
>> 
>> This is not practical if there is also concurrency within the chain.
>> One notable case is when the select/duplicate module is used in
>filter
>> mode and one of the select/duplicate target is threaded, e.g.:
>> 
>>   #duplicate{dst=transcode{...},dst=transcode{...}}:std{...}
>> 
>> In that example, the standard output can be called from both
>transcode
>> filters.
>> 
>> To solve this, add a lock for each element in the stream output
>chain.
>> Alternatively, we could drop filter mode from select/duplicate,
>leaving
>> only the (more common) output mode where duplicate is the last
>element
>> in the chain.
>> ---
>>   src/stream_output/stream_output.c | 44
>+++++++++++++++++++++++++++----
>>   1 file changed, 39 insertions(+), 5 deletions(-)
>> 
>> diff --git a/src/stream_output/stream_output.c
>b/src/stream_output/stream_output.c
>> index e0769dc749..069fe57346 100644
>> --- a/src/stream_output/stream_output.c
>> +++ b/src/stream_output/stream_output.c
>> @@ -736,38 +736,71 @@ static void mrl_Clean( mrl_t *p_mrl )
>>   
>>   struct sout_stream_private {
>>       sout_stream_t stream;
>> +    vlc_mutex_t lock;
>>       module_t *module;
>>   };
>>   
>>   #define sout_stream_priv(s) \
>>           container_of(s, struct sout_stream_private, stream)
>>   
>> +static void sout_StreamLock(sout_stream_t *s)
>> +{
>> +    vlc_mutex_lock(&sout_stream_priv(s)->lock);
>> +}
>> +
>> +static void sout_StreamUnlock(sout_stream_t *s)
>> +{
>> +    vlc_mutex_unlock(&sout_stream_priv(s)->lock);
>> +}
>> +
>>   void *sout_StreamIdAdd(sout_stream_t *s, const es_format_t *fmt)
>>   {
>> -    return s->ops->add(s, fmt);
>> +    void *id;
>> +
>> +    sout_StreamLock(s);
>> +    id = s->ops->add(s, fmt);
>> +    sout_StreamUnlock(s);
>> +    return id;
>>   }
>>   
>>   void sout_StreamIdDel(sout_stream_t *s, void *id)
>>   {
>> +    sout_StreamLock(s);
>>       s->ops->del(s, id);
>> +    sout_StreamUnlock(s);
>>   }
>>   
>>   int sout_StreamIdSend(sout_stream_t *s, void *id, block_t *b)
>>   {
>> -    return s->ops->send(s, id, b);
>> +    int val;
>> +
>> +    sout_StreamLock(s);
>> +    val = s->ops->send(s, id, b);
>> +    sout_StreamUnlock(s);
>> +    return val;
>>   }
>>   
>>   void sout_StreamFlush(sout_stream_t *s, void *id)
>>   {
>>       if (s->ops->flush != NULL)
>> +    {
>> +        sout_StreamLock(s);
>>           s->ops->flush(s, id);
>> +        sout_StreamUnlock(s);
>> +    }
>>   }
>>   
>>   int sout_StreamControlVa(sout_stream_t *s, int i_query, va_list
>args)
>>   {
>> -    if (s->ops->control == NULL)
>> -        return VLC_EGENERIC;
>> -    return s->ops->control(s, i_query, args);
>> +    int val = VLC_EGENERIC;
>> +
>> +    if (s->ops->control != NULL)
>> +    {
>> +        sout_StreamLock(s);
>> +        val = s->ops->control(s, i_query, args);
>> +        sout_StreamUnlock(s);
>> +    }
>> +    return val;
>>   }
>>   
>>   /* Destroy a "stream_out" module */
>> @@ -823,6 +856,7 @@ static sout_stream_t *sout_StreamNew(
>vlc_object_t *parent, char *psz_name,
>>       if (unlikely(priv == NULL))
>>           return NULL;
>>   
>> +    vlc_mutex_init(&priv->lock);
>>       p_stream = &priv->stream;
>>       p_stream->psz_name = psz_name;
>>       p_stream->p_cfg    = p_cfg;
>> -- 
>> 2.28.0
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>> 
>_______________________________________________
>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/20201012/54bcb6ee/attachment.html>