OSD: fix some thread-unsafe constructs, and avoid vlc_object_find

src/osd/osd.c

@@ -138,20 +138,20 @@ static osd_state_t *osd_StateChange( osd_button_t *p_button, const int i_state )
 osd_menu_t *__osd_MenuCreate( vlc_object_t *p_this, const char *psz_file )
 {
     osd_menu_t  *p_osd = NULL;
-    vlc_value_t lockval;
+    vlc_value_t lockval, val;
     int         i_volume = 0;
     int         i_steps = 0;
 
     /* to be sure to avoid multiple creation */
     var_Create( p_this->p_libvlc, "osd_mutex", VLC_VAR_MUTEX );
+    var_Create( p_this->p_libvlc, "osd", VLC_VAR_ADDRESS );
     var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
     vlc_mutex_lock( lockval.p_address );
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
+    var_Get( p_this->p_libvlc, "osd", &val );
+    if( val.p_address == NULL )
     {
         static const char osdmenu_name[] = "osd menu";
-        vlc_value_t val;
 
         p_osd = vlc_custom_create( p_this, sizeof( *p_osd ), VLC_OBJECT_OSDMENU,
                                     osdmenu_name );
@@ -190,10 +190,15 @@ osd_menu_t *__osd_MenuCreate( vlc_object_t *p_this, const char *psz_file )
         var_Create( p_osd, "osd-menu-update", VLC_VAR_BOOL );
         var_Create( p_osd, "osd-menu-visible", VLC_VAR_BOOL );
 
-        val.b_bool = false;
-        var_Set( p_osd, "osd-menu-update", val );
-        var_Set( p_osd, "osd-menu-visible", val );
+        var_SetBool( p_osd, "osd-menu-update", false );
+        var_SetBool( p_osd, "osd-menu-visible", false );
+
+        val.p_address = p_osd;
+        var_Set( p_this->p_libvlc, "osd", val );
     }
+    else
+        p_osd = val.p_address;
+    vlc_object_hold( p_osd );
     vlc_mutex_unlock( lockval.p_address );
     return p_osd;
 
@@ -214,21 +219,28 @@ void __osd_MenuDelete( vlc_object_t *p_this, osd_menu_t *p_osd )
 
     if( vlc_internals( VLC_OBJECT(p_osd) )->i_refcount == 1 )
     {
+        vlc_value_t val;
+
         var_Destroy( p_osd, "osd-menu-visible" );
         var_Destroy( p_osd, "osd-menu-update" );
         osd_ParserUnload( p_osd );
+        val.p_address = NULL;
+        var_Set( p_this->p_libvlc, "osd", val );
     }
 
     vlc_object_release( p_osd );
-    if( vlc_internals( VLC_OBJECT(p_osd) )->i_refcount > 0 )
-    {
-        vlc_mutex_unlock( lockval.p_address );
-        return;
-    }
-    p_osd = NULL;
     vlc_mutex_unlock( lockval.p_address );
 }
 
+static osd_menu_t *osd_Find( vlc_object_t *p_this )
+{
+    vlc_value_t val;
+
+    if( var_Get( p_this->p_libvlc, "osd", &val ) )
+        return NULL;
+    return val.p_address;
+}
+
 /* The volume can be modified in another interface while the OSD Menu
  * has not been instantiated yet. This routines updates the "volume OSD menu item"
  * to reflect the current state of the GUI.
@@ -262,20 +274,20 @@ static void osd_UpdateState( osd_menu_state_t *p_state, int i_x, int i_y,
 
 void __osd_MenuShow( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     osd_button_t *p_button = NULL;
     vlc_value_t lockval;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
+    p_osd = osd_Find( p_this );
     if( p_osd == NULL )
     {
-        msg_Err( p_this, "osd_MenuNext failed" );
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_MenuShow failed" );
         return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
 #if defined(OSD_MENU_DEBUG)
     msg_Dbg( p_osd, "menu on" );
 #endif
@@ -299,25 +311,25 @@ void __osd_MenuShow( vlc_object_t *p_this )
     }
     osd_SetMenuVisible( p_osd, true );
 
-    vlc_object_release( (vlc_object_t*) p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }
 
 void __osd_MenuHide( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     vlc_value_t lockval;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
+
+    p_osd = osd_Find( p_this );
     if( p_osd == NULL )
     {
-        msg_Err( p_this, "osd_MenuNext failed" );
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_MenuHide failed" );
         return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
 #if defined(OSD_MENU_DEBUG)
     msg_Dbg( p_osd, "menu off" );
 #endif
@@ -326,32 +338,26 @@ void __osd_MenuHide( vlc_object_t *p_this )
                 0, 0, NULL );
     osd_SetMenuUpdate( p_osd, true );
 
-    vlc_object_release( (vlc_object_t*) p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }
 
 void __osd_MenuActivate( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     osd_button_t *p_button = NULL;
     vlc_value_t lockval;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
-    {
-        msg_Err( p_this, "osd_MenuNext failed" );
-        return;
-    }
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
 
-    if( osd_isVisible( p_osd ) == false )
+    p_osd = osd_Find( p_this );
+    if( p_osd == NULL || !osd_isVisible( p_osd ) )
     {
-        vlc_object_release( (vlc_object_t*) p_osd );
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_MenuActivate failed" );
         return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
 #if defined(OSD_MENU_DEBUG)
     msg_Dbg( p_osd, "select" );
 #endif
@@ -361,14 +367,12 @@ void __osd_MenuActivate( vlc_object_t *p_this )
      */
     if( p_button && p_button->p_up )
     {
-        vlc_object_release( (vlc_object_t*) p_osd );
         vlc_mutex_unlock( lockval.p_address );
         __osd_MenuUp( p_this );   /* "menu select" means go to menu item above. */
         return;
     }
     if( p_button && p_button->p_down )
     {
-        vlc_object_release( (vlc_object_t*) p_osd );
         vlc_mutex_unlock( lockval.p_address );
         __osd_MenuDown( p_this ); /* "menu select" means go to menu item below. */
         return;
@@ -389,32 +393,26 @@ void __osd_MenuActivate( vlc_object_t *p_this )
         msg_Dbg( p_osd, "select (%d, %s)", config_GetInt( p_osd, p_button->psz_action ), p_button->psz_action );
 #endif
     }
-    vlc_object_release( (vlc_object_t*) p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }
 
 void __osd_MenuNext( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     osd_button_t *p_button = NULL;
     vlc_value_t lockval;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
-    {
-        msg_Err( p_this, "osd_MenuNext failed" );
-        return;
-    }
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
 
-    if( osd_isVisible( p_osd ) == false )
+    p_osd = osd_Find( p_this );
+    if( p_osd == NULL || !osd_isVisible( p_osd ) )
     {
-        vlc_object_release( (vlc_object_t*) p_osd );
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_MenuNext failed" );
         return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
     p_button = p_osd->p_state->p_visible;
     if( p_button )
     {
@@ -440,32 +438,25 @@ void __osd_MenuNext( vlc_object_t *p_this )
     msg_Dbg( p_osd, "direction right [button %s]", p_osd->p_state->p_visible->psz_action );
 #endif
 
-    vlc_object_release( (vlc_object_t*) p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }
 
 void __osd_MenuPrev( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     osd_button_t *p_button = NULL;
     vlc_value_t lockval;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
+    p_osd = osd_Find( p_this );
+    if( p_osd == NULL || !osd_isVisible( p_osd ) )
     {
+        vlc_mutex_unlock( lockval.p_address );
         msg_Err( p_this, "osd_MenuPrev failed" );
         return;
     }
 
-    if( osd_isVisible( p_osd ) == false )
-    {
-        vlc_object_release( (vlc_object_t*) p_osd );
-        return;
-    }
-
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
     p_button = p_osd->p_state->p_visible;
     if( p_button )
     {
@@ -491,34 +482,28 @@ void __osd_MenuPrev( vlc_object_t *p_this )
     msg_Dbg( p_osd, "direction left [button %s]", p_osd->p_state->p_visible->psz_action );
 #endif
 
-    vlc_object_release( (vlc_object_t*) p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }
 
 void __osd_MenuUp( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     osd_button_t *p_button = NULL;
     vlc_value_t lockval;
 #if defined(OSD_MENU_DEBUG)
     vlc_value_t val;
 #endif
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
-    {
-        msg_Err( p_this, "osd_MenuDown failed" );
-        return;
-    }
 
-    if( osd_isVisible( p_osd ) == false )
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
+    p_osd = osd_Find( p_this );
+    if( p_osd == NULL || !osd_isVisible( p_osd ) )
     {
-        vlc_object_release( (vlc_object_t*) p_osd );
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_MenuActivate failed" );
         return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
     p_button = p_osd->p_state->p_visible;
     if( p_button )
     {
@@ -562,35 +547,29 @@ void __osd_MenuUp( vlc_object_t *p_this )
     msg_Dbg( p_osd, "direction up [button %s]", p_osd->p_state->p_visible->psz_action );
 #endif
 
-    vlc_object_release( (vlc_object_t*) p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }
 
 void __osd_MenuDown( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     osd_button_t *p_button = NULL;
     vlc_value_t lockval;
 #if defined(OSD_MENU_DEBUG)
     vlc_value_t val;
 #endif
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
-    {
-        msg_Err( p_this, "osd_MenuDown failed" );
-        return;
-    }
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
 
-    if( osd_isVisible( p_osd ) == false )
+    p_osd = osd_Find( p_this );
+    if( p_osd == NULL || !osd_isVisible( p_osd ) )
     {
-        vlc_object_release( (vlc_object_t*) p_osd );
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_MenuActivate failed" );
         return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
     p_button = p_osd->p_state->p_visible;
     if( p_button )
     {
@@ -634,7 +613,6 @@ void __osd_MenuDown( vlc_object_t *p_this )
     msg_Dbg( p_osd, "direction down [button %s]", p_osd->p_state->p_visible->psz_action );
 #endif
 
-    vlc_object_release( (vlc_object_t*) p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }
 
@@ -655,23 +633,25 @@ static int osd_VolumeStep( vlc_object_t *p_this, int i_volume, int i_steps )
  */
 void __osd_Volume( vlc_object_t *p_this )
 {
-    osd_menu_t *p_osd = NULL;
+    osd_menu_t *p_osd;
     osd_button_t *p_button = NULL;
     vlc_value_t lockval;
     int i_volume = 0;
     int i_steps = 0;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
+
+    p_osd = osd_Find( p_this );
     if( p_osd == NULL )
     {
+        vlc_mutex_unlock( lockval.p_address );
         msg_Err( p_this, "OSD menu volume update failed" );
         return;
     }
 
     if( p_osd->p_state && p_osd->p_state->p_volume )
     {
-        var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-        vlc_mutex_lock( lockval.p_address );
 
         p_button = p_osd->p_state->p_volume;
         if( p_osd->p_state->p_volume )
@@ -691,9 +671,8 @@ void __osd_Volume( vlc_object_t *p_this )
             osd_SetMenuUpdate( p_osd, true );
             osd_SetMenuVisible( p_osd, true );
         }
-        vlc_mutex_unlock( lockval.p_address );
     }
-    vlc_object_release( p_osd );
+    vlc_mutex_unlock( lockval.p_address );
 }
 
 osd_button_t *__osd_ButtonFind( vlc_object_t *p_this, int i_x, int i_y,
@@ -704,22 +683,17 @@ osd_button_t *__osd_ButtonFind( vlc_object_t *p_this, int i_x, int i_y,
     osd_button_t *p_button;
     vlc_value_t lockval;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
-    {
-        msg_Err( p_this, "OSD menu button find failed" );
-        return NULL;
-    }
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
 
-    if( osd_isVisible( p_osd ) == false )
+    p_osd = osd_Find( p_this );
+    if( p_osd == NULL || !osd_isVisible( p_osd ) )
     {
-        vlc_object_release( p_osd );
-        return NULL;
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_ButtonFind failed" );
+        return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
     p_button = p_osd->p_button;
     for( ; p_button != NULL; p_button = p_button->p_next )
     {
@@ -766,13 +740,11 @@ osd_button_t *__osd_ButtonFind( vlc_object_t *p_this, int i_x, int i_y,
         if( ( i_x >= i_x_offset ) && ( i_x <= i_x_offset + i_width ) &&
             ( i_y >= i_y_offset ) && ( i_y <= i_y_offset + i_height ) )
         {
-            vlc_object_release( p_osd );
             vlc_mutex_unlock( lockval.p_address );
             return p_button;
         }
     }
 
-    vlc_object_release( p_osd );
     vlc_mutex_unlock( lockval.p_address );
     return NULL;
 }
@@ -786,22 +758,17 @@ void __osd_ButtonSelect( vlc_object_t *p_this, osd_button_t *p_button )
     osd_button_t *p_old;
     vlc_value_t lockval;
 
-    p_osd = vlc_object_find( p_this, VLC_OBJECT_OSDMENU, FIND_ANYWHERE );
-    if( p_osd == NULL )
-    {
-        msg_Err( p_this, "OSD menu button select failed" );
-        return;
-    }
+    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
+    vlc_mutex_lock( lockval.p_address );
 
-    if( osd_isVisible( p_osd ) == false )
+    p_osd = osd_Find( p_this );
+    if( p_osd == NULL || !osd_isVisible( p_osd ) )
     {
-        vlc_object_release( (vlc_object_t*) p_osd );
+        vlc_mutex_unlock( lockval.p_address );
+        msg_Err( p_this, "osd_ButtonSelect failed" );
         return;
     }
 
-    var_Get( p_this->p_libvlc, "osd_mutex", &lockval );
-    vlc_mutex_lock( lockval.p_address );
-
     p_old = p_osd->p_state->p_visible;
     if( p_old )
     {
@@ -824,6 +791,5 @@ void __osd_ButtonSelect( vlc_object_t *p_this, osd_button_t *p_button )
     msg_Dbg( p_osd, "button selected is [button %s]", p_osd->p_state->p_visible->psz_action );
 #endif
 
-    vlc_object_release( p_osd );
     vlc_mutex_unlock( lockval.p_address );
 }