<html><head></head><body>Hi,<br><br>There is no threading 101 here. It's common sense in threading that some data is readable and some is not. Otherwise you couldn't even get a hold of the lock objects.<br><br>Increasing lock scope for taste reasons is horrible. It just gives the wrong impression about the requirements, and creates confusion.<br><br>Totally agree with Thomas against this change.<br><br><div class="gmail_quote">Le 10 octobre 2019 09:05:07 GMT+03: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">On 2019-10-09 18:15, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Le keskiviikkona 9. lokakuuta 2019, 18.50.23 EEST Steve Lhomme a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">On 2019-10-09 17:48, Thomas Guillem wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">On Wed, Oct 9, 2019, at 17:36, Steve Lhomme wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"><hr> src/video_output/video_output.c | 3 +--<br> 1 file changed, 1 insertion(+), 2 deletions(-)<br><br> diff --git a/src/video_output/video_output.c<br> b/src/video_output/video_output.c<br> index a5046913867..df27a402b8b 100644<br> --- a/src/video_output/video_output.c<br> +++ b/src/video_output/video_output.c<br> @@ -1013,9 +1013,8 @@ static int<br> ThreadDisplayRenderPicture(vout_thread_t *vout, bool is_forced)<br><br> if (filtered->date != sys->displayed.current->date)<br> <br> msg_Warn(vout, "Unsupported timestamp modifications done by<br><br> chain_interactive");<br><br> - vout_display_t *vd = sys->display;<br> -<br><br> vlc_mutex_lock(&sys->display_lock);<br><br> + vout_display_t *vd = sys->display;<br></blockquote>It is not needed since this variable is set before the thread is created.<br></blockquote>At the very least it's easier to read than wonder why this right exactly<br>before a lock. Why make things more complicated to read ?<br></blockquote>On the complexity metric, it's exactly the same - not more or less<br>complicated.<br><br>And it gives the wrong impression that the value needs the lock to be read. So<br>it's really just worse. I agree with Thomas.<br></blockquote><br>When I read this I read: the coherence is not guaranteed. This is <br>multithreading 101. Forcing the reader to know the whole context of <br>where sys->display comes from and when it exists or is modified, just to <br>read this function, is just plain dumb. There is no technical reason. <br>The only reason given is that because of the conjunctions of all the <br>planets around this function it is actually possible to read the value <br>without the lock (and then lock any access to that value for everyone <br>else because it would have terrible consequences).<br><br>To me it's just keeping a complexity just for the sake of it.<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>