VAAPI-XCB: rewrite Surface handling.

The surface handling has been rewritten to be more consistent and to
avoid reusing surfaces that are still in use by the display thread. Some
VAAPI wrapper drivers use the DirectSurface mode, meaning that the surface
handed by the application is used directly for displaying. It does not
copy the surface into a display surface. When the driver does this, then
you cannot reuse this surface which is being displayed for rendering the
next picture too

modules/codec/avcodec/va.c

@@ -60,123 +60,223 @@ static void vlc_va_unlock(void)
 }
 
 /* */
-static VASurfaceID *vlc_va_alloc_surfaces( vlc_va_conn_t *conn,
-    const int width, const int height, const int fourcc, unsigned int *requested )
+static vlc_va_surface_t *alloc_surfaces( vlc_va_conn_t *conn,
+                                         const int width, const int height,
+                                         const int fourcc, unsigned int *requested )
 {
     assert( *requested > 0 );
 
-    VASurfaceID *p_surfaces = (VASurfaceID *)calloc(*requested, sizeof(VASurfaceID));
-    if( p_surfaces == NULL )
+    VASurfaceID *p_ids = (VASurfaceID *)calloc(*requested, sizeof(VASurfaceID));
+    if( p_ids == NULL )
         return NULL;
 
     VAStatus status = vaCreateSurfaces( conn->p_display, width, height, fourcc,
-                                        *requested, p_surfaces );
+                                        *requested, p_ids );
     if( status != VA_STATUS_SUCCESS )
     {
-        free( p_surfaces );
+        free( p_ids );
+        return NULL;
+    }
+
+    vlc_va_surface_t *p_surfaces = calloc(*requested, sizeof(vlc_va_surface_t));
+    if( p_surfaces == NULL )
+    {
+        free( p_ids );
         return NULL;
     }
 
+    for( unsigned int i = 0; i < *requested; i++ )
+    {
+        p_surfaces[i].i_id = p_ids[i];
+        vlc_mutex_init(&p_surfaces[i].lock);
+    }
+
+    free( p_ids );
     return p_surfaces;
 }
 
-static VASurfaceID *vlc_va_realloc_surfaces( vlc_va_conn_t *conn,
-    const int width, const int height, const int fourcc, unsigned int *requested )
+static vlc_va_surface_t *realloc_surfaces( vlc_va_conn_t *conn,
+                                           const int width, const int height,
+                                           const int fourcc, unsigned int *requested )
 {
     assert( *requested > 0 );
 
-    VASurfaceID *p_surfaces = (VASurfaceID *)realloc( conn->p_surface_ids,
-                                                     (*requested) * sizeof(VASurfaceID) );
+    vlc_va_surface_t *p_surfaces = (vlc_va_surface_t *)realloc( conn->pool.p_surfaces,
+                                                                (*requested) * sizeof(vlc_va_surface_t) );
     if( p_surfaces == NULL )
         return NULL;
 
-    int count = *requested - conn->i_surface_count;
+    const unsigned int count = *requested - conn->pool.count;
     assert( count > 0 );
 
-    VASurfaceID *p_tmp = &p_surfaces[conn->i_surface_count + 1];
+    VASurfaceID *ids = calloc(count, sizeof(VASurfaceID));
+    if( ids == NULL )
+        return NULL;
     VAStatus status = vaCreateSurfaces( conn->p_display, width, height, fourcc,
-                                        count, p_tmp );
+                                        count, ids );
     if( status != VA_STATUS_SUCCESS )
+        abort(); /* FATAL ERROR */
+
+    for( unsigned int i = 0; i < count; i++ )
     {
-        free( p_surfaces );
-        return NULL;
+        vlc_va_surface_t *p = &p_surfaces[conn->pool.count + i];
+        p->i_id = ids[i];
+        vlc_mutex_init(&p->lock);
     }
+    conn->pool.count += count;
+    assert(conn->pool.count == *requested);
+    free(ids);
     return p_surfaces;
 }
 
-static VASurfaceID *vlc_va_create_surfaces( vlc_va_conn_t *conn,
-    const int width, const int height, const int fourcc, const unsigned int num )
+static bool create_surfaces( vlc_va_conn_t *conn,
+                                    const int width, const int height,
+                                    const int fourcc, const unsigned int num )
 {
     assert(conn);
 
     if( (width <= 0) || (height <= 0) || (num <= 0) )
-        return NULL;
+        return false;
 
     if( conn->i_ref_count > 1 )
     {
-        assert( conn->p_surface_ids != NULL );
+        assert( conn->pool.p_surfaces != NULL );
 
-        if( num > conn->i_surface_count )
+        if( num > conn->pool.count )
         {
             assert(0);
             /* FIXME: probably wise to destroy decoding context first
                and then recreate it */
             unsigned requested = num;
-            vlc_va_realloc_surfaces( conn, width, height, fourcc, &requested );
+            realloc_surfaces( conn, width, height, fourcc, &requested );
             assert( num == requested );
         }
-        return conn->p_surface_ids;
+        return true;
     }
 
-    assert( conn->p_surface_ids == NULL );
-    assert( conn->i_surface_count == 0 );
+    assert( conn->pool.p_surfaces == NULL );
+    assert( conn->pool.count == 0 );
 
     unsigned requested = num;
-    conn->p_surface_ids = vlc_va_alloc_surfaces( conn, width, height,
-                                                fourcc, &requested );
+    conn->pool.p_surfaces = alloc_surfaces( conn, width, height,
+                                            fourcc, &requested );
     assert( num == requested );
-    if( conn->p_surface_ids )
-        conn->i_surface_count = requested;
-    return conn->p_surface_ids;
+    conn->pool.current = 0;
+    if( conn->pool.p_surfaces )
+    {
+        conn->pool.count = requested;
+        return true;
+    }
+    return false;
 }
 
-static void vlc_va_destroy_surfaces( vlc_va_conn_t *conn )
+static void destroy_surfaces( vlc_va_conn_t *conn )
 {
     assert( conn );
-    assert( conn->i_surface_count > 0 );
-    assert( conn->p_surface_ids );
+    assert( conn->pool.count > 0 );
+    assert( conn->pool.p_surfaces );
 
     if( conn->i_ref_count > 1 )
         return;
 
-    for( unsigned int i = 0; i < conn->i_surface_count; i++ )
+    for( unsigned int i = 0; i < conn->pool.count; i++ )
     {
-        VASurfaceID i_id = conn->p_surface_ids[i];
+        vlc_va_surface_t *surface = &conn->pool.p_surfaces[i];
+
+        vlc_mutex_lock(&surface->lock);
+        if( surface->i_refcount > 1 )
+            abort();
+
 #if VA_CHECK_VERSION(0,31,0)
-        if( vaSyncSurface( conn->p_display, i_id ) )
+        if( vaSyncSurface( conn->p_display, surface->i_id ) )
 #else
-        if( vaSyncSurface( conn->p_display, conn->i_context_id, i_id ) )
+        if( vaSyncSurface( conn->p_display, conn->i_context_id, surface->i_id ) )
 #endif
         {
             fprintf(stderr, "Fatal error reboot your system\n" );
             abort();
         }
+
+        surface->i_refcount --;
+        assert(surface->i_refcount == 0);
+
+        vaDestroySurfaces( conn->p_display, &surface->i_id, 1 );
+        vlc_mutex_unlock(&surface->lock);
+
+        vlc_mutex_destroy(&surface->lock);
     }
 
-    if( conn->p_surface_ids != NULL )
-        vaDestroySurfaces( conn->p_display, conn->p_surface_ids,
-                           conn->i_surface_count );
+    free(conn->pool.p_surfaces);
+
+    conn->pool.count = 0;
+    conn->pool.p_surfaces = NULL;
+}
+
+static vlc_va_surface_t *get_surface(vlc_va_conn_t *conn)
+{
+    assert( conn );
+    assert( conn->pool.count > 0 );
+    assert( conn->pool.p_surfaces );
 
-    free(conn->p_surface_ids );
+    unsigned int next = conn->pool.current;
+    unsigned int tries = 0;
+    while (tries < conn->pool.count)
+    {
+        VAStatus status;
+        VASurfaceStatus ready;
+        vlc_va_surface_t *surface;
+
+        surface = &conn->pool.p_surfaces[next];
+
+        vlc_mutex_lock(&surface->lock);
+        VASurfaceID id = surface->i_id;
+        status = vaQuerySurfaceStatus(conn->p_display, id, &ready);
+        if ((status == VA_STATUS_SUCCESS) &&
+            (ready == VASurfaceReady) &&
+            (surface->i_refcount == 0))
+        {
+            surface->i_refcount++;
+            conn->pool.current = (next + 1) % conn->pool.count;
+            vlc_mutex_unlock(&surface->lock);
+            return surface;
+        }
+        vlc_mutex_unlock(&surface->lock);
+
+        next = (next + 1) % conn->pool.count;
+        tries++;
+    }
+    return NULL;
+}
 
-    conn->i_surface_count = 0;
-    conn->p_surface_ids = NULL;
+static void release_surface(vlc_va_conn_t *conn, vlc_va_surface_t *surface)
+{
+    assert( conn );
+    assert( conn->pool.count > 0 );
+    assert( conn->pool.p_surfaces );
+    assert( surface );
+
+    vlc_mutex_lock(&surface->lock);
+    if (surface->i_refcount == 0)
+        abort();
+
+    VAStatus status;
+    VASurfaceStatus ready;
+    VASurfaceID id = surface->i_id;
+    status = vaQuerySurfaceStatus(conn->p_display, id, &ready);
+    if ((status == VA_STATUS_SUCCESS) &&
+        (ready == VASurfaceReady))
+    {
+        surface->i_refcount--;
+        assert(surface->i_refcount == 0);
+    }
+    vlc_mutex_unlock(&surface->lock);
 }
 
 /* Global VAAPI connection state */
 static vlc_va_conn_t vlc_va_conn = {
     .x11 = NULL,
     .p_display = 0,
+    .b_direct_rendering = false,
     /* libva version */
     .i_version_major = 0,
     .i_version_minor = 0,
@@ -186,11 +286,16 @@ static vlc_va_conn_t vlc_va_conn = {
     .lock = vlc_va_lock,
     .unlock = vlc_va_unlock,
     /* surfaces - NOTE: must be called with lock held */
-    .i_surface_count = 0,
-    .p_surface_ids = NULL,
+    .pool = {
+        .count = 0,
+        .current = 0,
+        .p_surfaces = NULL,
+    },
     .i_context_id = VA_INVALID_ID,
-    .create_surfaces = vlc_va_create_surfaces,
-    .destroy_surfaces = vlc_va_destroy_surfaces,
+    .create_surfaces = create_surfaces,
+    .destroy_surfaces = destroy_surfaces,
+    .get_surface = get_surface,
+    .release_surface = release_surface,
 };
 
 /* */
@@ -199,6 +304,32 @@ static vlc_va_conn_t *vlc_va_get_conn( void )
     return (vlc_va_conn_t *) &vlc_va_conn;
 }
 
+static bool uses_direct_mapping(vlc_va_conn_t *conn)
+{
+    VADisplayAttribute attr;
+    VAStatus status;
+
+#if VA_CHECK_VERSION(0,34,0)
+    attr.type  = VADisplayAttribRenderMode;
+    attr.flags = VA_DISPLAY_ATTRIB_GETTABLE;
+
+    status = vaGetDisplayAttributes(conn->p_display, &attr, 1);
+    if (status == VA_STATUS_SUCCESS)
+        return !(attr.value & (VA_RENDER_MODE_LOCAL_OVERLAY|
+                               VA_RENDER_MODE_EXTERNAL_OVERLAY));
+#else
+    /* Does the driver use DirectSurface mode then we cannot use the
+       same surface while it is still being in the display pipeline. */
+    attr.type  = VADisplayAttribDirectSurface;
+    attr.flags = VA_DISPLAY_ATTRIB_GETTABLE;
+
+    status = vaGetDisplayAttributes(conn->p_display, &attr, 1);
+    if (status == VA_STATUS_SUCCESS)
+        return !attr.value;
+#endif
+    return false;
+}
+
 vlc_va_conn_t *vlc_va_Initialize( const char *display )
 {
     vlc_mutex_lock( &vlc_va_conn_lock );
@@ -227,6 +358,7 @@ vlc_va_conn_t *vlc_va_Initialize( const char *display )
         goto error;
 
     conn->i_ref_count++;
+    conn->b_direct_rendering = uses_direct_mapping(conn);
     vlc_mutex_unlock( &vlc_va_conn_lock );
     return conn;
 
@@ -246,8 +378,8 @@ void vlc_va_Terminate( vlc_va_conn_t *conn )
     conn->i_ref_count--;
     if( conn->i_ref_count == 0 )
     {
-        assert( conn->i_surface_count == 0 );
-        assert( conn->p_surface_ids == NULL );
+        assert( conn->pool.count == 0 );
+        assert( conn->pool.p_surfaces == NULL );
 
         assert( conn->p_display );
         vaTerminate( conn->p_display );

modules/codec/avcodec/va.h

@@ -28,6 +28,7 @@ typedef struct vlc_va_t vlc_va_t;
 
 struct vlc_va_t {
     char *description;
+    bool direct_rendering;
     vlc_object_t *obj;
 
     int  (*setup)(vlc_va_t *, void **hw, vlc_fourcc_t *output,
@@ -37,6 +38,7 @@ struct vlc_va_t {
     int  (*extract)(vlc_va_t *, picture_t *dst, AVFrame *src);
     void (*put)(vlc_va_t *, AVFrame *dst, picture_t *src);
     int  (*display)(vlc_va_t *, picture_t *dst);
+    bool (*query)(vlc_va_t *, picture_t *src);
     void (*close)(vlc_va_t *);
 };
 
@@ -61,6 +63,10 @@ static inline void vlc_va_Put(vlc_va_t *va, AVFrame *dst, picture_t *src)
 {
     va->put(va, dst, src);
 }
+static inline bool vlc_va_QueryReady(vlc_va_t *va, picture_t *src)
+{
+    return va->query(va, src);
+}
 static inline int vlc_va_Display(vlc_va_t *va, picture_t *dst)
 {
     return va->display(va, dst);

modules/codec/avcodec/vaapi.c

@@ -57,19 +57,15 @@ typedef struct
     vlc_va_conn_t *conn;
 
     VAConfigID    i_config_id;
-    VAContextID   i_context_id;
 
     struct vaapi_context hw_ctx;
 
     /* */
     int          i_surface_count;
-    unsigned int i_surface_order;
     int          i_surface_width;
     int          i_surface_height;
     vlc_fourcc_t i_surface_chroma;
 
-    vlc_va_surface_t *p_surface;
-
     VAImage      image;
     copy_cache_t image_cache;
 
@@ -117,7 +113,7 @@ static int Open( vlc_va_vaapi_t *p_va, int i_codec_id, int i_count )
         break;
     case CODEC_ID_H264:
         i_profile = VAProfileH264High;
-        i_surface_count = 16+1;
+        i_surface_count = 20+1;
         break;
     default:
         return VLC_EGENERIC;
@@ -130,7 +126,6 @@ static int Open( vlc_va_vaapi_t *p_va, int i_codec_id, int i_count )
     /* */
     memset( p_va, 0, sizeof(*p_va) );
     p_va->i_config_id  = VA_INVALID_ID;
-    p_va->i_context_id = VA_INVALID_ID;
     p_va->image.image_id = VA_INVALID_ID;
 
     /* Create a VA display */
@@ -198,21 +193,6 @@ error:
     return VLC_EGENERIC;
 }
 
-static inline void DestroySurfaces( vlc_va_conn_t *conn )
-{
-    assert( conn );
-    assert( conn->p_surface_ids );
-    conn->destroy_surfaces( conn );
-}
-
-
-static inline VASurfaceID *CreateSurfaces( vlc_va_conn_t *conn,
-    const int width, const int height, const int fourcc, const int num )
-{
-    assert( conn );
-    return conn->create_surfaces( conn, width, height, fourcc, num );
-}
-
 static void DestroyDecodingContext( vlc_va_vaapi_t *p_va )
 {
     p_va->conn->lock();
@@ -226,16 +206,14 @@ static void DestroyDecodingContext( vlc_va_vaapi_t *p_va )
         CopyCleanCache( &p_va->image_cache );
     }
 
-    if( p_va->i_context_id != VA_INVALID_ID )
-        vaDestroyContext( p_va->conn->p_display, p_va->i_context_id );
+    if( p_va->conn->i_context_id != VA_INVALID_ID )
+        vaDestroyContext( p_va->conn->p_display, p_va->conn->i_context_id );
 
-    DestroySurfaces( p_va->conn );
-    free( p_va->p_surface );
+    vlc_va_conn_DestroySurfaces( p_va->conn );
 
     /* */
+    p_va->conn->i_context_id = VA_INVALID_ID;
     p_va->image.image_id = VA_INVALID_ID;
-    p_va->i_context_id = VA_INVALID_ID;
-    p_va->p_surface = NULL;
     p_va->i_surface_width = 0;
     p_va->i_surface_height = 0;
     p_va->conn->unlock();
@@ -249,44 +227,37 @@ static int CreateDecodingContext( vlc_va_vaapi_t *p_va, void **pp_hw_ctx, vlc_fo
     p_va->conn->lock();
 
     /* Create surfaces */
-    VASurfaceID *pi_surface_id = CreateSurfaces( p_va->conn, i_width, i_height,
-                                                VA_RT_FORMAT_YUV420, p_va->i_surface_count );
-    if( pi_surface_id == NULL )
+    bool ok = vlc_va_conn_CreateSurface( p_va->conn, i_width, i_height,
+                                         VA_RT_FORMAT_YUV420, p_va->i_surface_count );
+    if( !ok )
     {
-        p_va->i_surface_count = 0;
         p_va->conn->unlock();
         return VLC_EGENERIC;
     }
 
     /* */
-    p_va->i_surface_count = p_va->conn->i_surface_count;
-    p_va->p_surface = calloc( p_va->i_surface_count, sizeof(*p_va->p_surface) );
-    if( !p_va->p_surface )
-    {
-        p_va->conn->unlock();
-        return VLC_EGENERIC;
-    }
     p_va->image.image_id = VA_INVALID_ID;
-    p_va->i_context_id   = VA_INVALID_ID;
+    assert(p_va->conn->i_context_id == VA_INVALID_ID);
 
-    for( int i = 0; i < p_va->i_surface_count; i++ )
+    /* Create a context */
+    const unsigned int count = vlc_va_conn_SurfacesCount(p_va->conn);
+    VASurfaceID *p_ids = calloc(count, sizeof(VASurfaceID));
+    if( !p_ids )
+        goto error;
+    for( unsigned int i = 0; i < count; i++ )
     {
-        vlc_va_surface_t *p_surface = &p_va->p_surface[i];
-
-        p_surface->i_id = pi_surface_id[i];
-        p_surface->i_refcount = 0;
-        p_surface->i_order = 0;
+        p_ids[i] = p_va->conn->pool.p_surfaces[i].i_id;
     }
-
-    /* Create a context */
     if( vaCreateContext( p_va->conn->p_display, p_va->i_config_id,
                          i_width, i_height, VA_PROGRESSIVE,
-                         pi_surface_id, p_va->i_surface_count, &p_va->i_context_id ) )
+                         p_ids, count,
+                         &p_va->conn->i_context_id ) != VA_STATUS_SUCCESS )
     {
-        p_va->i_context_id = VA_INVALID_ID;
+        free(p_ids);
+        p_va->conn->i_context_id = VA_INVALID_ID;
         goto error;
     }
-    p_va->conn->i_context_id = p_va->i_context_id;
+    free(p_ids);
 
     /* Find and create a supported image chroma */
     int i_fmt_count = vaMaxNumImageFormats( p_va->conn->p_display );
@@ -301,7 +272,7 @@ static int CreateDecodingContext( vlc_va_vaapi_t *p_va, void **pp_hw_ctx, vlc_fo
     }
 
     VAImage testImage;
-    if(vaDeriveImage(p_va->conn->p_display, pi_surface_id[0], &testImage) == VA_STATUS_SUCCESS)
+    if(vaDeriveImage(p_va->conn->p_display, p_va->conn->pool.p_surfaces[0].i_id, &testImage) == VA_STATUS_SUCCESS)
     {
         p_va->b_supports_derive = true;
         vaDestroyImage(p_va->conn->p_display, testImage.image_id);
@@ -320,7 +291,8 @@ static int CreateDecodingContext( vlc_va_vaapi_t *p_va, void **pp_hw_ctx, vlc_fo
                 continue;
             }
             /* Validate that vaGetImage works with this format */
-            if( vaGetImage( p_va->conn->p_display, pi_surface_id[0],
+            if( vaGetImage( p_va->conn->p_display,
+                            p_va->conn->pool.p_surfaces[0].i_id,
                             0, 0, i_width, i_height,
                             p_va->image.image_id) )
             {
@@ -353,7 +325,7 @@ static int CreateDecodingContext( vlc_va_vaapi_t *p_va, void **pp_hw_ctx, vlc_fo
     memset( &p_va->hw_ctx, 0, sizeof(p_va->hw_ctx) );
     p_va->hw_ctx.display    = p_va->conn->p_display;
     p_va->hw_ctx.config_id  = p_va->i_config_id;
-    p_va->hw_ctx.context_id = p_va->i_context_id;
+    p_va->hw_ctx.context_id = p_va->conn->i_context_id;
 
     /* */
     p_va->i_surface_chroma = i_chroma;
@@ -406,7 +378,7 @@ static int Extract( vlc_va_t *p_external, picture_t *p_picture, AVFrame *p_ff )
 #if VA_CHECK_VERSION(0,31,0)
     if( vaSyncSurface( p_va->conn->p_display, i_surface_id ) )
 #else
-    if( vaSyncSurface( p_va->conn->p_display, p_va->i_context_id, i_surface_id ) )
+    if( vaSyncSurface( p_va->conn->p_display, p_va->conn->i_context_id, i_surface_id ) )
 #endif
         goto error;
 
@@ -490,7 +462,7 @@ static void Put(vlc_va_t *p_external, AVFrame *p_ff, picture_t *p_picture )
         abort();
 
     picture_sys_t *p_surface = p_picture->p_sys;
-    assert(p_surface->i_refcount == 1);
+    assert(p_surface->i_refcount == 2);
 
     /* */
     for( int i = 0; i < 4; i++ )
@@ -519,7 +491,7 @@ static int DisplayPicture( vlc_va_t *p_external, picture_t *p_picture )
 #if VA_CHECK_VERSION(0,31,0)
     if( vaSyncSurface( p_va->conn->p_display, i_surface_id ) )
 #else
-    if( vaSyncSurface( p_va->conn->p_display, p_va->i_context_id, i_surface_id ) )
+    if( vaSyncSurface( p_va->conn->p_display, p_va->conn->i_context_id, i_surface_id ) )
 #endif
         goto error;
 
@@ -535,33 +507,19 @@ error:
 static int Get( vlc_va_t *p_external, AVFrame *p_ff )
 {
     vlc_va_vaapi_t *p_va = vlc_va_vaapi_Get(p_external);
-    int i_old;
-    int i;
-
-    /* Grab an unused surface, in case none are, try the oldest
-     * XXX using the oldest is a workaround in case a problem happens with ffmpeg */
-    for( i = 0, i_old = 0; i < p_va->i_surface_count; i++ )
-    {
-        vlc_va_surface_t *p_surface = &p_va->p_surface[i];
 
-        if( p_surface->i_refcount == 0 )
-            break;
+    p_va->conn->lock();
 
-        if( p_surface->i_order < p_va->p_surface[i_old].i_order )
-            i_old = i;
+    vlc_va_surface_t *p_surface = vlc_va_conn_GetSurface(p_va->conn);
+    if (!p_surface)
+    {
+        msg_Warn(p_external->obj, "No free surface found.");
+        p_va->conn->unlock();
+        return VLC_EGENERIC;
     }
-    if( i >= p_va->i_surface_count )
-        i = i_old;
-
-    vlc_va_surface_t *p_surface = &p_va->p_surface[i];
-
-    /* NOTE: reset surface refcount otherwise they will be leaked and
-     * mayhem may happen down the road.
-     */
-    p_surface->i_refcount = 1;
-    p_surface->i_order = p_va->i_surface_order++;
 
     /* */
+    vlc_mutex_lock(&p_surface->lock);
     for( int i = 0; i < 4; i++ )
     {
         p_ff->data[i] = NULL;
@@ -570,6 +528,9 @@ static int Get( vlc_va_t *p_external, AVFrame *p_ff )
         if( i == 0 || i == 3 )
             p_ff->data[i] = (void*)(uintptr_t)p_surface->i_id;/* Yummie */
     }
+    vlc_mutex_unlock(&p_surface->lock);
+    p_va->conn->unlock();
+
     return VLC_SUCCESS;
 }
 
@@ -577,18 +538,20 @@ static void Release( vlc_va_t *p_external, AVFrame *p_ff )
 {
     vlc_va_vaapi_t *p_va = vlc_va_vaapi_Get(p_external);
 
+    p_va->conn->lock();
     VASurfaceID i_surface_id = (VASurfaceID)(uintptr_t)p_ff->data[3];
 
-    for( int i = 0; i < p_va->i_surface_count; i++ )
+    const unsigned int count = vlc_va_conn_SurfacesCount(p_va->conn);
+    for( unsigned int i = 0; i < count; i++ )
     {
-        vlc_va_surface_t *p_surface = &p_va->p_surface[i];
+        vlc_va_surface_t *p_surface = &p_va->conn->pool.p_surfaces[i];
+        if (!p_surface)
+            abort();
 
         if( p_surface->i_id == i_surface_id )
-        {
-            if( p_surface->i_refcount > 0 )
-                p_surface->i_refcount--;
-        }
+            vlc_va_conn_ReleaseSurface(p_va->conn, p_surface);
     }
+    p_va->conn->unlock();
 }
 
 static void Close( vlc_va_vaapi_t *p_va )
@@ -615,6 +578,33 @@ static void Delete( vlc_va_t *p_external )
     free( p_va );
 }
 
+static bool QuerySurfaceReady( vlc_va_t *p_external, picture_t *pic )
+{
+    vlc_va_vaapi_t *p_va = vlc_va_vaapi_Get(p_external);
+
+    assert(pic);
+    assert(pic->p_sys);
+    assert(p_va);
+
+    if (!p_va->conn)
+        abort();
+
+    p_va->conn->lock();
+
+    bool ready = false;
+    VAStatus status;
+    VASurfaceStatus surface;
+    VASurfaceID id = pic->p_sys->i_id;
+    status = vaQuerySurfaceStatus(p_va->conn->p_display, id, &surface);
+    if ((status == VA_STATUS_SUCCESS) &&
+        (surface == VASurfaceReady))
+        ready = true;
+
+    p_va->conn->unlock();
+
+    return ready;
+}
+
 /* */
 vlc_va_t *vlc_va_NewVaapi( vlc_object_t *obj, int i_codec_id, int i_count )
 {
@@ -632,6 +622,7 @@ vlc_va_t *vlc_va_NewVaapi( vlc_object_t *obj, int i_codec_id, int i_count )
     }
 
     /* */
+    p_va->va.direct_rendering = p_va->conn->b_direct_rendering;
     p_va->va.obj = obj;
     p_va->va.setup = Setup;
     p_va->va.get = Get;
@@ -639,6 +630,7 @@ vlc_va_t *vlc_va_NewVaapi( vlc_object_t *obj, int i_codec_id, int i_count )
     p_va->va.release = Release;
     p_va->va.extract = Extract;
     p_va->va.display = DisplayPicture;
+    p_va->va.query = QuerySurfaceReady;
     p_va->va.close = Delete;
     return &p_va->va;
 }

modules/codec/avcodec/vaapi.h

@@ -29,10 +29,13 @@
  * context.
  */
 typedef struct vlc_va_conn_t vlc_va_conn_t;
+typedef struct picture_sys_t vlc_va_surface_t;
+
 struct vlc_va_conn_t
 {
     Display      *x11; /* x11 display connection */
     VADisplay     p_display;
+    bool          b_direct_rendering; /* VAAPI uses direct rendering */
     int           i_version_major;
     int           i_version_minor;
     int           i_ref_count;
@@ -41,14 +44,21 @@ struct vlc_va_conn_t
     void (*lock)(void);
     void (*unlock)(void);
 
-    /* NOTE: must be called with lock held */
-    unsigned int  i_surface_count;
-    VASurfaceID  *p_surface_ids;
+    /* NOTE: must be called with conn->lock held */
+    struct {
+        unsigned int count;   /* number of surfaces allocated */
+        unsigned int current; /* current used surface (private use only) */
+        vlc_va_surface_t *p_surfaces;
+    } pool; /* vlc_va_surface_t pool */
+
     VAContextID   i_context_id;
 
-    VASurfaceID *(*create_surfaces)(vlc_va_conn_t *conn, const int width, const int height,
+    bool (*create_surfaces)(vlc_va_conn_t *conn, const int width, const int height,
                                    const int fourcc, const unsigned num);
     void (*destroy_surfaces)(vlc_va_conn_t *conn);
+
+    vlc_va_surface_t *(*get_surface)(vlc_va_conn_t *conn);
+    void (*release_surface)(vlc_va_conn_t *conn, vlc_va_surface_t *surface);
 };
 
 /* Initialize shared connection to libva */
@@ -57,7 +67,7 @@ vlc_va_conn_t *vlc_va_Initialize(const char *display);
 /* Deinitialize shared connection to libva */
 void vlc_va_Terminate(vlc_va_conn_t *conn);
 
-static inline VASurfaceID *vlc_va_conn_CreateSurface(vlc_va_conn_t *conn,
+static inline bool vlc_va_conn_CreateSurface(vlc_va_conn_t *conn,
     const int width, const int height, const int fourcc, const int num)
 {
     return conn->create_surfaces(conn, width, height, fourcc, num);
@@ -68,34 +78,43 @@ static inline void vlc_va_conn_DestroySurfaces(vlc_va_conn_t *conn)
     conn->destroy_surfaces(conn);
 }
 
-static inline VASurfaceID *vlc_va_conn_GetSurfaces(vlc_va_conn_t *conn)
+static inline vlc_va_surface_t *vlc_va_conn_GetSurface(vlc_va_conn_t *conn)
 {
-    return conn->p_surface_ids;
+    return conn->get_surface(conn);
 }
 
-static inline int vlc_va_conn_SurfacesCount(vlc_va_conn_t *conn)
+static inline void vlc_va_conn_ReleaseSurface(vlc_va_conn_t *conn, vlc_va_surface_t *surface)
 {
-    return conn->i_surface_count;
+    conn->release_surface(conn, surface);
 }
 
-/* vlc_va_surface_t */
-typedef struct
+static inline unsigned int vlc_va_conn_SurfacesCount(vlc_va_conn_t *conn)
 {
-    VASurfaceID  i_id;
-    int          i_refcount;
-    unsigned int i_order;
-} vlc_va_surface_t;
+    return conn->pool.count;
+}
 
 /* picture_sys_t */
 struct picture_sys_t
 {
-    /* Link surface with picture_sys_t permanently */
     VASurfaceID  i_id;
     int          i_refcount;
+    unsigned int i_cache;    /* subpicture cache identifier */
 
-    /* subpicture */
-    unsigned int i_cache;
+    vlc_mutex_t  lock;
 };
 
+static inline void vlc_va_SurfaceHold(vlc_va_surface_t *surface)
+{
+    vlc_mutex_lock(&surface->lock);
+    surface->i_refcount++;
+    vlc_mutex_unlock(&surface->lock);
+}
+static inline void vlc_va_SurfaceRelease(vlc_va_surface_t *surface)
+{
+    vlc_mutex_lock(&surface->lock);
+    surface->i_refcount--;
+    vlc_mutex_unlock(&surface->lock);
+}
+
 #endif
 #endif

modules/codec/avcodec/vaapi_x11.c

@@ -357,7 +357,7 @@ void CloseVaapiX11(vlc_object_t *obj)
     if (sys->vaconn)
     {
         sys->vaconn->lock();
-        sys->vaconn->destroy_surfaces(sys->vaconn);
+        vlc_va_conn_DestroySurfaces(sys->vaconn);
         sys->vaconn->unlock();
         vlc_va_Terminate(sys->vaconn);
     }
@@ -572,6 +572,18 @@ static void DisplayVASurface(vout_display_t *vd, picture_t *picture)
 
     vout_display_place_t place;
     vout_display_PlacePicture(&place, &vd->source, vd->cfg, false);
+/*
+#define VA_FILTER_SCALING_DEFAULT       0x00000000
+#define VA_FILTER_SCALING_FAST          0x00000100
+#define VA_FILTER_SCALING_HQ            0x00000200
+#define VA_FILTER_SCALING_NL_ANAMORPHIC 0x00000300
+#define VA_FILTER_SCALING_MASK          0x00000f00
+*/
+    unsigned int flags = VA_FILTER_SCALING_DEFAULT;
+    /* NOTE: vaCreateContext() specifies VA_PROGRESSIVE */
+    flags |= picture->b_progressive ? VA_FRAME_PICTURE :
+                 (picture->b_top_field_first ? VA_TOP_FIELD : VA_BOTTOM_FIELD);
+    msg_Dbg( vd, "Displaying surface id: %d / %p", surface->i_id, picture );
 
     VAStatus status;
     status = vaPutSurface(sys->vaconn->p_display, surface->i_id, sys->window,
@@ -579,9 +591,7 @@ static void DisplayVASurface(vout_display_t *vd, picture_t *picture)
                           vd->source.i_visible_width, vd->source.i_visible_height,
                           place.x, place.y, place.width, place.height,
                           NULL, 0, /* clipping */
-                          /* NOTE: vaCreateContext() specifies VA_PROGRESSIVE */
-                          picture->b_progressive ? VA_FRAME_PICTURE :
-                               (picture->b_top_field_first ? VA_TOP_FIELD : VA_BOTTOM_FIELD));
+                          flags);
     if (status != VA_STATUS_SUCCESS)
         msg_Err(vd, "failed displaying picture: %d (surface id=%d): %p",
                 status, surface->i_id, picture);
@@ -648,9 +658,11 @@ out:
  */
 static int PictureLock(picture_t *picture)
 {
-    picture_sys_t *surface = picture->p_sys;
-    if (surface)
-        surface->i_refcount++;
+    vlc_va_surface_t *surface = picture->p_sys;
+    if (surface == NULL)
+        abort();
+
+    vlc_va_SurfaceHold(surface);
     return VLC_SUCCESS;
 }
 
@@ -658,12 +670,11 @@ static void PictureUnlock(picture_t *picture)
 {
     assert(picture->p_sys);
 
-    picture_sys_t *surface = picture->p_sys;
+    vlc_va_surface_t *surface = picture->p_sys;
     if (surface == NULL)
         abort();
 
-    if (surface->i_refcount > 0)
-        surface->i_refcount--;
+    vlc_va_SurfaceRelease(surface);
 }
 
 /* PictureRelease is called when the picture pool is destroyed. */
@@ -672,7 +683,6 @@ static void PictureRelease(picture_t *picture)
     if (picture->i_refcount > 0)
         picture->i_refcount--;
 
-    free(picture->p_sys);
     picture->p_sys = NULL;
 }
 
@@ -689,7 +699,7 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
 
     /* Create surfaces */
     sys->vaconn->lock();
-    unsigned alloc_count = __MAX(sys->vaconn->i_surface_count, requested_count);
+    unsigned int alloc_count = __MAX(vlc_va_conn_SurfacesCount(sys->vaconn), requested_count);
 
     picture_t **pic_array = (picture_t **) calloc(alloc_count, sizeof(picture_t *));
     if (!pic_array)
@@ -702,21 +712,11 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
         if (!pic)
             break;
         assert(pic->p_sys == NULL);
-        picture_sys_t *pic_sys = (picture_sys_t *) malloc(sizeof(picture_sys_t));
-        if (!pic_sys)
-        {
-            picture_Release(pic);
-            break;
-        }
-        pic->format.i_chroma = VLC_CODEC_VAAPI_SURFACE;
 
-        pic_sys->i_id = VA_INVALID_SURFACE;
-        pic_sys->i_refcount = 0;
-        pic_sys->i_cache = 0;
+        pic->format.i_chroma = VLC_CODEC_VAAPI_SURFACE;
 
         pic_array[count] = pic;
         pic_array[count]->b_progressive = true;
-        pic_array[count]->p_sys = pic_sys;
         pic_array[count]->pf_release = PictureRelease;
     }
 
@@ -725,24 +725,26 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
         goto error;
 
     unsigned int i_surface_count = __MIN(count, alloc_count);
-
-    /* Create surfaces */
-    VASurfaceID *pi_surface_id;
-    pi_surface_id = sys->vaconn->create_surfaces(sys->vaconn,
-                                vd->fmt.i_visible_width, vd->fmt.i_visible_height,
-                                VA_RT_FORMAT_YUV420, i_surface_count);
-    assert( pi_surface_id );
     assert( i_surface_count == count );
 
-    if (pi_surface_id == NULL)
+    msg_Info( vd, "Requested %d/%d surfaces for picture pool.",
+              i_surface_count, vlc_va_conn_SurfacesCount(sys->vaconn) );
+
+    /* Create surfaces */
+    if( !vlc_va_conn_CreateSurface( sys->vaconn,
+                                    vd->fmt.i_visible_width,
+                                    vd->fmt.i_visible_height,
+                                    VA_RT_FORMAT_YUV420, i_surface_count ) )
+    {
+        /* FAIL */
         goto error;
+    }
 
     for (unsigned int i = 0; i < i_surface_count; i++)
     {
-        picture_sys_t *pic_sys = pic_array[i]->p_sys;
-        assert(pic_sys);
-
-        pic_sys->i_id = pi_surface_id[i];
+        vlc_va_surface_t *surface = vlc_va_conn_GetSurface( sys->vaconn );
+        if( surface )
+            pic_array[i]->p_sys = (picture_sys_t *)surface;
     }
 
     /* Register pool with video output core */
@@ -770,7 +772,14 @@ error:
     {
         picture_t *pic = pic_array[i];
         if (pic)
+        {
+            if (pic->p_sys)
+            {
+                vlc_va_surface_t *surface = (vlc_va_surface_t *) pic->p_sys;
+                vlc_va_conn_ReleaseSurface(sys->vaconn, surface);
+            }
             picture_Release(pic);
+        }
     }
     free(pic_array);