Clean up input_item_t functions and usages.

It fixes a bunch of missing locks and remove unused functions.
It splits input_internal.h

include/vlc_input.h

@@ -120,15 +120,11 @@ VLC_EXPORT( void, input_item_AddSubItem, ( input_item_t *p_parent, input_item_t
 /* Flags handled within input_item_AddOpt() */
 #define VLC_INPUT_OPTION_UNIQUE  0x100
 
-VLC_EXPORT( int, input_item_AddOpt, ( input_item_t *, const char *str, unsigned flags ) );
-VLC_EXPORT( int, input_item_AddOption, (input_item_t *item, const char *str) );
-VLC_EXPORT( int ,input_item_AddOption, (input_item_t *item, const char *str) );
-VLC_EXPORT( bool,input_item_HasErrorWhenReading, (input_item_t *item) );
-VLC_EXPORT( void, input_item_SetMeta, ( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz_val ));
-
-VLC_EXPORT( bool,input_item_HasErrorWhenReading, (input_item_t *item) );
-
-VLC_EXPORT( bool,input_item_MetaMatch, ( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz ) );
+VLC_EXPORT( int,  input_item_AddOpt, ( input_item_t *, const char *str, unsigned flags ) );
+VLC_EXPORT( int,  input_item_AddOption, (input_item_t *, const char * ) );
+VLC_EXPORT( bool, input_item_HasErrorWhenReading, ( input_item_t * ) );
+VLC_EXPORT( void, input_item_SetMeta, ( input_item_t *, vlc_meta_type_t meta_type, const char *psz_val ));
+VLC_EXPORT( bool, input_item_MetaMatch, ( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz ) );
 VLC_EXPORT( char *, input_item_GetMeta, ( input_item_t *p_i, vlc_meta_type_t meta_type ) );
 VLC_EXPORT( char *, input_item_GetName, ( input_item_t * p_i ) );
 VLC_EXPORT( char *, input_item_GetURI, ( input_item_t * p_i ) );
@@ -137,8 +133,6 @@ VLC_EXPORT(mtime_t, input_item_GetDuration, ( input_item_t * p_i ) );
 VLC_EXPORT( void,   input_item_SetDuration, ( input_item_t * p_i, mtime_t i_duration ));
 VLC_EXPORT( bool,   input_item_IsPreparsed, ( input_item_t *p_i ));
 VLC_EXPORT( bool,   input_item_IsArtFetched, ( input_item_t *p_i ));
-VLC_EXPORT( const vlc_meta_t *, input_item_GetMetaObject, ( input_item_t *p_i ));
-VLC_EXPORT( void,   input_item_MetaMerge, ( input_item_t *p_i, const vlc_meta_t * p_new_meta ));
 
 
 #define input_item_SetTitle( item, b )       input_item_SetMeta( item, vlc_meta_Title, b )
@@ -419,7 +413,10 @@ typedef enum input_state_e
 #define INPUT_UPDATE_META       0x0040
 #define INPUT_UPDATE_SIGNAL     0x0080
 
-/** Get the input item for an input thread */
+/** Get the input item for an input thread
+ * FIXME see src/input/item.c but is is unsafe unless
+ * you hold p_input
+ */
 VLC_EXPORT(input_item_t*, input_GetItem, (input_thread_t*));
 
 typedef struct input_thread_private_t input_thread_private_t;

src/Makefile.am

@@ -322,6 +322,7 @@ SOURCES_libvlc_common = \
 	input/event.h \
 	input/stream.h \
 	input/input_internal.h \
+	input/input_interface.h \
 	input/vlm_internal.h \
 	input/stream.c \
 	input/stream_memory.c \

src/input/input.c

@@ -60,8 +60,8 @@
  *****************************************************************************/
 static void Destructor( input_thread_t * p_input );
 
-static  void* Run            ( vlc_object_t *p_this );
-static  void* RunAndDestroy  ( vlc_object_t *p_this );
+static  void *Run            ( vlc_object_t *p_this );
+static  void *RunAndDestroy  ( vlc_object_t *p_this );
 
 static input_thread_t * Create  ( vlc_object_t *, input_item_t *,
                                   const char *, bool, sout_instance_t * );
@@ -2791,20 +2791,13 @@ static void InputUpdateMeta( input_thread_t *p_input, vlc_meta_t *p_meta )
     }
     free( psz_arturl );
 
-    /* A bit ugly */
-    p_meta = NULL;
-    if( vlc_dictionary_keys_count( &p_item->p_meta->extra_tags ) > 0 )
-    {
-        p_meta = vlc_meta_New();
-        vlc_meta_Merge( p_meta, input_item_GetMetaObject( p_item ) );
-    }
+    vlc_mutex_unlock( &p_item->lock );
 
     if( psz_title )
+    {
         input_item_SetName( p_item, psz_title );
-    free( psz_title );
-
-    vlc_mutex_unlock( &p_item->lock );
-
+        free( psz_title );
+    }
     input_item_SetPreparsed( p_item, true );
 
     input_SendEventMeta( p_input );
@@ -2914,10 +2907,11 @@ static void input_ChangeState( input_thread_t *p_input, int i_state )
     else if( i_state == END_S )
         p_input->b_eof = true;
 
-    input_item_SetHasErrorWhenReading( p_input->p->input.p_item, (i_state == ERROR_S) );
-
     if( b_changed )
+    {
+        input_item_SetErrorWhenReading( p_input->p->input.p_item, p_input->b_error );
         input_SendEventState( p_input, i_state );
+    }
 }
 
 

src/input/input_interface.h

+/*****************************************************************************
+ * input_interface.h: Input functions usable ouside input code.
+ *****************************************************************************
+ * Copyright (C) 1998-2008 the VideoLAN team
+ * $Id$
+ *
+ * Authors: Laurent Aimar < fenrir _AT_ videolan _DOT_ org >
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+
+#if defined(__PLUGIN__) || defined(__BUILTIN__) || !defined(__LIBVLC__)
+# error This header file can only be included from LibVLC.
+#endif
+
+#ifndef _INPUT_INTERFACE_H
+#define _INPUT_INTERFACE_H 1
+
+#include <vlc_input.h>
+#include <libvlc.h>
+
+/**********************************************************************
+ * Item metadata
+ **********************************************************************/
+int  input_ArtFind( playlist_t *, input_item_t * );
+int  input_DownloadAndCacheArt( playlist_t *, input_item_t * );
+
+void input_item_SetPreparsed( input_item_t *p_i, bool b_preparsed );
+
+typedef struct playlist_album_t
+{
+    char *psz_artist;
+    char *psz_album;
+    char *psz_arturl;
+    bool b_found;
+} playlist_album_t;
+
+
+void input_item_SetArtNotFound( input_item_t *p_i, bool b_not_found );
+void input_item_SetArtFetched( input_item_t *p_i, bool b_art_fetched );
+
+/* misc/stats.c
+ * FIXME it should NOT be defined here or not coded in misc/stats.c */
+input_stats_t *stats_NewInputStats( input_thread_t *p_input );
+
+/* input.c */
+#define input_CreateThreadExtended(a,b,c,d) __input_CreateThreadExtended(VLC_OBJECT(a),b,c,d)
+input_thread_t *__input_CreateThreadExtended ( vlc_object_t *, input_item_t *, const char *, sout_instance_t * );
+
+sout_instance_t * input_DetachSout( input_thread_t *p_input );
+
+#endif

src/input/input_internal.h

@@ -32,6 +32,7 @@
 #include <vlc_demux.h>
 #include <vlc_input.h>
 #include <libvlc.h>
+#include "input_interface.h"
 
 /*****************************************************************************
  *  Private input fields
@@ -233,93 +234,19 @@ static inline void input_ControlPush( input_thread_t *p_input,
     vlc_mutex_unlock( &p_input->p->lock_control );
 }
 
-/** Stuff moved out of vlc_input.h -- FIXME: should probably not be inline
- * anyway. */
-
-static inline void input_item_SetPreparsed( input_item_t *p_i, bool preparsed )
-{
-    bool send_event = false;
-
-    if( !p_i->p_meta )
-        p_i->p_meta = vlc_meta_New();
-
-    vlc_mutex_lock( &p_i->lock );
-    int new_status;
-    if( preparsed )
-        new_status = p_i->p_meta->i_status | ITEM_PREPARSED;
-    else
-        new_status = p_i->p_meta->i_status & ~ITEM_PREPARSED;
-    if( p_i->p_meta->i_status != new_status )
-    {
-        p_i->p_meta->i_status = new_status;
-        send_event = true;
-    }
-
-    vlc_mutex_unlock( &p_i->lock );
-
-    if( send_event )
-    {
-        vlc_event_t event;
-        event.type = vlc_InputItemPreparsedChanged;
-        event.u.input_item_preparsed_changed.new_status = new_status;
-        vlc_event_send( &p_i->event_manager, &event );
-    }
-}
-
-static inline void input_item_SetArtNotFound( input_item_t *p_i, bool notfound )
-{
-    if( !p_i->p_meta )
-        p_i->p_meta = vlc_meta_New();
-
-    if( notfound )
-        p_i->p_meta->i_status |= ITEM_ART_NOTFOUND;
-    else
-        p_i->p_meta->i_status &= ~ITEM_ART_NOTFOUND;
-}
-
-static inline void input_item_SetArtFetched( input_item_t *p_i, bool artfetched )
-{
-    if( !p_i->p_meta )
-        p_i->p_meta = vlc_meta_New();
-
-    if( artfetched )
-        p_i->p_meta->i_status |= ITEM_ART_FETCHED;
-    else
-        p_i->p_meta->i_status &= ~ITEM_ART_FETCHED;
-}
-
-void input_item_SetHasErrorWhenReading( input_item_t *p_i, bool error );
-
 /**********************************************************************
  * Item metadata
  **********************************************************************/
-typedef struct playlist_album_t
-{
-    char *psz_artist;
-    char *psz_album;
-    char *psz_arturl;
-    bool b_found;
-} playlist_album_t;
-
-int         input_ArtFind       ( playlist_t *, input_item_t * );
-int         input_DownloadAndCacheArt ( playlist_t *, input_item_t * );
-
-/* Becarefull; p_item lock HAS to be taken */
+/* input_ExtractAttachmentAndCacheArt:
+ *  Becarefull; p_item lock HAS to be taken */
 void input_ExtractAttachmentAndCacheArt( input_thread_t *p_input );
 
+void input_item_SetErrorWhenReading( input_item_t *p_i, bool b_error );
+
 /***************************************************************************
  * Internal prototypes
  ***************************************************************************/
 
-/* misc/stats.c */
-input_stats_t *stats_NewInputStats( input_thread_t *p_input );
-
-/* input.c */
-#define input_CreateThreadExtended(a,b,c,d) __input_CreateThreadExtended(VLC_OBJECT(a),b,c,d)
-input_thread_t *__input_CreateThreadExtended ( vlc_object_t *, input_item_t *, const char *, sout_instance_t * );
-
-sout_instance_t * input_DetachSout( input_thread_t *p_input );
-
 /* var.c */
 void input_ControlVarInit ( input_thread_t * );
 void input_ControlVarStop( input_thread_t * );

src/input/item.c

@@ -39,6 +39,7 @@ static void GuessType( input_item_t *p_item );
 static inline void input_item_Init( vlc_object_t *p_o, input_item_t *p_i )
 {
     memset( p_i, 0, sizeof(input_item_t) );
+
     p_i->psz_name = NULL;
     p_i->psz_uri = NULL;
     TAB_INIT( p_i->i_es, p_i->es );
@@ -115,20 +116,83 @@ static inline void input_item_Clean( input_item_t *p_i )
 
     vlc_mutex_destroy( &p_i->lock );
 }
+void input_item_SetErrorWhenReading( input_item_t *p_i, bool b_error )
+{
+    bool b_changed;
+
+    vlc_mutex_lock( &p_i->lock );
+
+    b_changed = p_i->b_error_when_reading != b_error;
+    p_i->b_error_when_reading = b_error;
+
+    vlc_mutex_unlock( &p_i->lock );
 
-void input_item_SetHasErrorWhenReading( input_item_t *p_i, bool error )
+    if( b_changed )
+    {
+        vlc_event_t event;
+
+        event.type = vlc_InputItemErrorWhenReadingChanged;
+        event.u.input_item_error_when_reading_changed.new_value = b_error;
+        vlc_event_send( &p_i->event_manager, &event );
+    }
+}
+void input_item_SetPreparsed( input_item_t *p_i, bool b_preparsed )
 {
-    vlc_event_t event;
+    bool b_send_event = false;
 
-    if( p_i->b_error_when_reading == error )
-        return;
+    vlc_mutex_lock( &p_i->lock );
 
-    p_i->b_error_when_reading = error;
+    if( !p_i->p_meta )
+        p_i->p_meta = vlc_meta_New();
 
-    /* Notify interested third parties */
-    event.type = vlc_InputItemErrorWhenReadingChanged;
-    event.u.input_item_error_when_reading_changed.new_value = error;
-    vlc_event_send( &p_i->event_manager, &event );
+    int i_new_status;
+    if( b_preparsed )
+        i_new_status = p_i->p_meta->i_status | ITEM_PREPARSED;
+    else
+        i_new_status = p_i->p_meta->i_status & ~ITEM_PREPARSED;
+    if( p_i->p_meta->i_status != i_new_status )
+    {
+        p_i->p_meta->i_status = i_new_status;
+        b_send_event = true;
+    }
+
+    vlc_mutex_unlock( &p_i->lock );
+
+    if( b_send_event )
+    {
+        vlc_event_t event;
+        event.type = vlc_InputItemPreparsedChanged;
+        event.u.input_item_preparsed_changed.new_status = i_new_status;
+        vlc_event_send( &p_i->event_manager, &event );
+    }
+}
+void input_item_SetArtNotFound( input_item_t *p_i, bool b_not_found )
+{
+    vlc_mutex_lock( &p_i->lock );
+
+    if( !p_i->p_meta )
+        p_i->p_meta = vlc_meta_New();
+
+    if( b_not_found )
+        p_i->p_meta->i_status |= ITEM_ART_NOTFOUND;
+    else
+        p_i->p_meta->i_status &= ~ITEM_ART_NOTFOUND;
+
+    vlc_mutex_unlock( &p_i->lock );
+}
+void input_item_SetArtFetched( input_item_t *p_i, bool b_art_fetched )
+{
+    vlc_mutex_lock( &p_i->lock );
+
+    if( !p_i->p_meta )
+        p_i->p_meta = vlc_meta_New();
+
+    if( b_art_fetched )
+        p_i->p_meta->i_status |= ITEM_ART_FETCHED;
+    else
+        p_i->p_meta->i_status &= ~ITEM_ART_FETCHED;
+
+    vlc_mutex_unlock( &p_i->lock );
 }
 
 void input_item_SetMeta( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz_val )
@@ -149,6 +213,9 @@ void input_item_SetMeta( input_item_t *p_i, vlc_meta_type_t meta_type, const cha
 
 /**
  * Get the item from an input thread
+ * FIXME it does not increase ref count of the item.
+ * if it is used after p_input is destroyed nothing prevent it from
+ * being freed.
  */
 input_item_t *input_GetItem( input_thread_t *p_input )
 {
@@ -156,81 +223,82 @@ input_item_t *input_GetItem( input_thread_t *p_input )
     return p_input->p->input.p_item;
 }
 
+/* FIXME GRRRRRRRRRR args should be in the reverse order to be 
+ * consistant with (nearly?) all or copy funcs */
 void input_item_CopyOptions( input_item_t *p_parent,
-                                          input_item_t *p_child )
+                             input_item_t *p_child )
 {
-    int i;
-    for( i = 0 ; i< p_parent->i_options; i++ )
+    vlc_mutex_lock( &p_parent->lock );
+
+    for( int i = 0 ; i< p_parent->i_options; i++ )
     {
-        char *psz_option= strdup( p_parent->ppsz_options[i] );
-        if( !strcmp( psz_option, "meta-file" ) )
-        {
-            free( psz_option );
+        if( !strcmp( p_parent->ppsz_options[i], "meta-file" ) )
             continue;
-        }
-        p_child->i_options++;
-        p_child->ppsz_options = (char **)realloc( p_child->ppsz_options,
-                                                  p_child->i_options *
-                                                  sizeof( char * ) );
-        p_child->ppsz_options[p_child->i_options-1] = psz_option;
-        p_child->optflagc++;
-        p_child->optflagv = (uint8_t *)realloc( p_child->optflagv,
-                                                p_child->optflagc );
-        p_child->optflagv[p_child->optflagc - 1] = p_parent->optflagv[i];
+
+        input_item_AddOpt( p_child,
+                           p_parent->ppsz_options[i],
+                           p_parent->optflagv[i] );
     }
-}
 
-void input_item_SetName( input_item_t *p_item, const char *psz_name )
-{
-    free( p_item->psz_name );
-    p_item->psz_name = strdup( psz_name );
+    vlc_mutex_unlock( &p_parent->lock );
 }
 
 /* This won't hold the item, but can tell to interested third parties
  * Like the playlist, that there is a new sub item. With this design
  * It is not the input item's responsability to keep all the ref of
  * the input item children. */
-void input_item_AddSubItem( input_item_t *p_parent,
-                                         input_item_t *p_child )
+void input_item_AddSubItem( input_item_t *p_parent, input_item_t *p_child )
 {
-    vlc_event_t event;
+    vlc_mutex_lock( &p_parent->lock );
 
     p_parent->i_type = ITEM_TYPE_PLAYLIST;
 
+    vlc_mutex_unlock( &p_parent->lock );
+
     /* Notify interested third parties */
+    vlc_event_t event;
+
     event.type = vlc_InputItemSubItemAdded;
     event.u.input_item_subitem_added.p_new_child = p_child;
     vlc_event_send( &p_parent->event_manager, &event );
 }
 
-int input_item_AddOption (input_item_t *item, const char *str)
+int input_item_AddOption( input_item_t *p_item, const char *psz_option )
 {
-    return input_item_AddOpt (item, str, VLC_INPUT_OPTION_TRUSTED);
+    return input_item_AddOpt( p_item, psz_option, VLC_INPUT_OPTION_TRUSTED );
 }
 
-bool input_item_HasErrorWhenReading (input_item_t *item)
+bool input_item_HasErrorWhenReading( input_item_t *p_item )
 {
-    return item->b_error_when_reading;
+    vlc_mutex_lock( &p_item->lock );
+
+    bool b_error = p_item->b_error_when_reading;
+
+    vlc_mutex_unlock( &p_item->lock );
+
+    return b_error;
 }
 
-bool input_item_MetaMatch( input_item_t *p_i, vlc_meta_type_t meta_type, const char *psz )
+bool input_item_MetaMatch( input_item_t *p_i,
+                           vlc_meta_type_t meta_type, const char *psz )
 {
     vlc_mutex_lock( &p_i->lock );
+
     if( !p_i->p_meta )
     {
         vlc_mutex_unlock( &p_i->lock );
         return false;
     }
-    const char * meta = vlc_meta_Get( p_i->p_meta, meta_type );
-    bool ret = meta && strcasestr( meta, psz );
+    const char *psz_meta = vlc_meta_Get( p_i->p_meta, meta_type );
+    bool b_ret = psz_meta && strcasestr( psz_meta, psz );
+
     vlc_mutex_unlock( &p_i->lock );
 
-    return ret;
+    return b_ret;
 }
 
-char * input_item_GetMeta( input_item_t *p_i, vlc_meta_type_t meta_type )
+char *input_item_GetMeta( input_item_t *p_i, vlc_meta_type_t meta_type )
 {
-    char * psz = NULL;
     vlc_mutex_lock( &p_i->lock );
 
     if( !p_i->p_meta )
@@ -239,6 +307,7 @@ char * input_item_GetMeta( input_item_t *p_i, vlc_meta_type_t meta_type )
         return NULL;
     }
 
+    char *psz = NULL;
     if( vlc_meta_Get( p_i->p_meta, meta_type ) )
         psz = strdup( vlc_meta_Get( p_i->p_meta, meta_type ) );
 
@@ -246,88 +315,93 @@ char * input_item_GetMeta( input_item_t *p_i, vlc_meta_type_t meta_type )
     return psz;
 }
 
-char * input_item_GetName( input_item_t * p_i )
+char *input_item_GetName( input_item_t *p_item )
 {
-    vlc_mutex_lock( &p_i->lock );
-    char *psz_s = p_i->psz_name ? strdup( p_i->psz_name ) : NULL;
-    vlc_mutex_unlock( &p_i->lock );
-    return psz_s;
+    vlc_mutex_lock( &p_item->lock );
+
+    char *psz_name = p_item->psz_name ? strdup( p_item->psz_name ) : NULL;
+
+    vlc_mutex_unlock( &p_item->lock );
+    return psz_name;
 }
+void input_item_SetName( input_item_t *p_item, const char *psz_name )
+{
+    vlc_mutex_lock( &p_item->lock );
 
-char * input_item_GetURI( input_item_t * p_i )
+    free( p_item->psz_name );
+    p_item->psz_name = strdup( psz_name );
+
+    vlc_mutex_unlock( &p_item->lock );
+}
+
+char *input_item_GetURI( input_item_t *p_i )
 {
     vlc_mutex_lock( &p_i->lock );
+
     char *psz_s = p_i->psz_uri ? strdup( p_i->psz_uri ) : NULL;
+
     vlc_mutex_unlock( &p_i->lock );
     return psz_s;
 }
-
-void input_item_SetURI( input_item_t * p_i, char * psz_uri )
+void input_item_SetURI( input_item_t *p_i, char *psz_uri )
 {
     vlc_mutex_lock( &p_i->lock );
+
     free( p_i->psz_uri );
     p_i->psz_uri = strdup( psz_uri );
+
     vlc_mutex_unlock( &p_i->lock );
 }
 
-mtime_t input_item_GetDuration( input_item_t * p_i )
+mtime_t input_item_GetDuration( input_item_t *p_i )
 {
     vlc_mutex_lock( &p_i->lock );
+
     mtime_t i_duration = p_i->i_duration;
+
     vlc_mutex_unlock( &p_i->lock );
     return i_duration;
 }
 
-void input_item_SetDuration( input_item_t * p_i, mtime_t i_duration )
+void input_item_SetDuration( input_item_t *p_i, mtime_t i_duration )
 {
-    bool send_event = false;
+    bool b_send_event = false;
 
     vlc_mutex_lock( &p_i->lock );
     if( p_i->i_duration != i_duration )
     {
         p_i->i_duration = i_duration;
-        send_event = true;
+        b_send_event = true;
     }
     vlc_mutex_unlock( &p_i->lock );
 
-    if ( send_event == true )
+    if( b_send_event )
     {
         vlc_event_t event;
+
         event.type = vlc_InputItemDurationChanged;
         event.u.input_item_duration_changed.new_duration = i_duration;
         vlc_event_send( &p_i->event_manager, &event );
     }
-
-    return;
 }
 
 
-bool input_item_IsPreparsed( input_item_t *p_i )
+bool input_item_IsPreparsed( input_item_t *p_item )
 {
-    return p_i->p_meta ? p_i->p_meta->i_status & ITEM_PREPARSED : false ;
-}
-
-bool input_item_IsArtFetched( input_item_t *p_i )
-{
-    return p_i->p_meta ? p_i->p_meta->i_status & ITEM_ART_FETCHED : false ;
-}
-
-/* FIXME dangerous, unlocked */
-const vlc_meta_t * input_item_GetMetaObject( input_item_t *p_i )
-{
-    if( !p_i->p_meta )
-        p_i->p_meta = vlc_meta_New();
+    vlc_mutex_lock( &p_item->lock );
+    bool b_preparsed = p_item->p_meta ? ( p_item->p_meta->i_status & ITEM_PREPARSED ) != 0 : false;
+    vlc_mutex_unlock( &p_item->lock );
 
-    return p_i->p_meta;
+    return b_preparsed;
 }
 
-/* FIXME dangerous, unlocked */