<!DOCTYPE html><html><head><title></title><style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div><br></div><div><br></div><div>On Tue, Oct 20, 2020, at 13:51, Rémi Denis-Courmont wrote:<br></div><blockquote type="cite" id="qt" style=""><div>Hi,<br></div><div><br></div><div>You can't assume that the release call is anything special. There are use cases for destroying a media player before the UI. Moreover, there are UI frameworks where you cannot destroy the media player(s) after the GUI.<br></div><div><br></div><div>Again, you have to cope with blocking calls somehowm. There are no ways around that, not even in 4.0.<br></div><div><br></div><div><br></div><div>I have no objections to adding APIs per se. I do however have objections to making major changes to existing APIs.<br></div><div><br></div><div>And then, keeping multiple inputs for a single player at the same time is unreasonable, especially as a stable update. It causes subtle and not so subtle functionality and performance regressions.<br></div></blockquote><div><br></div><div>I already said I agreed with you on that point.<br></div><div>I did a new version that is opening new medias from the background thread once the old media is closed.<br></div><div><br></div><div>cf. new patch here <a href="https://code.videolan.org/tguillem/vlc-legacy/-/commit/8a746bee0e884cfd97d97c1a740e8ef769aab54f">https://code.videolan.org/tguillem/vlc-legacy/-/commit/8a746bee0e884cfd97d97c1a740e8ef769aab54f</a><br></div><div><br></div><div>I'm not sure this patch should be applied. It will be used by VLCKit. It will ease the transition between VLC 3.0 and 4.0 since we can start removing some async hacks in VLCKit now.<br></div><div><br></div><blockquote type="cite" id="qt" style=""><div><br></div><div><br></div><div>In other words, I have problems both with the rationale and the implementation of this patchset.<br></div><div><br></div><div class="qt-gmail_quote"><div>Le 20 octobre 2020 11:53:23 GMT+03:00, Thomas Guillem <thomas@gllm.fr> 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><br></div><div><br></div><div>On Mon, Oct 19, 2020, at 17:28, Rémi Denis-Courmont 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>Le maanantaina 19. lokakuuta 2020, 17.48.16 EEST Thomas Guillem a écrit :<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(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><blockquote class="qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(138, 226, 52);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>From what I have seen, deadlocks are usually<br></div><div>caused by threading mistakes in applications. In particular, if there is a<br></div><div>deadlock, how come libvlc_media_player_release() is not exhibiting the<br></div><div>same problem?<br></div></blockquote><div>libvlc_media_player_release() will block but not deadlock since there should<br></div><div>not be any users interaction when it's called.<br></div></blockquote><div>I don't follow. If it blocks the thread, and if that thread is the UI main <br></div><div>loop, it prevents user interactions. I could just as well write <br></div><div>"libvlc_media_player_stop() will block but not deadlock". The point of <br></div><div>asynchtonous stop is to make stop faster UX because synchronous stop is <br></div><div>potentially slow.<br></div><div><br></div><div>The point is *not* to solve any deadlock. The calling thread needs to be able <br></div><div>to cope with blocking LibVLC calls in any case. You cannot avoid joining the <br></div><div>LibVLC threads eventually.<br></div></blockquote><div><br></div><div>The point of this patch is to move the blocking call to the last _release() call, when the UI is destroyed. That is what is done in VLC 4.0, every player calls are non-blocking, except vlc_player_Delete() (that is joining the input_thread).<br></div><div><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>I don't follow the rationale for this patchset at all.<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(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;">By the way, I don't like the fact that it is refcounted<br></blockquote><div>I don't like it either. I am pretty sure that I argued to remove some useless <br></div><div>reference counters in the past, and that other people rejected it, so don't <br></div><div>tell me, though.<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(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;">since the user can't know if the call will block or not.<br></blockquote><div>Even without reference count, you would have to assume that it might block.<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(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><blockquote class="qt-gmail_quote" style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(138, 226, 52);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>If it's really necessary to update LibVLC 3.x, then that's that, but<br></div><div>you're a bit light on details here, IMO.<br></div></blockquote><div>Here is an example of deadlock <a href="https://code.videolan.org/-/snippets/1286/raw">https://code.videolan.org/-/snippets/1286/raw</a><br></div><div><br></div><div>1/ libvlc_media_list_player_play_item_at_index() that will call<br></div><div>input_Close() is called from a background thread (as a hack, since it is a<br></div><div>blocking call).<br></div><div>2/ The setting of the new media cause the ios vout to wait<br></div><div>for the mainthread.<br></div><div>3/ libvlc_video_get_track_count() is called from the<br></div><div>mainthread and wait for the input_thread that is closing.<br></div><div><br></div><div> => Deadlock.<br></div></blockquote><div>This is not valid to begin with. You cannot have a thread in <br></div><div>libvlc_media_list_player_play_item_at_index() and another thread in <br></div><div>libvlc_video_get_track_count() simultaneously.<br></div><div><br></div><div>And then the patch causes LibVLC to potentially accumulate a large number of <br></div><div>input threads. This is a terrible idea IMO. It's not even a question of <br></div><div>extending the API/ABI of a stable release.<br></div></blockquote><div><br></div><div>Yep, that will make it hard to backport.<br></div><div>For the record, the vlc_player open its new input when the old one is closed (input_Close() -> vlc_join()).<br></div><div>We could backport such feature to keep only one opened media, but it will be a huge hotfix...<br></div><div><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>雷米‧德尼-库尔蒙<br></div><div><a href="http://www.remlab.net/">http://www.remlab.net/</a><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><div><br></div></body></html>