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

[Thomas Guillem]

On Wed, Jan 20, 2021, at 09:26, Rémi Denis-Courmont wrote:
> 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.
+1

> 
> 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.0vlc-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é. 
> _______________________________________________
> 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/20210120/659b59d3/attachment.html>