[vlc-devel] [PATCH 1/2] Fixing recents not updating with cli

Gal Vinograd bl3nderb at aol.com
Thu Nov 28 08:26:19 CET 2013


Recents playlist is now updating with command line arguments

After taking Rémi Denis-Courmont's code review under considuration

I think the memory leak that you described does not occur,
the only call to this function (playlist_Create) is pl_Get and if playlist_Create returns null, pl_Get abort the entire application,
similar allocations such as p_playlist->p_playing coded in the same way in this function
---
 include/vlc_playlist.h           | 14 +++++++++++
 src/libvlccore.sym               |  2 ++
 src/playlist/engine.c            | 18 +++++++++++---
 src/playlist/item.c              |  7 ++++++
 src/playlist/loadsave.c          | 37 +++++++++++++++-------------
 src/playlist/playlist_internal.h |  7 +++---
 src/playlist/tree.c              | 53 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 114 insertions(+), 24 deletions(-)

diff --git a/include/vlc_playlist.h b/include/vlc_playlist.h
index 10108cb..296cc5b 100644
--- a/include/vlc_playlist.h
+++ b/include/vlc_playlist.h
@@ -173,6 +173,7 @@ struct playlist_t
     playlist_item_t *     p_root;
     playlist_item_t *     p_playing;
     playlist_item_t *     p_media_library;
+    playlist_item_t *     p_recents;
 
     //Phony ones, point to those above;
     playlist_item_t *     p_root_category; /**< Root of category tree */
@@ -190,6 +191,9 @@ struct playlist_add_t
     int i_item; /**< Playist id of the playlist_item_t */
 };
 
+/** Recents limit */
+#define VLC_PLAYLIST_RECENTS_LIMIT 5
+
 /* A bit of macro magic to generate an enum out of the following list,
  * and later, to generate a list of static functions out of the same list.
  * There is also SORT_RANDOM, which is always last and handled specially.
@@ -262,6 +266,12 @@ VLC_API playlist_t * pl_Get( vlc_object_t * );
 #define playlist_Prev(p) playlist_Control(p,PLAYLIST_SKIP, pl_Unlocked, -1)
 #define playlist_Skip(p,i) playlist_Control(p,PLAYLIST_SKIP, pl_Unlocked,  (i) )
 
+#define playlist_MLDump(p) playlist_PlaylistDump( p, p->p_media_library, "ml.xspf", _("Media Library") )
+#define playlist_RecentsDump(p) playlist_PlaylistDump( p, p->p_recents, "recents.xspf", _("Recents") )
+
+#define playlist_MLLoad(p) playlist_PlaylistLoad( p, p->p_media_library, "ml.xspf", _("Media Library") )
+#define playlist_RecentsLoad(p) playlist_PlaylistLoad( p, p->p_recents, "recents.xspf", _("Recents") )
+
 VLC_API void playlist_Lock( playlist_t * );
 VLC_API void playlist_Unlock( playlist_t * );
 VLC_API void playlist_AssertLocked( playlist_t * );
@@ -321,6 +331,10 @@ VLC_API int playlist_Export( playlist_t *p_playlist, const char *psz_name, playl
  */
 VLC_API int playlist_Import( playlist_t *p_playlist, const char *psz_file );
 
+VLC_API int playlist_PlaylistDump( playlist_t *p_playlist, playlist_item_t *p_node, char *psz_filename, char *psz_name );
+VLC_API int playlist_PlaylistLoad( playlist_t *p_playlist, playlist_item_t *p_node, char *psz_filename, char *psz_name );
+
+
 /********************** Services discovery ***********************/
 
 /** Add a list of comma-separated service discovery modules */
diff --git a/src/libvlccore.sym b/src/libvlccore.sym
index 5efc67e..2d82546 100644
--- a/src/libvlccore.sym
+++ b/src/libvlccore.sym
@@ -347,6 +347,8 @@ playlist_VolumeSet
 playlist_VolumeUp
 playlist_MuteSet
 playlist_MuteGet
+playlist_PlaylistLoad
+playlist_PlaylistDump
 pl_Get
 resolve_xml_special_chars
 sdp_AddAttribute
diff --git a/src/playlist/engine.c b/src/playlist/engine.c
index 8207c79..9ad17af 100644
--- a/src/playlist/engine.c
+++ b/src/playlist/engine.c
@@ -203,7 +203,7 @@ static playlist_t *playlist_Create( vlc_object_t *p_parent )
     /* Allocate structure */
     p = vlc_custom_create( p_parent, sizeof( *p ), "playlist" );
     if( !p )
-        return NULL;
+        return NULL; //No need to free the memory, the application gets aborted
 
     assert( offsetof( playlist_private_t, public_data ) == 0 );
     p_playlist = &p->public_data;
@@ -248,7 +248,7 @@ static playlist_t *playlist_Create( vlc_object_t *p_parent )
     p_playlist->p_root = playlist_NodeCreate( p_playlist, NULL, NULL,
                                     PLAYLIST_END, 0, NULL );
     PL_UNLOCK;
-    if( !p_playlist->p_root ) return NULL;
+    if( !p_playlist->p_root ) return NULL; //No need to free the memory, the application gets aborted
 
     /* Create currently playing items node */
     PL_LOCK;
@@ -258,7 +258,19 @@ static playlist_t *playlist_Create( vlc_object_t *p_parent )
 
     PL_UNLOCK;
 
-    if( !p_playlist->p_playing ) return NULL;
+    if( !p_playlist->p_playing ) return NULL; //No need to free the memory, the application gets aborted
+    /* Create recently played items node */
+    PL_LOCK;
+    p_playlist->p_recents = playlist_NodeCreate(
+        p_playlist, _( "Recents" ), p_playlist->p_root,
+        PLAYLIST_END, PLAYLIST_RO_FLAG, NULL );
+
+
+    PL_UNLOCK;
+
+    if( !p_playlist->p_recents ) return NULL; //No need to free the memory, the application gets aborted
+
+    playlist_RecentsLoad( p_playlist );
 
     /* Create media library node */
     const bool b_ml = var_InheritBool( p_parent, "media-library");
diff --git a/src/playlist/item.c b/src/playlist/item.c
index e902063..5ce563e 100644
--- a/src/playlist/item.c
+++ b/src/playlist/item.c
@@ -464,6 +464,10 @@ int playlist_AddInput( playlist_t* p_playlist, input_item_t *p_input,
         PL_UNLOCK_IF( !b_locked );
         return VLC_ENOMEM;
     }
+
+    playlist_NodeAppendOrMove( p_playlist, p_item, p_playlist->p_recents );
+    playlist_NodeCrop( p_playlist, p_playlist->p_recents, VLC_PLAYLIST_RECENTS_LIMIT, true );
+
     AddItem( p_playlist, p_item,
              b_playlist ? p_playlist->p_playing :
                           p_playlist->p_media_library , i_mode, i_pos );
@@ -471,6 +475,9 @@ int playlist_AddInput( playlist_t* p_playlist, input_item_t *p_input,
     GoAndPreparse( p_playlist, i_mode, p_item );
 
     PL_UNLOCK_IF( !b_locked );
+
+    playlist_RecentsDump( p_playlist );
+
     return VLC_SUCCESS;
 }
 
diff --git a/src/playlist/loadsave.c b/src/playlist/loadsave.c
index 85a3e9e..2286661 100644
--- a/src/playlist/loadsave.c
+++ b/src/playlist/loadsave.c
@@ -105,35 +105,37 @@ int playlist_Import( playlist_t *p_playlist, const char *psz_file )
 static void input_item_subitem_tree_added( const vlc_event_t * p_event,
                                       void * user_data )
 {
-    playlist_t *p_playlist = user_data;
+    playlist_item_t *p_node = user_data;
+    playlist_t *p_playlist = p_node->p_playlist;
     input_item_node_t *p_root =
         p_event->u.input_item_subitem_tree_added.p_root;
 
     PL_LOCK;
-    playlist_InsertInputItemTree ( p_playlist, p_playlist->p_media_library,
+    playlist_InsertInputItemTree ( p_playlist, p_node,
                                    p_root, 0, false );
     PL_UNLOCK;
 }
 
-int playlist_MLLoad( playlist_t *p_playlist )
+int playlist_PlaylistLoad( playlist_t *p_playlist, playlist_item_t *p_node, char *psz_filename, char *psz_name )
 {
     input_item_t *p_input;
 
     char *psz_datadir = config_GetUserDir( VLC_DATA_DIR );
     if( !psz_datadir ) /* XXX: This should never happen */
     {
-        msg_Err( p_playlist, "no data directory, cannot load media library") ;
+        msg_Err( p_playlist, "no data directory, cannot load %s", psz_name );
         return VLC_EGENERIC;
     }
 
     char *psz_file;
-    if( asprintf( &psz_file, "%s" DIR_SEP "ml.xspf", psz_datadir ) == -1 )
+
+    if( asprintf( &psz_file, "%s" DIR_SEP "%s", psz_datadir, psz_filename ) == -1 )
         psz_file = NULL;
     free( psz_datadir );
     if( psz_file == NULL )
         return VLC_ENOMEM;
 
-    /* lousy check for media library file */
+    /* lousy check for the file */
     struct stat st;
     if( vlc_stat( psz_file, &st ) )
     {
@@ -149,20 +151,20 @@ int playlist_MLLoad( playlist_t *p_playlist )
     const char *const options[1] = { "meta-file", };
     /* that option has to be cleaned in input_item_subitem_tree_added() */
     /* vlc_gc_decref() in the same function */
-    p_input = input_item_NewExt( psz_uri, _("Media Library"),
+    p_input = input_item_NewExt( psz_uri, psz_name,
                                  1, options, VLC_INPUT_OPTION_TRUSTED, -1 );
     free( psz_uri );
     if( p_input == NULL )
         return VLC_EGENERIC;
 
     PL_LOCK;
-    if( p_playlist->p_media_library->p_input )
-        vlc_gc_decref( p_playlist->p_media_library->p_input );
+    if( p_node->p_input )
+        vlc_gc_decref( p_node->p_input );
 
-    p_playlist->p_media_library->p_input = p_input;
+    p_node->p_input = p_input;
 
     vlc_event_attach( &p_input->event_manager, vlc_InputItemSubItemTreeAdded,
-                        input_item_subitem_tree_added, p_playlist );
+                        input_item_subitem_tree_added, p_node );
 
     pl_priv(p_playlist)->b_doing_ml = true;
     PL_UNLOCK;
@@ -174,12 +176,12 @@ int playlist_MLLoad( playlist_t *p_playlist )
     PL_UNLOCK;
 
     vlc_event_detach( &p_input->event_manager, vlc_InputItemSubItemTreeAdded,
-                        input_item_subitem_tree_added, p_playlist );
+                        input_item_subitem_tree_added, p_node );
 
     return VLC_SUCCESS;
 }
 
-int playlist_MLDump( playlist_t *p_playlist )
+int playlist_PlaylistDump( playlist_t *p_playlist, playlist_item_t *p_node, char *psz_filename, char *psz_name )
 {
     char *psz_datadir;
 
@@ -187,11 +189,11 @@ int playlist_MLDump( playlist_t *p_playlist )
 
     if( !psz_datadir ) /* XXX: This should never happen */
     {
-        msg_Err( p_playlist, "no data directory, cannot save media library") ;
+        msg_Err( p_playlist, "no data directory, cannot save" );
         return VLC_EGENERIC;
     }
 
-    char psz_dirname[ strlen( psz_datadir ) + sizeof( DIR_SEP "ml.xspf")];
+    char psz_dirname[ strlen( psz_datadir ) + sizeof( DIR_SEP ) + strlen( psz_filename ) ];
     strcpy( psz_dirname, psz_datadir );
     free( psz_datadir );
     if( config_CreateDir( (vlc_object_t *)p_playlist, psz_dirname ) )
@@ -199,9 +201,10 @@ int playlist_MLDump( playlist_t *p_playlist )
         return VLC_EGENERIC;
     }
 
-    strcat( psz_dirname, DIR_SEP "ml.xspf" );
+    strcat( psz_dirname, DIR_SEP );
+    strcat( psz_dirname, psz_filename );
 
-    playlist_Export( p_playlist, psz_dirname, p_playlist->p_media_library,
+    playlist_Export( p_playlist, psz_dirname, p_node,
                      "export-xspf" );
 
     return VLC_SUCCESS;
diff --git a/src/playlist/playlist_internal.h b/src/playlist/playlist_internal.h
index 9b6c000..ecd8ee2 100644
--- a/src/playlist/playlist_internal.h
+++ b/src/playlist/playlist_internal.h
@@ -112,10 +112,6 @@ 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 * );
 
-/* Load/Save */
-int playlist_MLLoad( playlist_t *p_playlist );
-int playlist_MLDump( playlist_t *p_playlist );
-
 /**********************************************************************
  * Item management
  **********************************************************************/
@@ -145,6 +141,9 @@ int playlist_DeleteItem( playlist_t * p_playlist, playlist_item_t *, bool);
 void ResetCurrentlyPlaying( playlist_t *p_playlist, playlist_item_t *p_cur );
 void ResyncCurrentIndex( playlist_t *p_playlist, playlist_item_t *p_cur );
 
+int playlist_NodeAppendOrMove( playlist_t *p_playlist, playlist_item_t *p_item, playlist_item_t *p_parent );
+
+void playlist_NodeCrop( playlist_t *p_playlist, playlist_item_t *p_parent, int i_limit, bool b_delete_first );
 /**
  * @}
  */
diff --git a/src/playlist/tree.c b/src/playlist/tree.c
index eca12b1..573e61a 100644
--- a/src/playlist/tree.c
+++ b/src/playlist/tree.c
@@ -24,6 +24,8 @@
 # include "config.h"
 #endif
 
+#include <math.h>
+
 #include <vlc_common.h>
 #include <assert.h>
 #include "vlc_playlist.h"
@@ -226,6 +228,57 @@ int playlist_NodeInsert( playlist_t *p_playlist,
 }
 
 /**
+ * Append item to a node or move an existing to the top
+ *
+ * \param p_playlist the playlist
+ * \param p_item the item to append or move
+ * \param p_parent the parent nove
+ * \return VLC_SUCCESS or an error
+ */
+int playlist_NodeAppendOrMove( playlist_t *p_playlist,
+                         playlist_item_t *p_item,
+                         playlist_item_t *p_parent )
+{
+    PL_ASSERT_LOCKED;
+
+    for ( int i = 0; i < p_parent->i_children; i++ )
+    {
+        if ( !strcmp( p_parent->pp_children[i]->p_input->psz_uri, p_item->p_input->psz_uri ) )
+        {
+            p_item = p_parent->pp_children[i];
+            REMOVE_ELEM( p_parent->pp_children, p_parent->i_children, i );
+            break;
+        }
+    }
+
+    return playlist_NodeInsert( p_playlist, p_item, p_parent, -1 );
+}
+
+
+/**
+ * Cropping a playlist node to a specific length
+ *
+ * \param p_playlist the playlist
+ * \param p_parent the parent nove
+ * \param i_limit the new length of the playlist
+ * \param b_delete_first if true, start cropping from the first item
+ */
+void playlist_NodeCrop( playlist_t *p_playlist,
+                         playlist_item_t *p_parent,
+                         int i_limit,
+                         bool b_delete_first )
+{
+    PL_ASSERT_LOCKED;
+
+    i_limit = p_parent->i_children - i_limit;
+
+    for ( int i = 0; i < i_limit; i++ )
+    {
+        REMOVE_ELEM( p_parent->pp_children, p_parent->i_children, b_delete_first ? i : p_parent->i_children - 1 - i );
+    }
+}
+
+/**
  * Deletes an item from the children of a node
  *
  * \param p_playlist the playlist
-- 
1.8.3.2




More information about the vlc-devel mailing list