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

[Thomas Guillem]

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