First pass to clean up snapshot code (vout).

It is thread safe (or should be).
No snapshot should be lost.
The image encoding is now done outside vout thread (in the callback).

diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
index d5335aef6248f1e685e97aa0a78f06c93297209c..27aad5424d228e16b5f03321e7bef8ee0d343df0 100644 (file)
--- a/src/video_output/video_output.c
+++ b/src/video_output/video_output.c
@@ -399,6 +399,12 @@ vout_thread_t * __vout_Create( vlc_object_t *p_parent, video_format_t *p_fmt )
     p_vout->p->b_picture_empty = false;
     p_vout->p->i_picture_qtype = QTYPE_NONE;
 
+    p_vout->p->snapshot.b_available = true;
+    p_vout->p->snapshot.i_request = 0;
+    p_vout->p->snapshot.p_picture = NULL;
+    vlc_mutex_init( &p_vout->p->snapshot.lock );
+    vlc_cond_init( &p_vout->p->snapshot.wait );
+
     /* Initialize locks */
     vlc_mutex_init( &p_vout->picture_lock );
     vlc_cond_init( &p_vout->p->picture_wait );
@@ -575,6 +581,12 @@ void vout_Close( vout_thread_t *p_vout )
     p_vout->p->b_done = true;
     vlc_cond_signal( &p_vout->p->change_wait );
     vlc_mutex_unlock( &p_vout->change_lock );
+
+    vlc_mutex_lock( &p_vout->p->snapshot.lock );
+    p_vout->p->snapshot.b_available = false;
+    vlc_cond_broadcast( &p_vout->p->snapshot.wait );
+    vlc_mutex_unlock( &p_vout->p->snapshot.lock );
+
     vlc_join( p_vout->p->thread, NULL );
     module_unneed( p_vout, p_vout->p_module );
     p_vout->p_module = NULL;
@@ -599,6 +611,21 @@ static void vout_Destructor( vlc_object_t * p_this )
     vlc_mutex_destroy( &p_vout->change_lock );
     vlc_mutex_destroy( &p_vout->p->vfilter_lock );
 
+    /* */
+    for( ;; )
+    {
+        picture_t *p_picture = p_vout->p->snapshot.p_picture;
+        if( !p_picture )
+            break;
+
+        p_vout->p->snapshot.p_picture = p_picture->p_next;
+
+        picture_Release( p_picture );
+    }
+    vlc_cond_destroy( &p_vout->p->snapshot.wait );
+    vlc_mutex_destroy( &p_vout->p->snapshot.lock );
+
+    /* */
     free( p_vout->p->psz_filter_chain );
     free( p_vout->p->psz_title );
 
@@ -1166,7 +1193,9 @@ static void* RunThread( void *p_this )
 
         /* FIXME it is ugly that b_snapshot is not locked but I do not
          * know which lock to use (here and in the snapshot callback) */
-        const bool b_snapshot = p_vout->p->b_snapshot && p_picture != NULL;
+        vlc_mutex_lock( &p_vout->p->snapshot.lock );
+        const bool b_snapshot = p_vout->p->snapshot.i_request > 0 && p_picture != NULL;
+        vlc_mutex_unlock( &p_vout->p->snapshot.lock );
 
         /*
          * Check for subpictures to display
@@ -1188,10 +1217,25 @@ static void* RunThread( void *p_this )
          */
         if( p_directbuffer && b_snapshot )
         {
-            /* FIXME lock (see b_snapshot) */
-            p_vout->p->b_snapshot = false;
-
-            vout_Snapshot( p_vout, p_directbuffer );
+            vlc_mutex_lock( &p_vout->p->snapshot.lock );
+            assert( p_vout->p->snapshot.i_request > 0 );
+            while( p_vout->p->snapshot.i_request > 0 )
+            {
+                picture_t *p_pic = picture_New( p_vout->fmt_out.i_chroma,
+                                                p_vout->fmt_out.i_width,
+                                                p_vout->fmt_out.i_height,
+                                                p_vout->fmt_out.i_aspect  );
+                if( !p_pic )
+                    break;
+
+                picture_Copy( p_pic, p_directbuffer );
+
+                p_pic->p_next = p_vout->p->snapshot.p_picture;
+                p_vout->p->snapshot.p_picture = p_pic;
+                p_vout->p->snapshot.i_request--;
+            }
+            vlc_cond_broadcast( &p_vout->p->snapshot.wait );
+            vlc_mutex_unlock( &p_vout->p->snapshot.lock );
         }
 
         /*

diff --git a/src/video_output/vout_internal.h b/src/video_output/vout_internal.h
index 8b9a578e49ae4df529ab99bd997da6f41ef325e1..77099c69fb936b02e41cf48bd3323d1e1c5bfa32 100644 (file)
--- a/src/video_output/vout_internal.h
+++ b/src/video_output/vout_internal.h
@@ -86,8 +86,15 @@ struct vout_thread_sys_t
     filter_chain_t *p_vf2_chain;
     char           *psz_vf2;
 
-    /* Misc */
-    bool            b_snapshot;     /**< take one snapshot on the next loop */
+    /* Snapshot interface */
+    struct
+    {
+        bool        b_available;
+        int         i_request;
+        picture_t   *p_picture;
+        vlc_mutex_t lock;
+        vlc_cond_t  wait;
+    } snapshot;
 
     /* Show media title on videoutput */
     bool            b_title_show;
@@ -107,7 +114,5 @@ picture_t *vout_RenderPicture( vout_thread_t *, picture_t *,
  */
 void vout_UsePictureLocked( vout_thread_t *p_vout, picture_t *p_pic  );
 
-int vout_Snapshot( vout_thread_t *, picture_t *p_pic );
-
 #endif
 

diff --git a/src/video_output/vout_intf.c b/src/video_output/vout_intf.c
index dece00398be30b616d9492d9c81a6fcd3b312d5b..1d40a45e472e9844ce216e492b750a4417f77aac 100644 (file)
--- a/src/video_output/vout_intf.c
+++ b/src/video_output/vout_intf.c
@@ -584,13 +584,9 @@ static char *VoutSnapshotGetDefaultDirectory( void )
     return psz_path;
 }
 
-int vout_Snapshot( vout_thread_t *p_vout, picture_t *p_picture_private )
+#warning "Remove vout_Snapshot"
+static int vout_Snapshot( vout_thread_t *p_vout, picture_t *p_pic )
 {
-    /* FIXME find why it is sometimes needed (format being wrong/invalid) */
-    picture_t pic = *p_picture_private;
-    pic.format = p_vout->fmt_out;
-    picture_t *p_pic = &pic;
-
     /* */
     char *psz_path = var_GetNonEmptyString( p_vout, "snapshot-path" );
 
@@ -817,6 +813,46 @@ error:
     return VLC_EGENERIC;
 }
 
+static picture_t *VoutGetSnapshot( vout_thread_t *p_vout, mtime_t i_timeout )
+{
+    vout_thread_sys_t *p_sys = p_vout->p;
+
+    vlc_mutex_lock( &p_sys->snapshot.lock );
+    p_sys->snapshot.i_request++;
+
+    const mtime_t i_deadline = mdate() + i_timeout;
+    while( p_sys->snapshot.b_available && !p_sys->snapshot.p_picture &&
+           mdate() < i_deadline )
+    {
+        vlc_cond_timedwait( &p_sys->snapshot.wait, &p_sys->snapshot.lock,
+                            i_timeout );
+    }
+
+    picture_t *p_picture = p_sys->snapshot.p_picture;
+    if( p_picture )
+        p_sys->snapshot.p_picture = p_picture->p_next;
+    else if( p_sys->snapshot.i_request > 0 )
+        p_sys->snapshot.i_request--;
+    vlc_mutex_unlock( &p_sys->snapshot.lock );
+
+    if( !p_picture )
+        msg_Err( p_vout, "Failed to grab a snapshot" );
+
+    return p_picture;
+}
+
+static void VoutSaveSnapshot( vout_thread_t *p_vout )
+{
+    /* 500ms timeout */
+    picture_t *p_picture = VoutGetSnapshot( p_vout, 500*1000 );
+
+    if( p_picture )
+    {
+        vout_Snapshot( p_vout, p_picture );
+        picture_Release( p_picture );
+    }
+}
+
 /*****************************************************************************
  * Handle filters
  *****************************************************************************/
@@ -1222,11 +1258,10 @@ 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);
+
+    VoutSaveSnapshot( p_vout );
     return VLC_SUCCESS;
 }