[vlc-devel] [PATCH 3.x 0/2] RFC: libvlc_media_player: add async stop

Rémi Denis-Courmont remi at remlab.net
Tue Oct 20 13:51:36 CEST 2020


Hi,

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.

Again, you have to cope with blocking calls somehowm. There are no ways around that, not even in 4.0.


I have no objections to adding APIs per se. I do however have objections to making major changes to existing APIs.

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.


In other words, I have problems both with the rationale and the implementation of this patchset.

Le 20 octobre 2020 11:53:23 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>
>On Mon, Oct 19, 2020, at 17:28, Rémi Denis-Courmont wrote:
>> Le maanantaina 19. lokakuuta 2020, 17.48.16 EEST Thomas Guillem a
>écrit :
>> > > From what I have seen, deadlocks are usually
>> > > caused by threading mistakes in applications. In particular, if
>there is a
>> > > deadlock, how come libvlc_media_player_release() is not
>exhibiting the
>> > > same problem?
>> > 
>> > libvlc_media_player_release() will block but not deadlock since
>there should
>> > not be any users interaction when it's called.
>> 
>> I don't follow. If it blocks the thread, and if that thread is the UI
>main 
>> loop, it prevents user interactions. I could just as well write 
>> "libvlc_media_player_stop() will block but not deadlock". The point
>of 
>> asynchtonous stop is to make stop faster UX because synchronous stop
>is 
>> potentially slow.
>> 
>> The point is *not* to solve any deadlock. The calling thread needs to
>be able 
>> to cope with blocking LibVLC calls in any case. You cannot avoid
>joining the 
>> LibVLC threads eventually.
>
>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).
>
>> 
>> I don't follow the rationale for this patchset at all.
>> 
>> > By the way, I don't like the fact that it is refcounted
>> 
>> I don't like it either. I am pretty sure that I argued to remove some
>useless 
>> reference counters in the past, and that other people rejected it, so
>don't 
>> tell me, though.
>> 
>> > since the user can't know if the call will block or not.
>> 
>> Even without reference count, you would have to assume that it might
>block.
>> 
>> > > If it's really necessary to update LibVLC 3.x, then that's that,
>but
>> > > you're a bit light on details here, IMO.
>> > 
>> > Here is an example of deadlock
>https://code.videolan.org/-/snippets/1286/raw
>> > 
>> > 1/ libvlc_media_list_player_play_item_at_index() that will call
>> > input_Close() is called from a background thread (as a hack, since
>it is a
>> > blocking call).
>> 
>> > 2/ The setting of the new media cause the ios vout to wait
>> > for the mainthread.
>> > 3/ libvlc_video_get_track_count() is called from the
>> > mainthread and wait for the input_thread that is closing.
>> > 
>> >  => Deadlock.
>> 
>> This is not valid to begin with. You cannot have a thread in 
>> libvlc_media_list_player_play_item_at_index() and another thread in  
>> libvlc_video_get_track_count() simultaneously.
>> 
>> And then the patch causes LibVLC to potentially accumulate a large
>number of 
>> input threads. This is a terrible idea IMO. It's not even a question
>of 
>> extending the API/ABI of a stable release.
>
>Yep, that will make it hard to backport.
>For the record, the vlc_player open its new input when the old one is
>closed (input_Close() -> vlc_join()).
>We could backport such feature to keep only one opened media, but it
>will be a huge hotfix...
>
>> 
>> -- 
>> 雷米‧德尼-库尔蒙
>> http://www.remlab.net/
>> 
>> 
>> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20201020/8018d2d8/attachment.html>


More information about the vlc-devel mailing list