[vlc-devel] [vlc-commits] skins2: ensure playlist gets stopped before terminating vlc.

[Rémi Denis-Courmont]

Le tiistaina 3. joulukuuta 2019, 16.33.52 EET erwan.tulou at gmail.com a écrit :
>    here is what I think
> 
>    at vlc termination, desallocation should indeed be done in a lifo
> style, which means
>    first : stop and destroy all interfaces
>    second : stop and destroy playlist

That's how it works already and how it always worked, and it is just fine for 
control interfaces. It does not work for main interfaces because they need to 
configure the window allocation, the logging, etc, before the playlist starts.

Currently, it just works by accident because the non-main interfaces don't 
normally start the playlist - but they can and this leads to a race condition 
at start up.

> More generally, any interface(embedded video or not) should be able to
> be started and stopped at any time.

That's a completely pointless restriction if the playlist is started before 
the interface and stopped after them. And it breaks the documented use case of 
enabling the HTTP interface on the fly.

> That means that interfaces with embedded video must gracefully ask to
> regain their ressources (vout_window_ReportClose). Most likely, it means
> that the video ES goes unselected, the audio visualization goes
> unchecked, ... and the player just keeps working.

That's fundamentally racy. You can't prevent something else interfering with 
that logic: the demuxer adding another ES, or another control interface 
reselecting the ES.

You're trying to impose intractable requirements on everything else to avoid 
solving design and implementation issues in the main interfaces.

> At the vout window level, vout_window_Close() is **useless**. Today, it
> is simply used in place of the Close() function when unloading the vout
> window module. So, just remove it everywhere, and reinstate the Close().

What the heck? this changes exactly nothing, except for breaking type safety.

> In short, at the core level, what is needed is:
> - implementing vout_window_ReportClose() (today it ends up in msg_Err ..
> + no-op)

Whiel this should be fixed, it's a completely separate problem that solves 
exactly nothing in this context. You still have to consider the case that the 
video does not stem from the playlist and thus cannot be "stopped". Also what 
was already explained before.

> - just remove vout_window_Close() everywhere

Zero arguments. This is nothing but a red herring.

> for the vout_window module
> -  reinstate Close() function instead of responding to vout_window_Close()

Hell no.

> for GUI interface with vout window capabilities (Qt and skins2)
> - remove playlist forcefully being stopped at interface termination
> - gracefully ask to regain ressources (vout_window_ReportClose) whenever
> it is useful (and anyway needed at module termination whether because
> vlc is terminating or just the module gets unselected)
> 
> at the libvlc level,
> - a state-of-the-art desallocation (the lifo style) where the playlist
> is stopped and destroyed in fine after all interfaces have been stopped.
> 
> takeway:
> - vout_window_Close() is useless

BS.

> - vout_window_ReportClose() is key

BS.

-- 
雷米‧德尼-库尔蒙
http://www.remlab.net/