<html><head></head><body>Hi,<br><br>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><br>Again, you have to cope with blocking calls somehowm. There are no ways around that, not even in 4.0.<br><br><br>I have no objections to adding APIs per se. I do however have objections to making major changes to existing APIs.<br><br>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><br><br>In other words, I have problems both with the rationale and the implementation of this patchset.<br><br><div class="gmail_quote">Le 20 octobre 2020 11:53:23 GMT+03:00, Thomas Guillem <thomas@gllm.fr> 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"><br><br>On Mon, Oct 19, 2020, at 17:28, 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 maanantaina 19. lokakuuta 2020, 17.48.16 EEST Thomas Guillem a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">From what I have seen, deadlocks are usually<br>caused by threading mistakes in applications. In particular, if there is a<br>deadlock, how come libvlc_media_player_release() is not exhibiting the<br>same problem?<br></blockquote>libvlc_media_player_release() will block but not deadlock since there should<br>not be any users interaction when it's called.<br></blockquote>I don't follow. If it blocks the thread, and if that thread is the UI main <br>loop, it prevents user interactions. I could just as well write <br>"libvlc_media_player_stop() will block but not deadlock". The point of <br>asynchtonous stop is to make stop faster UX because synchronous stop is <br>potentially slow.<br><br>The point is *not* to solve any deadlock. The calling thread needs to be able <br>to cope with blocking LibVLC calls in any case. You cannot avoid joining the <br>LibVLC threads eventually.<br></blockquote><br>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><br>> <br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">I don't follow the rationale for this patchset at all.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">By the way, I don't like the fact that it is refcounted<br></blockquote>I don't like it either. I am pretty sure that I argued to remove some useless <br>reference counters in the past, and that other people rejected it, so don't <br>tell me, though.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">since the user can't know if the call will block or not.<br></blockquote>Even without reference count, you would have to assume that it might block.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">If it's really necessary to update LibVLC 3.x, then that's that, but<br>you're a bit light on details here, IMO.<br></blockquote>Here is an example of deadlock <a href="https://code.videolan.org/-/snippets/1286/raw">https://code.videolan.org/-/snippets/1286/raw</a><br><br>1/ libvlc_media_list_player_play_item_at_index() that will call<br>input_Close() is called from a background thread (as a hack, since it is a<br>blocking call).<br>2/ The setting of the new media cause the ios vout to wait<br>for the mainthread.<br>3/ libvlc_video_get_track_count() is called from the<br>mainthread and wait for the input_thread that is closing.<br><br> => Deadlock.<br></blockquote>This is not valid to begin with. You cannot have a thread in <br>libvlc_media_list_player_play_item_at_index() and another thread in <br>libvlc_video_get_track_count() simultaneously.<br><br>And then the patch causes LibVLC to potentially accumulate a large number of <br>input threads. This is a terrible idea IMO. It's not even a question of <br>extending the API/ABI of a stable release.<br></blockquote><br>Yep, that will make it hard to backport.<br>For the record, the vlc_player open its new input when the old one is closed (input_Close() -> vlc_join()).<br>We could backport such feature to keep only one opened media, but it will be a huge hotfix...<br><br>> <br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">-- <br>雷米‧德尼-库尔蒙<br><a href="http://www.remlab.net/">http://www.remlab.net/</a><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></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>