From 7f717d69f39e3be46972fb9a462748424b09d0c3 Mon Sep 17 00:00:00 2001 From: Pierre d'Herbemont Date: Tue, 15 Jul 2008 09:17:37 +0200 Subject: [PATCH] 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 | 43 ++++++--------- src/playlist/engine.c | 90 ++++++++++++++++++++------------ src/playlist/item.c | 5 +- src/playlist/playlist_internal.h | 9 ++++ src/playlist/search.c | 6 +-- 5 files changed, 87 insertions(+), 66 deletions(-) diff --git a/src/playlist/control.c b/src/playlist/control.c index 0b7ae07713..fbbf9412e8 100644 --- a/src/playlist/control.c +++ b/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; diff --git a/src/playlist/engine.c b/src/playlist/engine.c index 5a6a90e7fc..8ecbea4f72 100644 --- a/src/playlist/engine.c +++ b/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 ); diff --git a/src/playlist/item.c b/src/playlist/item.c index 39f4ead6e2..66573c98c6 100644 --- a/src/playlist/item.c +++ b/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 ) diff --git a/src/playlist/playlist_internal.h b/src/playlist/playlist_internal.h index 070de3fad5..155dae0851 100644 --- a/src/playlist/playlist_internal.h +++ b/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 */ diff --git a/src/playlist/search.c b/src/playlist/search.c index 0cd55cadd3..5d9238a22f 100644 --- a/src/playlist/search.c +++ b/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++ ) -- 2.39.2