[vlc-devel] [vlc-commits] playlist: playlist_NodeAddInput() asserts the playlist lock
Steve Lhomme
robux4 at gmail.com
Thu Nov 17 09:58:33 CET 2016
On Wed, Nov 16, 2016 at 11:05 PM, Rémi Denis-Courmont <git at videolan.org> wrote:
> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Wed Nov 16 23:12:27 2016 +0200| [3f8bbdd86859575d4ac0c42c017de4adf8944b4a] | committer: Rémi Denis-Courmont
>
> playlist: playlist_NodeAddInput() asserts the playlist lock
>
> As any function that requires a valid playlist item as parameter, the
> function can only make sense with the lock already held by the caller.
>
>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=3f8bbdd86859575d4ac0c42c017de4adf8944b4a
> ---
>
> include/vlc_playlist.h | 2 +-
> modules/gui/macosx/VLCPlaylist.m | 2 +-
> modules/gui/qt/recents.cpp | 9 +++++----
> modules/gui/skins2/vars/playtree.cpp | 2 +-
> src/playlist/item.c | 34 +++++++++++++---------------------
> src/playlist/services_discovery.c | 2 +-
> 6 files changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/include/vlc_playlist.h b/include/vlc_playlist.h
> index d0ee7b0..f6db8b1 100644
> --- a/include/vlc_playlist.h
> +++ b/include/vlc_playlist.h
> @@ -336,7 +336,7 @@ VLC_API int playlist_DeleteFromInput( playlist_t *, input_item_t *, bool );
> VLC_API int playlist_Add( playlist_t *, const char *, const char *, int, int, bool, bool );
> VLC_API int playlist_AddExt( playlist_t *, const char *, const char *, int, int, mtime_t, int, const char *const *, unsigned, bool, bool );
> VLC_API int playlist_AddInput( playlist_t *, input_item_t *, int, int, bool, bool );
> -VLC_API playlist_item_t * playlist_NodeAddInput( playlist_t *, input_item_t *, playlist_item_t *, int, int, bool );
> +VLC_API playlist_item_t * playlist_NodeAddInput( playlist_t *, input_item_t *, playlist_item_t *, int, int );
> VLC_API int playlist_NodeAddCopy( playlist_t *, playlist_item_t *, playlist_item_t *, int );
>
> /********************************** Item search *************************/
> diff --git a/modules/gui/macosx/VLCPlaylist.m b/modules/gui/macosx/VLCPlaylist.m
> index efa0e37..f9d264d 100644
> --- a/modules/gui/macosx/VLCPlaylist.m
> +++ b/modules/gui/macosx/VLCPlaylist.m
> @@ -714,7 +714,7 @@
>
> int i_pos = (i_position == -1) ? PLAYLIST_END : i_position + i_current_offset++;
> playlist_item_t *p_item = playlist_NodeAddInput(p_playlist, p_input, p_parent,
> - PLAYLIST_INSERT, i_pos, pl_Locked);
> + PLAYLIST_INSERT, i_pos);
> if (!p_item)
> continue;
>
> diff --git a/modules/gui/qt/recents.cpp b/modules/gui/qt/recents.cpp
> index 8d2fe6f..ff75e56 100644
> --- a/modules/gui/qt/recents.cpp
> +++ b/modules/gui/qt/recents.cpp
> @@ -158,9 +158,9 @@ void RecentsMRL::save()
>
> playlist_item_t *RecentsMRL::toPlaylist(int length)
> {
> - playlist_Lock(THEPL);
> + vlc_playlist_locker locker(THEPL);
> +
> playlist_item_t *p_node_recent = playlist_NodeCreate(THEPL, _("Recently Played"), THEPL->p_root, PLAYLIST_END, PLAYLIST_RO_FLAG);
> - playlist_Unlock(THEPL);
>
> if ( p_node_recent == NULL ) return NULL;
>
> @@ -170,10 +170,11 @@ playlist_item_t *RecentsMRL::toPlaylist(int length)
> for (int i = 0; i < length; i++)
> {
> input_item_t *p_input = input_item_New(qtu(recents.at(i)), NULL);
> - playlist_NodeAddInput(THEPL, p_input, p_node_recent, PLAYLIST_APPEND, PLAYLIST_END, false);
> + playlist_NodeAddInput(THEPL, p_input, p_node_recent, PLAYLIST_APPEND, PLAYLIST_END);
> }
>
> - return p_node_recent;
> + /* locker goes out of scope and node is invalidated here */
> + return NULL;
It's probably true so toPlaylist() always returns NULL, so
saveRecentsToPlaylist() never does anything now.
> }
>
> void RecentsMRL::playMRL( const QString &mrl )
> diff --git a/modules/gui/skins2/vars/playtree.cpp b/modules/gui/skins2/vars/playtree.cpp
> index eb9518d..38357f1 100644
> --- a/modules/gui/skins2/vars/playtree.cpp
> +++ b/modules/gui/skins2/vars/playtree.cpp
> @@ -351,7 +351,7 @@ void Playtree::insertItems( VarTree& elem, const std::list<std::string>& files,
> i_mode |= PLAYLIST_GO;
>
> playlist_NodeAddInput( m_pPlaylist, pItem, p_node,
> - i_mode, i_pos, pl_Locked );
> + i_mode, i_pos );
> }
>
> fin:
> diff --git a/src/playlist/item.c b/src/playlist/item.c
> index 9a11e6c..e2c25fe 100644
> --- a/src/playlist/item.c
> +++ b/src/playlist/item.c
> @@ -474,31 +474,24 @@ int playlist_AddInput( playlist_t* p_playlist, input_item_t *p_input,
> * \param i_pos the position in the playlist where to add. If this is
> * PLAYLIST_END the item will be added at the end of the playlist
> * regardless of its size
> - * \param b_locked TRUE if the playlist is locked
> * \return the new playlist item
> */
> playlist_item_t * playlist_NodeAddInput( playlist_t *p_playlist,
> input_item_t *p_input,
> playlist_item_t *p_parent,
> - int i_mode, int i_pos,
> - bool b_locked )
> + int i_mode, int i_pos )
> {
> - playlist_item_t *p_item;
> + PL_ASSERT_LOCKED;
> +
> assert( p_input );
> assert( p_parent && p_parent->i_children != -1 );
>
> - PL_LOCK_IF( !b_locked );
> -
> - p_item = playlist_ItemNewFromInput( p_playlist, p_input );
> - if( p_item == NULL )
> - goto end;
> - AddItem( p_playlist, p_item, p_parent, i_mode, i_pos );
> -
> - GoAndPreparse( p_playlist, i_mode, p_item );
> -
> -end:
> - PL_UNLOCK_IF( !b_locked );
> -
> + playlist_item_t *p_item = playlist_ItemNewFromInput( p_playlist, p_input );
> + if( likely(p_item != NULL) )
> + {
> + AddItem( p_playlist, p_item, p_parent, i_mode, i_pos );
> + GoAndPreparse( p_playlist, i_mode, p_item );
> + }
> return p_item;
> }
>
> @@ -819,10 +812,9 @@ static int RecursiveAddIntoParent (
> if( !(b_flat && b_children) )
> {
> p_new_item = playlist_NodeAddInput( p_playlist,
> - p_child_node->p_item,
> - p_parent,
> - PLAYLIST_INSERT, i_pos,
> - pl_Locked );
> + p_child_node->p_item,
> + p_parent,
> + PLAYLIST_INSERT, i_pos );
> if( !p_new_item ) return i_pos;
>
> i_pos++;
> @@ -872,7 +864,7 @@ static int RecursiveInsertCopy (
> {
> p_new_item = playlist_NodeAddInput( p_playlist, p_new_input,
> p_parent, PLAYLIST_INSERT,
> - i_pos, pl_Locked );
> + i_pos );
> vlc_gc_decref( p_new_input );
> }
> }
> diff --git a/src/playlist/services_discovery.c b/src/playlist/services_discovery.c
> index d4fdf10..6009348 100644
> --- a/src/playlist/services_discovery.c
> +++ b/src/playlist/services_discovery.c
> @@ -180,7 +180,7 @@ static void playlist_sd_item_added(services_discovery_t *sd,
> }
>
> playlist_NodeAddInput(playlist, p_input, parent,
> - PLAYLIST_APPEND, PLAYLIST_END, pl_Locked);
> + PLAYLIST_APPEND, PLAYLIST_END);
> playlist_Unlock(playlist);
> }
>
>
> _______________________________________________
> vlc-commits mailing list
> vlc-commits at videolan.org
> https://mailman.videolan.org/listinfo/vlc-commits
More information about the vlc-devel
mailing list