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

[Steve Lhomme]

On 2021-01-20 9: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.

Yes, it's probably not worth all the things in missing.c and the code 
should behave the same as it already works on all builds with ENABLE_SOUT.
I didn't see any code that might not work on platforms where it's not 
enabled.

> 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
>         <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  <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
>