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

[erwan.tulou at gmail.com]

Hi all,

On 03/12/2019 14:14, Alexandre Janniaux wrote:
> Can we refocus on what will be correct to do instead of
> loosing time on what is wrong?

    After diving into the code, I think we are not that far away from a 
clean solution.

   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

More generally, any interface(embedded video or not) should be able to 
be started and stopped at any time. The playlist should not be forced to 
stop because an interface is stopped.

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.

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().

A recap of what we have at the vout_window level  :
- Open () (vout window module Open function)
- vout_window_enabled()
- vout_window_disabled()
- Close() (vout window module Close function)

I suggest that the window ID is valid only in the enabled phase. It is 
up to each vout window implementation to choose whether the same window 
ID is reused or a new window ID is provided when vout_window_enabled() 
is called.
Of course, when disabled, that does not  prevent the vout window module 
to continue advertizing for instance hotplugged new monitors or anything 
else, and also vlc core issuing commands as long as it is not the 
display accessing the window ID. What the vout window module does with 
the commands passed in disabled state is implementation dependent. (is 
it a no-op, does it retain the state for future uses, ... ?)

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

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

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
- vout_window_ReportClose() is key

comments/critics (no animosity) welcomed

Rgds
Erwan