[vlc-devel] [PATCH 9/9] resource: inline RequestSout()

[Rémi Denis-Courmont]

Hi,

Obviously we don't need the lock, other than as a run-time sanity check. But it was taken in the preexisting code, so I prefer to keep it as far as this patch set is concerned.

By the way, I think we should remove !ENABLE_SOUT from the core (but keep it in modules/). The savings are negligible.

Le 20 janvier 2021 08:46:53 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>Patchset LGTM apart from comments below.
>
>On 2021-01-19 19:21, remi at remlab.net wrote:
>> From: Rémi Denis-Courmont <remi at remlab.net>
>> 
>> (no functional changes)
>> ---
>>   src/input/resource.c | 68
>+++++++++++++++++++-------------------------
>>   1 file changed, 29 insertions(+), 39 deletions(-)
>> 
>> diff --git a/src/input/resource.c b/src/input/resource.c
>> index 1f8a5d5e0e..7c4716251c 100644
>> --- a/src/input/resource.c
>> +++ b/src/input/resource.c
>> @@ -141,42 +141,6 @@ static void DestroySout( input_resource_t
>*p_resource )
>>       p_resource->p_sout = NULL;
>>   }
>>   
>> -static sout_instance_t *RequestSout( input_resource_t *p_resource,
>> -                                     sout_instance_t *p_sout, const
>char *psz_sout )
>> -{
>> -#ifdef ENABLE_SOUT
>> -    assert( !p_sout );
>> -
>> -    /* Check the validity of the sout */
>> -    if( p_resource->p_sout &&
>> -        strcmp( p_resource->p_sout->psz_sout, psz_sout ) )
>> -    {
>> -        msg_Dbg( p_resource->p_parent, "destroying unusable sout" );
>> -        DestroySout( p_resource );
>> -    }
>> -
>> -        if( p_resource->p_sout )
>> -        {
>> -            /* Reuse it */
>> -            msg_Dbg( p_resource->p_parent, "reusing sout" );
>> -            msg_Dbg( p_resource->p_parent, "you probably want to use
>gather stream_out" );
>> -        }
>> -        else
>> -        {
>> -            /* Create a new one */
>> -            p_resource->p_sout = sout_NewInstance(
>p_resource->p_parent, psz_sout );
>> -        }
>> -
>> -        p_sout = p_resource->p_sout;
>> -        p_resource->p_sout = NULL;
>> -
>> -        return p_sout;
>> -#else
>> -    VLC_UNUSED (p_resource); VLC_UNUSED (p_sout); VLC_UNUSED
>(psz_sout);
>> -    return NULL;
>> -#endif
>> -}
>> -
>>   /* */
>>   static void DestroyVout( input_resource_t *p_resource )
>>   {
>> @@ -609,12 +573,38 @@ void
>input_resource_StopFreeVout(input_resource_t *p_resource)
>>   /* */
>>   sout_instance_t *input_resource_RequestSout( input_resource_t
>*p_resource, const char *psz_sout )
>>   {
>> +    sout_instance_t *sout;
>> +
>>       assert(psz_sout != NULL);
>>       vlc_mutex_lock( &p_resource->lock );
>
>Do we need the lock if ENABLE_SOUT is not defined ? (IMO no) Is it even
>
>initialized in that case ? (IMO it shouldn't)
>
>> -    sout_instance_t *p_ret = RequestSout( p_resource, NULL, psz_sout
>);
>> -    vlc_mutex_unlock( &p_resource->lock );
>> +#ifdef ENABLE_SOUT
>> +    /* Check the validity of the sout */
>> +    if (p_resource->p_sout != NULL
>> +     && strcmp(p_resource->p_sout->psz_sout, psz_sout) != 0)
>
>Not a fan of the && at the beginning. I prefer the way it was in the 
>code you moved.
>
>> +    {
>> +        msg_Dbg(p_resource->p_parent, "destroying unusable sout");
>> +        DestroySout(p_resource);
>> +    }
>>   
>> -    return p_ret;
>> +    sout = p_resource->p_sout;
>> +
>> +    if (sout != NULL)
>> +    {
>> +        /* Reuse it */
>> +        msg_Dbg(p_resource->p_parent, "reusing sout");
>> +        msg_Dbg(p_resource->p_parent, "you probably want to use
>gather stream_out");
>> +        p_resource->p_sout = NULL;
>> +    }
>> +    else
>> +    {
>> +        /* Create a new one */
>> +        sout = sout_NewInstance(p_resource->p_parent, psz_sout);
>> +    }
>> +#else
>> +    sout = NULL;
>> +#endif
>> +    vlc_mutex_unlock( &p_resource->lock );
>> +    return sout;
>>   }
>>   
>>   void input_resource_PutSout(input_resource_t *resource,
>sout_instance_t *sout)
>> -- 
>> 2.30.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/20210120/9e6f88d5/attachment.html>