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