[vlc-devel] [vlc-commits] skins2: ensure playlist gets stopped before terminating vlc.
erwan.tulou at gmail.com
erwan.tulou at gmail.com
Mon Dec 2 21:22:46 CET 2019
On 02/12/2019 18:05, Rémi Denis-Courmont wrote:
> Le maanantaina 2. joulukuuta 2019, 14.11.28 EET erwan.tulou at gmail.com a écrit
>> On 02/12/2019 09:20, Alexandre Janniaux wrote:
>>> Actually stopping the playlist wont kill the video output
>>> module, only killing the input resource, meaning releasing
>>> the vlc_player will do.
>>> However, player and playlist might be used by multiple
>>> control interface and this problem is only a concern for
>>> the one providing the vout_window. Hence I believe it must
>>> be solved by the mechanism providing the window instead of
>>> relying on a much wider API like playlist and player.
>> I fully agree with that. And actually, the work is already partially
>> done. A 'vout_window_ReportClose' is available to let the vout know that
>> the windows ids won't be available any more.
> No. That ReportClose does *not* mean that the window ID is no longer valid.
> Even back when the earlier incarnation of that callback (in the old vout core)
> "worked", all it did was request an asynchronous stop of the playlist
> It just means that the window system has advised that the window should be
> closed - normally because the user clicked on the window close button. The
> window ID must remain valid until the window is actually released by the core.
> Otherwise, you *will* introduce intractable races for the consumer of the
> window and, e.g., deadlock or crash OpenGL.
Okay, I fully agree with this, and that's why I am raising the issue
with the new playlist.
vlc version 3.0 (the old playlist ) :
- stop the old playlist (this stops everything, all vout are freed)
- stop and destroy the interfaces (no problem since the vout are already
- delete the playlist
vlc version 4.0 (the new playlist)
- stop the new playlist (this keeps a vlc player and the last vout with
the vout window in a disabled state just in case we resume)
- stop the interfaces (Qt or skins2 terminates whatever vout is still
running, luckily vout is disabled)
- delete the playlist (well, now we get rid of the vout window by
issuing vout_window_close() => crash expected !!!!!)
The problem is that :
- we cannot delete the playlist before stopping the interfaces (both Qt
and skins2 expects a valid playlist throughout their lifetime)
- we cannot stop the interface before deleting the playlist, because
vout_window_close() is still expected to be called (this is a callback
function linked to the interface)
So, something has to be done to remedy this design issue. For now, there
is a hack in skins2 to avoid crash and Qt uses --no-embedded-video, so
it is not yet noticeable. Another workaround is that the
vout_window_close() does nothing, and do all the cleaning up in
vout_window_disable(). But then, why keep two functions if one needs to
be a no-op.
> The GUI can and should call ReportClose when the interface is being closed by
> the user. But it *cannot* destroy the window immediately, nor can it wait for
> it to be released, which would cause a dead lock (been there done that).
More information about the vlc-devel