Kill stupid VOUT_SNAPSHOT control.
authorRémi Denis-Courmont <rdenis@simphalempin.com>
Sat, 14 Feb 2009 17:58:03 +0000 (19:58 +0200)
committerRémi Denis-Courmont <rdenis@simphalempin.com>
Sat, 14 Feb 2009 18:02:31 +0000 (20:02 +0200)
Calling vout_Control() after vlc_object_find() or input_GetVouts is not
valid. It cannot be. There is no warranty that pf_control is valid (the
video output plugin could be initializing or deinitializing). Even if
there were a lock without the pf_control implementation (as with X11),
it still wouldn't work.

Anyway, in this particular case, we already have video-snapshot to do
the exact same thing in a safe manner.

There remain some issues with snapshots:
 - there is no synchronization of b_snapshoy with the video thread,
 - there is no/incomplete error handling,
 - there is no protection against multiple concurrent snapshot reqs.

12 files changed:
include/vlc_vout.h
modules/control/hotkeys.c
modules/control/http/rpn.c
modules/gui/macosx/vout.m
modules/gui/maemo/maemo_callbacks.c
modules/gui/qt4/actions_manager.cpp
modules/gui/skins2/commands/cmd_snapshot.cpp
modules/video_output/opengl.c
modules/video_output/opengllayer.m
src/control/mediacontrol_audio_video.c
src/control/video.c
src/video_output/vout_intf.c

index 9744319..2effe42 100644 (file)
@@ -705,7 +705,6 @@ enum output_query_e
     VOUT_SET_SIZE,         /* arg1= unsigned int, arg2= unsigned int, res= */
     VOUT_SET_STAY_ON_TOP,  /* arg1= bool       res=    */
     VOUT_REPARENT,
-    VOUT_SNAPSHOT,
     VOUT_CLOSE,
     VOUT_SET_FOCUS,         /* arg1= bool       res=    */
     VOUT_SET_VIEWPORT,      /* arg1= view rect, arg2=clip rect, res= */
index b4e5271..74cf5a1 100644 (file)
@@ -244,7 +244,7 @@ static void Run( intf_thread_t *p_intf )
         /* Video Output actions */
         else if( i_action == ACTIONID_SNAPSHOT )
         {
-            if( p_vout ) vout_Control( p_vout, VOUT_SNAPSHOT );
+            if( p_vout ) var_TriggerCallback( p_vout, "video-snapshot" );
         }
         else if( i_action == ACTIONID_TOGGLE_FULLSCREEN )
         {
index e4689ff..73aee51 100644 (file)
@@ -1125,7 +1125,7 @@ void EvaluateRPN( intf_thread_t *p_intf, mvar_t  *vars,
                 vout_thread_t *p_vout = input_GetVout( p_sys->p_input );
                 if( p_vout )
                 {
-                    vout_Control( p_vout, VOUT_SNAPSHOT );
+                    vout_TriggerCallback( p_vout, "video-snapshot" );
                     vlc_object_release( p_vout );
                     msg_Dbg( p_intf, "requested snapshot" );
                 }
index 645bc3d..4654777 100644 (file)
@@ -446,7 +446,7 @@ int DeviceCallback( vlc_object_t *p_this, const char *psz_variable,
 
 - (void)snapshot
 {
-    vout_Control( p_real_vout, VOUT_SNAPSHOT );
+    var_TriggerCallback( p_real_vout, "video-snapshot" );
 }
 
 - (void)manage
index 97ee4df..c14435c 100644 (file)
@@ -232,7 +232,7 @@ void snapshot_cb( GtkMenuItem *menuitem, gpointer user_data )
         return;
     }
 
-    vout_Control( p_intf->p_sys->p_vout, VOUT_SNAPSHOT );
+    var_TriggerCallback( p_intf->p_sys->p_vout, "video-snapshot" );
     hildon_banner_show_information( GTK_WIDGET( p_intf->p_sys->p_main_window ),
                                     NULL,
                                     "Snapshot taken" );
index ee5bcb6..edddb1a 100644 (file)
@@ -127,7 +127,7 @@ void ActionsManager::snapshot()
     vout_thread_t *p_vout = THEMIM->getVout();
     if( p_vout )
     {
-        vout_Control( p_vout, VOUT_SNAPSHOT );
+        var_TriggerCallback( p_vout, "video-snapshot" );
         vlc_object_release( p_vout );
     }
 }
index 80d12ca..bc2b020 100644 (file)
@@ -34,7 +34,7 @@ void CmdSnapshot::execute()
     if( pVout )
     {
         // Take a snapshot
-        vout_Control( pVout, VOUT_SNAPSHOT );
+        var_TriggerCallback( pVout, "video-snapshot" );
         vlc_object_release( pVout );
     }
 }
index c8a1cc0..2147d0c 100644 (file)
@@ -628,9 +628,6 @@ static int Control( vout_thread_t *p_vout, int i_query, va_list args )
 
     switch( i_query )
     {
-    case VOUT_SNAPSHOT:
-        return vout_vaControlDefault( p_vout, i_query, args );
-
     default:
         if( p_sys->p_vout->pf_control )
             return p_sys->p_vout->pf_control( p_sys->p_vout, i_query, args );
index 8536cd0..4884eef 100644 (file)
@@ -336,9 +336,6 @@ static int Control( vout_thread_t *p_vout, int i_query, va_list args )
 
     switch( i_query )
     {
-    case VOUT_SNAPSHOT:
-        return vout_vaControlDefault( p_vout, i_query, args );
-
     default:
         if( p_sys->p_vout->pf_control )
             return p_sys->p_vout->pf_control( p_sys->p_vout, i_query, args );
index 3e02263..23027d3 100644 (file)
@@ -102,7 +102,7 @@ mediacontrol_snapshot( mediacontrol_Instance *self,
        NULL (in case of error) or a pointer to valid data. */
     p_snapshot->p_data = ( char* )p_snapshot;
 
-    vout_Control( p_vout, VOUT_SNAPSHOT );
+    var_TriggerCallback( p_vout, "video-snapshot" );
     while ( p_snapshot->p_data == ( char* )p_snapshot )
     {
         vlc_cond_wait( &p_snapshot->p_condvar, &p_snapshot->p_mutex );
index 44eb252..ef081c6 100644 (file)
@@ -132,7 +132,7 @@ libvlc_video_take_snapshot( libvlc_media_player_t *p_mi, char *psz_filepath,
     var_SetString( p_vout, "snapshot-path", psz_filepath );
     var_SetString( p_vout, "snapshot-format", "png" );
 
-    vout_Control( p_vout, VOUT_SNAPSHOT );
+    var_TriggerCallback( p_vout, "video-snapshot" );
     vlc_object_release( p_vout );
 }
 
index 9794eee..6cb8459 100644 (file)
@@ -906,10 +906,6 @@ int vout_vaControlDefault( vout_thread_t *p_vout, int i_query, va_list args )
     (void)args;
     switch( i_query )
     {
-    case VOUT_SNAPSHOT:
-        p_vout->p->b_snapshot = true;
-        return VLC_SUCCESS;
-
     default:
         msg_Dbg( p_vout, "control query not supported" );
     }
@@ -1270,10 +1266,12 @@ static int FullscreenCallback( vlc_object_t *p_this, char const *psz_cmd,
 static int SnapshotCallback( vlc_object_t *p_this, char const *psz_cmd,
                        vlc_value_t oldval, vlc_value_t newval, void *p_data )
 {
+    vout_thread_t *p_vout = (vout_thread_t *)p_this;
+
+    /* FIXME: this is definitely not thread-safe */
+    p_vout->p->b_snapshot = true;
     VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);
     VLC_UNUSED(newval); VLC_UNUSED(p_data);
-    vout_thread_t *p_vout = (vout_thread_t *)p_this;
-    vout_Control( p_vout, VOUT_SNAPSHOT );
     return VLC_SUCCESS;
 }