[vlc-devel] commit: First pass to clean up snapshot code (vout). (Laurent Aimar )

git version control git at videolan.org
Sun Mar 8 14:18:08 CET 2009


vlc | branch: master | Laurent Aimar <fenrir at videolan.org> | Sun Mar  8 00:30:38 2009 +0100| [0a3d51a12806e45db99c3ea88477ad407583c6e6] | committer: Laurent Aimar 

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).

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=0a3d51a12806e45db99c3ea88477ad407583c6e6
---

 src/video_output/video_output.c  |   54 ++++++++++++++++++++++++++++++++++---
 src/video_output/vout_internal.h |   13 ++++++---
 src/video_output/vout_intf.c     |   53 +++++++++++++++++++++++++++++++------
 3 files changed, 102 insertions(+), 18 deletions(-)

diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
index d5335ae..27aad54 100644
--- 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 8b9a578..77099c6 100644
--- 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 dece003..1d40a45 100644
--- 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;
 }
 




More information about the vlc-devel mailing list