playlist: Use an accessor for current status item and current status node.

That may fix or help to fix some input item leaks.

src/playlist/control.c

@@ -122,8 +122,8 @@ static int PlaylistVAControl( playlist_t * p_playlist, int i_query, va_list args
         {
             p_playlist->request.i_status = PLAYLIST_RUNNING;
             p_playlist->request.b_request = true;
-            p_playlist->request.p_node = p_playlist->status.p_node;
-            p_playlist->request.p_item = p_playlist->status.p_item;
+            p_playlist->request.p_node = get_current_status_node( p_playlist );
+            p_playlist->request.p_item = get_current_status_item( p_playlist );
             p_playlist->request.i_skip = 0;
         }
         break;
@@ -154,8 +154,8 @@ static int PlaylistVAControl( playlist_t * p_playlist, int i_query, va_list args
         break;
 
     case PLAYLIST_SKIP:
-        p_playlist->request.p_node = p_playlist->status.p_node;
-        p_playlist->request.p_item = p_playlist->status.p_item;
+        p_playlist->request.p_node = get_current_status_node( p_playlist );
+        p_playlist->request.p_item = get_current_status_item( p_playlist );
         p_playlist->request.i_skip = (int) va_arg( args, int );
         /* if already running, keep running */
         if( p_playlist->status.i_status != PLAYLIST_STOPPED )
@@ -356,10 +356,10 @@ playlist_item_t * playlist_NextItem( playlist_t *p_playlist )
 
     /* Repeat and play/stop */
     if( !p_playlist->request.b_request && b_repeat == true &&
-         p_playlist->status.p_item )
+         get_current_status_item( p_playlist ) )
     {
         msg_Dbg( p_playlist,"repeating item" );
-        return p_playlist->status.p_item;
+        return get_current_status_item( p_playlist );
     }
     if( !p_playlist->request.b_request && b_playstop == true )
     {
@@ -367,9 +367,10 @@ playlist_item_t * playlist_NextItem( playlist_t *p_playlist )
         return NULL;
     }
 
-    if( !p_playlist->request.b_request && p_playlist->status.p_item )
+    if( !p_playlist->request.b_request &&
+        get_current_status_item( p_playlist ) )
     {
-        playlist_item_t *p_parent = p_playlist->status.p_item;
+        playlist_item_t *p_parent = get_current_status_item( p_playlist );
         while( p_parent )
         {
             if( p_parent->i_flags & PLAYLIST_SKIP_FLAG )
@@ -390,24 +391,12 @@ playlist_item_t * playlist_NextItem( playlist_t *p_playlist )
                         PLI_NAME( p_playlist->request.p_item ),
                         PLI_NAME( p_playlist->request.p_node ), i_skip );
 
-        /* Make sure the node wasn't deleted */
-        if( p_playlist->status.p_node &&
-            p_playlist->status.p_node->i_flags & PLAYLIST_REMOVE_FLAG )
-        {
-             PL_DEBUG( "%s was marked for deletion, deleting",
-                             PLI_NAME( p_playlist->status.p_node  ) );
-             playlist_ItemDelete( p_playlist->status.p_node );
-             /* Don't attempt to reuse that node */
-             if( p_playlist->status.p_node == p_playlist->request.p_node )
-                p_playlist->request.p_node = NULL;
-             p_playlist->status.p_node = NULL;
-        }
-
         if( p_playlist->request.p_node &&
-            p_playlist->request.p_node != p_playlist->status.p_node )
+            p_playlist->request.p_node != get_current_status_node( p_playlist ) )
         {
 
-            p_playlist->status.p_node = p_playlist->request.p_node;
+            set_current_status_node( p_playlist, p_playlist->request.p_node );
+            p_playlist->request.p_node = NULL;
             p_playlist->b_reset_currently_playing = true;
         }
 
@@ -462,13 +451,13 @@ playlist_item_t * playlist_NextItem( playlist_t *p_playlist )
         PL_DEBUG( "changing item without a request (current %i/%i)",
                   p_playlist->i_current_index, p_playlist->current.i_size );
         /* Cant go to next from current item */
-        if( p_playlist->status.p_item &&
-            p_playlist->status.p_item->i_flags & PLAYLIST_SKIP_FLAG )
+        if( get_current_status_item( p_playlist ) &&
+            get_current_status_item( p_playlist )->i_flags & PLAYLIST_SKIP_FLAG )
             return NULL;
 
         if( p_playlist->b_reset_currently_playing )
             ResetCurrentlyPlaying( p_playlist, b_random,
-                                   p_playlist->status.p_item );
+                                   get_current_status_item( p_playlist ) );
 
         p_playlist->i_current_index++;
         assert( p_playlist->i_current_index <= p_playlist->current.i_size );
@@ -504,7 +493,7 @@ int playlist_PlayItem( playlist_t *p_playlist, playlist_item_t *p_item )
     msg_Dbg( p_playlist, "creating new input thread" );
 
     p_input->i_nb_played++;
-    p_playlist->status.p_item = p_item;
+    set_current_status_item( p_playlist, p_item );
 
     p_playlist->status.i_status = PLAYLIST_RUNNING;
 

src/playlist/engine.c

@@ -277,6 +277,54 @@ input_thread_t * playlist_CurrentInput( playlist_t * p_playlist )
  * @}
  */
 
+/** Accessor for status item and status nodes.
+ */
+playlist_item_t * get_current_status_item( playlist_t * p_playlist )
+{
+    PL_ASSERT_LOCKED;
+
+    return p_playlist->status.p_item;
+}
+
+playlist_item_t * get_current_status_node( playlist_t * p_playlist )
+{
+    PL_ASSERT_LOCKED;
+
+    return p_playlist->status.p_node;
+}
+
+void set_current_status_item( playlist_t * p_playlist,
+    playlist_item_t * p_item )
+{
+    PL_ASSERT_LOCKED;
+
+    if( p_playlist->status.p_item &&
+        p_playlist->status.p_item->i_flags & PLAYLIST_REMOVE_FLAG &&
+        p_playlist->status.p_item != p_item )
+    {
+         PL_DEBUG( "%s was marked for deletion, deleting",
+                         PLI_NAME( p_playlist->status.p_item  ) );
+         playlist_ItemDelete( p_playlist->status.p_item );
+    }
+    p_playlist->status.p_item = p_item;
+}
+
+void set_current_status_node( playlist_t * p_playlist,
+    playlist_item_t * p_node )
+{
+    PL_ASSERT_LOCKED;
+
+    if( p_playlist->status.p_node &&
+        p_playlist->status.p_node->i_flags & PLAYLIST_REMOVE_FLAG &&
+        p_playlist->status.p_node != p_node )
+    {
+         PL_DEBUG( "%s was marked for deletion, deleting",
+                         PLI_NAME( p_playlist->status.p_node  ) );
+         playlist_ItemDelete( p_playlist->status.p_node );
+    }
+    p_playlist->status.p_node = p_node;
+}
+
 /**
  * Main loop
  *
@@ -294,7 +342,7 @@ void playlist_MainLoop( playlist_t *p_playlist )
         mdate() - p_playlist->last_rebuild_date > 30000 ) // 30 ms
     {
         ResetCurrentlyPlaying( p_playlist, var_GetBool( p_playlist, "random" ),
-                             p_playlist->status.p_item );
+                               get_current_status_item( p_playlist ) );
         p_playlist->last_rebuild_date = mdate();
     }
 
@@ -330,16 +378,7 @@ check_input:
             p_playlist->gc_date = mdate();
             p_playlist->b_cant_sleep = true;
 
-            if( p_playlist->status.p_item->i_flags
-                & PLAYLIST_REMOVE_FLAG )
-            {
-                 PL_DEBUG( "%s was marked for deletion, deleting",
-                                 PLI_NAME( p_playlist->status.p_item  ) );
-                 playlist_ItemDelete( p_playlist->status.p_item );
-                 if( p_playlist->request.p_item == p_playlist->status.p_item )
-                     p_playlist->request.p_item = NULL;
-                 p_playlist->status.p_item = NULL;
-            }
+            set_current_status_item( p_playlist, NULL );
 
             i_activity= var_GetInteger( p_playlist, "activity" );
             var_SetInteger( p_playlist, "activity", i_activity -
@@ -408,13 +447,7 @@ check_input:
             const bool b_gc_forced = p_playlist->status.i_status != PLAYLIST_STOPPED;
 
             p_playlist->status.i_status = PLAYLIST_STOPPED;
-            if( p_playlist->status.p_item &&
-                p_playlist->status.p_item->i_flags & PLAYLIST_REMOVE_FLAG )
-            {
-                PL_DEBUG( "deleting item marked for deletion" );
-                playlist_ItemDelete( p_playlist->status.p_item );
-                p_playlist->status.p_item = NULL;
-            }
+            set_current_status_item( p_playlist, NULL );
 
             /* Collect garbage */
             PL_UNLOCK;
@@ -494,22 +527,11 @@ void playlist_LastLoop( playlist_t *p_playlist )
 
     PL_LOCK;
 
-    if( p_playlist->status.p_node &&
-        p_playlist->status.p_node->i_flags & PLAYLIST_REMOVE_FLAG )
-    {
-         PL_DEBUG( "%s was marked for deletion, deleting",
-                         PLI_NAME( p_playlist->status.p_node  ) );
-         playlist_ItemDelete( p_playlist->status.p_node );
-         p_playlist->status.p_node = NULL;
-    }
-    if( p_playlist->status.p_item &&
-        p_playlist->status.p_item->i_flags & PLAYLIST_REMOVE_FLAG )
-    {
-         PL_DEBUG( "%s was marked for deletion, deleting",
-                         PLI_NAME( p_playlist->status.p_item  ) );
-         playlist_ItemDelete( p_playlist->status.p_item );
-         p_playlist->status.p_item = NULL;
-    }
+    /* Release the current node */
+    set_current_status_node( p_playlist, NULL );
+
+    /* Release the current item */
+    set_current_status_item( p_playlist, NULL );
 
     FOREACH_ARRAY( playlist_item_t *p_del, p_playlist->all_items )
         free( p_del->pp_children );

src/playlist/item.c

@@ -80,7 +80,8 @@ static void input_item_subitem_added( const vlc_event_t * p_event,
             return;
         }
 
-        b_play = b_play && p_item_in_category == p_playlist->status.p_item;
+        b_play = b_play &&
+            p_item_in_category == get_current_status_item( p_playlist );
 
         /* If this item is already a node don't transform it */
         if( p_item_in_category->i_children == -1 )
@@ -884,7 +885,7 @@ static int DeleteInner( playlist_t * p_playlist, playlist_item_t *p_item,
         ARRAY_REMOVE( p_playlist->items, i );
 
     /* Check if it is the current item */
-    if( p_playlist->status.p_item == p_item )
+    if( get_current_status_item( p_playlist ) == p_item )
     {
         /* Hack we don't call playlist_Control for lock reasons */
         if( b_stop )

src/playlist/playlist_internal.h

@@ -78,6 +78,11 @@ void playlist_FetcherLoop( playlist_fetcher_t * );
 
 void ResetCurrentlyPlaying( playlist_t *, bool, playlist_item_t * );
 
+playlist_item_t * get_current_status_item( playlist_t * p_playlist);
+playlist_item_t * get_current_status_node( playlist_t * p_playlist );
+void set_current_status_item( playlist_t *, playlist_item_t * );
+void set_current_status_node( playlist_t *, playlist_item_t * );
+
 /* Control */
 playlist_item_t * playlist_NextItem  ( playlist_t * );
 int playlist_PlayItem  ( playlist_t *, playlist_item_t * );
@@ -137,4 +142,8 @@ void playlist_set_current_input(
 #endif
 
 #define PLI_NAME( p ) p && p->p_input ? p->p_input->psz_name : "null"
+
+#define PL_ASSERT_LOCKED vlc_assert_locked( &(vlc_internals(p_playlist)->lock) );
+
+
 #endif /* !__LIBVLC_PLAYLIST_INTERNAL_H */

src/playlist/search.c

@@ -68,11 +68,11 @@ playlist_item_t * playlist_ItemGetByInput( playlist_t * p_playlist ,
 {
     int i;
     if( !b_locked ) PL_LOCK;
-    if( p_playlist->status.p_item &&
-        p_playlist->status.p_item->p_input == p_item )
+    if( get_current_status_item( p_playlist ) &&
+        get_current_status_item( p_playlist )->p_input == p_item )
     {
         if( !b_locked ) PL_UNLOCK;
-        return p_playlist->status.p_item;
+        return get_current_status_item( p_playlist );
     }
     /** \todo Check if this is always incremental and whether we can bsearch */
     for( i =  0 ; i < p_playlist->all_items.i_size; i++ )