From 53195e9fc7c31e8ae3aeead4879477aa950513b4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?R=C3=A9mi=20Denis-Courmont?= Date: Sat, 14 Feb 2009 19:58:03 +0200 Subject: [PATCH] Kill stupid VOUT_SNAPSHOT control. 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. --- include/vlc_vout.h | 1 - modules/control/hotkeys.c | 2 +- modules/control/http/rpn.c | 2 +- modules/gui/macosx/vout.m | 2 +- modules/gui/maemo/maemo_callbacks.c | 2 +- modules/gui/qt4/actions_manager.cpp | 2 +- modules/gui/skins2/commands/cmd_snapshot.cpp | 2 +- modules/video_output/opengl.c | 3 --- modules/video_output/opengllayer.m | 3 --- src/control/mediacontrol_audio_video.c | 2 +- src/control/video.c | 2 +- src/video_output/vout_intf.c | 10 ++++------ 12 files changed, 12 insertions(+), 21 deletions(-) diff --git a/include/vlc_vout.h b/include/vlc_vout.h index 9744319f59..2effe42939 100644 --- a/include/vlc_vout.h +++ b/include/vlc_vout.h @@ -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= */ diff --git a/modules/control/hotkeys.c b/modules/control/hotkeys.c index b4e52711ad..74cf5a19e4 100644 --- a/modules/control/hotkeys.c +++ b/modules/control/hotkeys.c @@ -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 ) { diff --git a/modules/control/http/rpn.c b/modules/control/http/rpn.c index e4689ffc6c..73aee512df 100644 --- a/modules/control/http/rpn.c +++ b/modules/control/http/rpn.c @@ -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" ); } diff --git a/modules/gui/macosx/vout.m b/modules/gui/macosx/vout.m index 645bc3d84e..4654777cd6 100644 --- a/modules/gui/macosx/vout.m +++ b/modules/gui/macosx/vout.m @@ -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 diff --git a/modules/gui/maemo/maemo_callbacks.c b/modules/gui/maemo/maemo_callbacks.c index 97ee4dfa84..c14435c010 100644 --- a/modules/gui/maemo/maemo_callbacks.c +++ b/modules/gui/maemo/maemo_callbacks.c @@ -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" ); diff --git a/modules/gui/qt4/actions_manager.cpp b/modules/gui/qt4/actions_manager.cpp index ee5bcb6adb..edddb1a0a5 100644 --- a/modules/gui/qt4/actions_manager.cpp +++ b/modules/gui/qt4/actions_manager.cpp @@ -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 ); } } diff --git a/modules/gui/skins2/commands/cmd_snapshot.cpp b/modules/gui/skins2/commands/cmd_snapshot.cpp index 80d12ca310..bc2b020868 100644 --- a/modules/gui/skins2/commands/cmd_snapshot.cpp +++ b/modules/gui/skins2/commands/cmd_snapshot.cpp @@ -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 ); } } diff --git a/modules/video_output/opengl.c b/modules/video_output/opengl.c index c8a1cc04e0..2147d0c204 100644 --- a/modules/video_output/opengl.c +++ b/modules/video_output/opengl.c @@ -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 ); diff --git a/modules/video_output/opengllayer.m b/modules/video_output/opengllayer.m index 8536cd0155..4884eef09a 100644 --- a/modules/video_output/opengllayer.m +++ b/modules/video_output/opengllayer.m @@ -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 ); diff --git a/src/control/mediacontrol_audio_video.c b/src/control/mediacontrol_audio_video.c index 3e022637f8..23027d3a2a 100644 --- a/src/control/mediacontrol_audio_video.c +++ b/src/control/mediacontrol_audio_video.c @@ -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 ); diff --git a/src/control/video.c b/src/control/video.c index 44eb252f8b..ef081c6911 100644 --- a/src/control/video.c +++ b/src/control/video.c @@ -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 ); } diff --git a/src/video_output/vout_intf.c b/src/video_output/vout_intf.c index 9794eeeda2..6cb8459bdc 100644 --- a/src/video_output/vout_intf.c +++ b/src/video_output/vout_intf.c @@ -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; } -- 2.39.2