[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