[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 13:11:28 CET 2019
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. For now the function is not
static void vout_display_window_CloseNotify(vout_window_t *window)
/* TODO: Nowhere to dispatch to currently.
* Needs callback to ES output to deselect ES? */
msg_Err(window, "window closed");
Implementing this function would kill two birds with one stone. When
terminating vlc, GUI modules with embedded videos can use it to properly
terminate vout. But, more generally, this closing of windows can occur
at any time, and it is wise to also properly terminate vout if this occurs.
When saying 'terminate vout', it means terminate and desallocate the
resource. This is a case where the window id must not be retained for a
possible future use. In other words, as the result of issueing
vout_window_ReportClose(), the response must be vout_window_Close().
Also, this is not only related to the player. It must be a more generic
thing because vout can also be an audio visualization or anything else.
Maybe the caller of vout_Create() has to provide a callback to deal with
> I wrote part of it that I shared as a github branch some
> weeks ago, but it is not complete yet. I had also explored
> alternative solution like a vlc_player_disable-like
> mechanism (that I called vlc_player_ResetResource iirc) but
> I find it not very elegant.
Yet, as of today, when terminating vlc, it is not clear who is
responsible for just properly stopping the playlist. I realized that
deleting the playlist doesn't stop the decoders for instance. The Qt
interface stops the playlist in MainInterface::closeEvent()
(modules/gui/qt/main_interface.cpp) which is a weird place. For lack of
a better place, the patch I wrote was just mimicking Qt because stopping
the playlist has to be done somewhere.
I actually think it is the libvlc overall responsability to stop the
playlist since starting the playlist can be done very early in the
process without any GUI already set up.
this vlc_playlist_disable would be very simple, lock the playlist, stop
it, unlock it + an enabled flag to prevent any new starting. And this is
just triggering a stop. The actual stop will happen much later when the
vout are indeed released by a proper desallocation mechanism between the
GUI and the vout core and all the listeners on the player/vout are
released by the GUI.
> Is skins2 interface embedded work using interface_widget
> from the Qt interface?
No, skins2 doesn't rely on Qt for vout windows. The Qt thing is only
dialog boxes, menus and hotkeys management (including key accelerators
from Qt). Yet, the full Qt Playlist and Player controllers are set up
which means that skins2 is highly dependent on Qt indirectly.
> Alexandre Janniaux
> On Fri, Nov 29, 2019 at 11:13:21PM +0100, erwan.tulou at gmail.com wrote:
>> On 29/11/2019 20:09, Rémi Denis-Courmont wrote:
>>> Le perjantaina 29. marraskuuta 2019, 20.43.29 EET Erwan Tulou a écrit :
>>>> vlc | branch: master | Erwan Tulou <erwan10 at videolan.org> | Fri Nov 29
>>>> 19:32:25 2019 +0100| [0594abc06cb1a57548a7c3bf977e3071e64ef022] |
>>>> committer: Erwan Tulou
>>>> skins2: ensure playlist gets stopped before terminating vlc.
>>>> modules/gui/skins2/src/skin_main.cpp | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>> diff --git a/modules/gui/skins2/src/skin_main.cpp
>>>> b/modules/gui/skins2/src/skin_main.cpp index 087dd82cc0..d52e904d8d 100644
>>>> --- a/modules/gui/skins2/src/skin_main.cpp
>>>> +++ b/modules/gui/skins2/src/skin_main.cpp
>>>> @@ -143,6 +143,12 @@ static void Close( vlc_object_t *p_this )
>>>> msg_Dbg( p_intf, "closing skins2 module" );
>>>> + // ensure the playlist is stopped
>>>> + vlc_playlist_t *playlist = vlc_intf_GetMainPlaylist( p_intf );
>>>> + vlc_playlist_Lock( playlist );
>>>> + vlc_playlist_Stop ( playlist );
>>>> + vlc_playlist_Unlock( playlist );
>>> This does not guarantee that the playlist is stopped, only that it was stopped
>>> until the Unlock. This whole patch does not make sense to me.
>> Yes, this is a lame way to work out a problem and it actually only solve one
>> part of the problem. (so far, the Qt interface does not offer a better
>> solution in that matter).
>> Ideally, once libvlc_Quit() is issued, the right order of disabling/removing
>> thing seems the following :
>> - disable the playlist (which means : stop all inputs, free all resources
>> such as vout and make sure it cannot start again)
>> - stop and desallocate the interfaces
>> - desallocate the playlist
>> In particular, it is important that all vouts be freed __before__ the
>> interfaces are gone, because the window ids have to remain valid (embedded
>> video) as long as the vout are alive.
>> maybe coding a vlc_playlist_disable() to be called first thing is the
>> solution ... any suggestions ?
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the vlc-devel