<html><head></head><body>Hi,<br><br>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.<br><br>By the way, I think we should remove !ENABLE_SOUT from the core (but keep it in modules/). The savings are negligible.<br><br><div class="gmail_quote">Le 20 janvier 2021 08:46:53 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">Patchset LGTM apart from comments below.<br><br>On 2021-01-19 19:21, remi@remlab.net wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">From: Rémi Denis-Courmont <remi@remlab.net><br><br>(no functional changes)<hr> src/input/resource.c | 68 +++++++++++++++++++-------------------------<br> 1 file changed, 29 insertions(+), 39 deletions(-)<br><br>diff --git a/src/input/resource.c b/src/input/resource.c<br>index 1f8a5d5e0e..7c4716251c 100644<br>--- a/src/input/resource.c<br>+++ b/src/input/resource.c<br>@@ -141,42 +141,6 @@ static void DestroySout( input_resource_t *p_resource )<br> p_resource->p_sout = NULL;<br> }<br> <br>-static sout_instance_t *RequestSout( input_resource_t *p_resource,<br>- sout_instance_t *p_sout, const char *psz_sout )<br>-{<br>-#ifdef ENABLE_SOUT<br>- assert( !p_sout );<br>-<br>- /* Check the validity of the sout */<br>- if( p_resource->p_sout &&<br>- strcmp( p_resource->p_sout->psz_sout, psz_sout ) )<br>- {<br>- msg_Dbg( p_resource->p_parent, "destroying unusable sout" );<br>- DestroySout( p_resource );<br>- }<br>-<br>- if( p_resource->p_sout )<br>- {<br>- /* Reuse it */<br>- msg_Dbg( p_resource->p_parent, "reusing sout" );<br>- msg_Dbg( p_resource->p_parent, "you probably want to use gather stream_out" );<br>- }<br>- else<br>- {<br>- /* Create a new one */<br>- p_resource->p_sout = sout_NewInstance( p_resource->p_parent, psz_sout );<br>- }<br>-<br>- p_sout = p_resource->p_sout;<br>- p_resource->p_sout = NULL;<br>-<br>- return p_sout;<br>-#else<br>- VLC_UNUSED (p_resource); VLC_UNUSED (p_sout); VLC_UNUSED (psz_sout);<br>- return NULL;<br>-#endif<br>-}<br>-<br> /* */<br> static void DestroyVout( input_resource_t *p_resource )<br> {<br>@@ -609,12 +573,38 @@ void input_resource_StopFreeVout(input_resource_t *p_resource)<br> /* */<br> sout_instance_t *input_resource_RequestSout( input_resource_t *p_resource, const char *psz_sout )<br> {<br>+ sout_instance_t *sout;<br>+<br> assert(psz_sout != NULL);<br> vlc_mutex_lock( &p_resource->lock );<br></blockquote><br>Do we need the lock if ENABLE_SOUT is not defined ? (IMO no) Is it even <br>initialized in that case ? (IMO it shouldn't)<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">- sout_instance_t *p_ret = RequestSout( p_resource, NULL, psz_sout );<br>- vlc_mutex_unlock( &p_resource->lock );<br>+#ifdef ENABLE_SOUT<br>+ /* Check the validity of the sout */<br>+ if (p_resource->p_sout != NULL<br>+ && strcmp(p_resource->p_sout->psz_sout, psz_sout) != 0)<br></blockquote><br>Not a fan of the && at the beginning. I prefer the way it was in the <br>code you moved.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">+ {<br>+ msg_Dbg(p_resource->p_parent, "destroying unusable sout");<br>+ DestroySout(p_resource);<br>+ }<br> <br>- return p_ret;<br>+ sout = p_resource->p_sout;<br>+<br>+ if (sout != NULL)<br>+ {<br>+ /* Reuse it */<br>+ msg_Dbg(p_resource->p_parent, "reusing sout");<br>+ msg_Dbg(p_resource->p_parent, "you probably want to use gather stream_out");<br>+ p_resource->p_sout = NULL;<br>+ }<br>+ else<br>+ {<br>+ /* Create a new one */<br>+ sout = sout_NewInstance(p_resource->p_parent, psz_sout);<br>+ }<br>+#else<br>+ sout = NULL;<br>+#endif<br>+ vlc_mutex_unlock( &p_resource->lock );<br>+ return sout;<br> }<br> <br> void input_resource_PutSout(input_resource_t *resource, sout_instance_t *sout)<br>-- <br>2.30.0<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>