<!DOCTYPE html><html><head><title></title><style type="text/css">
p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div>I have an other use case:<br></div><div><br></div><div> 1/ Open a http stream, server is not responding, the open function of the http2 module is waiting<br></div><div> 2/ The user cancel if after some time, the http2 open module is interrupted<br></div><div> 3/ The old http module is probed and will also wait indefinitely. The user can't cancel it anymore.<br></div><div><br></div><div>For me, it's clearly a bug, and a possible fix should be backported to VLC 3.0 too.<br></div><div><br></div><div>I agree that my 2 attempts to fix it are not good<br></div><div><br></div><div>1/ Interrupt patch: rejected by you, I'll revert it<br></div><div>2/ module probe patch: rejected by you. And indeed, breaking module loading even when the user cancel a media is not a perfect solution. For example it could prevent to load a module that will just save some contexts.<br></div><div><br></div><div>I will work on an other solution<br></div><div><br></div><div>On Thu, Oct 24, 2019, at 11:59, Rémi Denis-Courmont wrote:<br></div><blockquote type="cite" id="qt"><div>I don't agree with the problem statement. What it describes is intended behaviour, not a bug.<br></div><div><br></div><div class="qt-gmail_quote"><div>Le 24 octobre 2019 12:46:46 GMT+03:00, Thomas Guillem <thomas@gllm.fr> a écrit :<br></div><blockquote 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;" class="qt-gmail_quote"><div><br></div><div>On Thu, Oct 24, 2019, at 11:20, Rémi Denis-Courmont wrote:<br></div><blockquote id="qt-qt" type="cite"><div>You effectively reverted to the old broken logic from before the interrupt system, and disregarding documented API semantics, wrecking many weeks of work from me. Is what you did.<br></div></blockquote><div><br></div><div>Yes I made a mistake, I should have waited longer and/or ask again for review before pushing it.<br></div><div>I did not realize that my patch would cause such a mess. I'm sorry for that.<br></div><div><br></div><div>Is my alternate solution acceptable for you ?<br></div><div><br></div><blockquote id="qt-qt" type="cite"><div><br></div><div class="qt-qt-gmail_quote"><div>Le 24 octobre 2019 10:02:17 GMT+03:00, Thomas Guillem <thomas@gllm.fr> a écrit :<br></div><blockquote class="qt-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-qt-k9mail"><div><br></div><div><br></div><div>On Wed, Oct 23, 2019, at 21:25, Remi Denis-Courmont wrote:<br></div><blockquote class="qt-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>Le 2019-10-22 17:47, Thomas Guillem a écrit :<br></div><blockquote class="qt-qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>If a module is interrupted while waiting in a vlc_*_i11e*() function, <br></div><div>the i11e<br></div><div>function will return an error after restoring the interrupted state <br></div><div>(from<br></div><div>vlc_interrupt_finish()). This error will be carefully handled by the <br></div><div>module<br></div><div>that will close itself. If a second module is in the probe list, any <br></div><div>call to a<br></div><div>vlc_*_i11e*() function will act as not interrupted. This cause any <br></div><div>following<br></div><div>modules in the probe list to ignore this interrupted state.<br></div><div><br></div><div>To fix this issue, vlc_interrupt_finish() will now restore the <br></div><div>interrupted<br></div><div>state to the killed state.<hr> src/misc/interrupt.c | 2 +-<br></div><div> 1 file changed, 1 insertion(+), 1 deletion(-)<br></div><div><br></div><div>diff --git a/src/misc/interrupt.c b/src/misc/interrupt.c<br></div><div>index d2a04a72b3c..54810892fc1 100644<br></div><div>--- a/src/misc/interrupt.c<br></div><div>+++ b/src/misc/interrupt.c<br></div><div>@@ -151,7 +151,7 @@ static int vlc_interrupt_finish(vlc_interrupt_t <br></div><div>*ctx)<br></div><div>     if (ctx->interrupted)<br></div><div>     {<br></div><div>         ret = EINTR;<br></div><div>-        ctx->interrupted = false;<br></div><div>+        ctx->interrupted = atomic_load(&ctx->killed);<br></div></blockquote><div>This looks wrong. The interruption is only there to signal the change in <br></div><div>status. It's not supposed to fire again if the status is not changed.<br></div></blockquote><div><br></div><div>I sent this patch to open a discussion, I pushed the next day, maybe way too early, because vlc for iOS really need it in order to fix dialogs that are shown after the media is killed.<br></div><div><br></div><div>It's OK I can revert.<br></div><div><br></div><div>But I would like to discuss about the other way to fix the issue mentioned by this patch.<br></div><div>The only other fix I see if to abort the module probing (between 2 probes) if vlc_killed().<br></div><div><br></div><div>> <br></div><blockquote class="qt-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;"><blockquote class="qt-qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>}<br></div><div>vlc_mutex_unlock(&ctx->lock);<br></div><div>return ret;<br></div></blockquote><div>-- <br></div><div>Rémi Denis-Courmont<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>https://mailman.videolan.org/listinfo/vlc-devel<br></div></blockquote><div><br></div></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>https://mailman.videolan.org/listinfo/vlc-devel<br></div></blockquote><div><br></div></body></html>