VAAPI-X11: let the picture pool control pictures AND VAAPI surfaces

The video output is now responsible for the surfaces and the pictures. It
needs a special path in modules/codec/avcodec/video.c to make it happen.
This simplifies the code and locking considerably.

modules/codec/avcodec/va.h

@@ -34,7 +34,8 @@ struct vlc_va_t {
     int  (*get)(vlc_va_t *, AVFrame *frame);
     void (*release)(vlc_va_t *, AVFrame *frame);
     int  (*extract)(vlc_va_t *, picture_t *dst, AVFrame *src);
-    int  (*display)(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);
     void (*close)(vlc_va_t *);
 };
 
@@ -55,9 +56,13 @@ static inline int vlc_va_Extract(vlc_va_t *va, picture_t *dst, AVFrame *src)
 {
     return va->extract(va, dst, src);
 }
-static inline int vlc_va_Display(vlc_va_t *va, picture_t *dst, AVFrame *src)
+static inline void vlc_va_Put(vlc_va_t *va, AVFrame *dst, picture_t *src)
 {
-    return va->display(va, dst, src);
+    va->put(va, dst, src);
+}
+static inline int vlc_va_Display(vlc_va_t *va, picture_t *dst)
+{
+    return va->display(va, dst);
 }
 static inline void vlc_va_Delete(vlc_va_t *va)
 {

modules/codec/avcodec/vaapi.c

@@ -98,23 +98,23 @@ static int Open( vlc_va_vaapi_t *p_va, int i_codec_id )
     case CODEC_ID_MPEG1VIDEO:
     case CODEC_ID_MPEG2VIDEO:
         i_profile = VAProfileMPEG2Main;
-        i_surface_count = 17+2+1;
+        i_surface_count = 2+1;
         break;
     case CODEC_ID_MPEG4:
         i_profile = VAProfileMPEG4AdvancedSimple;
-        i_surface_count = 17+2+1;
+        i_surface_count = 2+1;
         break;
     case CODEC_ID_WMV3:
         i_profile = VAProfileVC1Main;
-        i_surface_count = 17+2+1;
+        i_surface_count = +1;
         break;
     case CODEC_ID_VC1:
         i_profile = VAProfileVC1Advanced;
-        i_surface_count = 17+2+1;
+        i_surface_count = 2+1;
         break;
     case CODEC_ID_H264:
         i_profile = VAProfileH264High;
-        i_surface_count = 13+16+1;
+        i_surface_count = 16+1;
         break;
     default:
         return VLC_EGENERIC;
@@ -407,34 +407,44 @@ error:
     return VLC_EGENERIC;
 }
 
-static vlc_va_surface_t *FindSurface( vlc_va_t *p_external, const VASurfaceID i_surface_id )
+static void Put(vlc_va_t *p_external, AVFrame *p_ff, picture_t *p_picture )
 {
     vlc_va_vaapi_t *p_va = vlc_va_vaapi_Get(p_external);
-    vlc_va_surface_t *p_surface = NULL;
 
-    for( int i = 0; i < p_va->i_surface_count; i++ )
+    p_va->conn->lock();
+
+    if (!p_picture->p_sys)
+        abort();
+
+    assert( p_picture->p_sys->i_state == FREE );
+
+    picture_sys_t *p_surface = p_picture->p_sys;
+    p_surface->i_state = DECODE;
+    p_surface->i_order = p_va->i_surface_order++;
+
+    /* */
+    for( int i = 0; i < 4; i++ )
     {
-        vlc_va_surface_t *p_tmp = &p_va->p_surface[i];
-        if( p_tmp->i_id == i_surface_id )
-        {
-            assert( p_tmp->i_state == DECODE );
-            assert( p_tmp->i_refcount == 1 );
+        p_ff->data[i] = NULL;
+        p_ff->linesize[i] = 0;
 
-            p_tmp->i_state = DISPLAY;
-            p_surface = p_tmp;
-            break;
-        }
+        if( i == 0 || i == 3 )
+            p_ff->data[i] = (void*)(uintptr_t)p_surface->i_id;/* Yummie */
     }
-    return p_surface;
+
+    p_va->conn->unlock();
 }
 
-static int DisplayPicture( vlc_va_t *p_external, picture_t *p_picture, AVFrame *p_ff )
+static int DisplayPicture( vlc_va_t *p_external, picture_t *p_picture )
 {
     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];
+    if (!p_picture->p_sys)
+        abort();
+
+    VASurfaceID i_surface_id = (VASurfaceID)p_picture->p_sys->i_id;
 
 #if VA_CHECK_VERSION(0,31,0)
     if( vaSyncSurface( p_va->conn->p_display, i_surface_id ) )
@@ -443,13 +453,9 @@ static int DisplayPicture( vlc_va_t *p_external, picture_t *p_picture, AVFrame *
 #endif
         goto error;
 
-    if (!p_picture->p_sys)
-        abort();
+    assert( p_picture->p_sys->i_state == DECODE );
 
-    /* FindSurface */
-    p_picture->p_sys->surface = FindSurface( p_external, i_surface_id );
-    if( !p_picture->p_sys->surface )
-        abort();
+    p_picture->p_sys->i_state = DISPLAY;
 
     p_va->conn->unlock();
     return VLC_SUCCESS;
@@ -573,6 +579,7 @@ vlc_va_t *vlc_va_NewVaapi( vlc_object_t *obj, int i_codec_id )
     p_va->va.obj = obj;
     p_va->va.setup = Setup;
     p_va->va.get = Get;
+    p_va->va.put = Put;
     p_va->va.release = Release;
     p_va->va.extract = Extract;
     p_va->va.display = DisplayPicture;

modules/codec/avcodec/vaapi.h

@@ -44,9 +44,17 @@ typedef struct
 
 struct picture_sys_t
 {
-    /* surface */
-    vlc_va_surface_t *surface;
+    /* */
     vlc_object_t     *obj;
+
+    /* Link surface with picture_sys_t permanently */
+    VASurfaceID  i_id;
+    int          i_refcount; /* no longer needed */
+    unsigned int i_order;    /* no longer needed */
+    vlc_va_state_t i_state;  /* no longer needed */
+
+    /* subpicture */
+    unsigned int i_cache;
 };
 
 #endif

modules/codec/avcodec/vaapi_x11.c

@@ -401,7 +401,7 @@ int OpenVaapiX11 (vlc_object_t *obj)
     if (FindVAFourCC(vd) != VLC_SUCCESS)
         goto error;
 
-    msg_Dbg(vd, "using VAAPI X11 window (version %d.%d)",
+    msg_Info(vd, "using VAAPI X11 window (version %d.%d)",
             sys->conn->i_version_major, sys->conn->i_version_minor);
 
     /* */
@@ -482,15 +482,15 @@ void CloseVaapiX11 (vlc_object_t *obj)
     vlc_array_clear(&sys->cache);
 
     /* */
-    if (sys->conn)
-        vlc_va_Terminate(sys->conn);
-
     if (sys->pool)
     {
         picture_pool_Delete(sys->pool);
         sys->pool = NULL;
     }
 
+    if (sys->conn)
+        vlc_va_Terminate(sys->conn);
+
     free(vd->sys);
 }
 
@@ -822,7 +822,7 @@ out_va_free:
 static int RenderDirectSubpicture(vout_display_t *vd, picture_t *picture, subpicture_t *subpicture)
 {
     vout_display_sys_t *sys = vd->sys;
-    vlc_va_surface_t *surface = picture->p_sys->surface;
+    picture_sys_t *surface = picture->p_sys;
 
     subpicture_cache_t *cache = cache_new();
     if (!cache)
@@ -931,7 +931,7 @@ static int RenderCachedSubpictures(vout_display_t *vd, picture_t *picture,
                                    subpicture_t *subpicture, subpicture_cache_t *cache)
 {
     vout_display_sys_t *sys = vd->sys;
-    vlc_va_surface_t *surface = picture->p_sys->surface;
+    picture_sys_t *surface = picture->p_sys;
 
     const int count = vlc_array_count(&cache->subpictures);
 #ifdef VAAPI_DEBUG
@@ -1003,13 +1003,13 @@ static void Render(vout_display_t *vd, picture_t *picture, subpicture_t *subpict
     assert(!subpicture->b_ephemer);
 
     /* Cleanup for reclaimed surface */
-    if (picture->p_sys && picture->p_sys->surface &&
-        (picture->p_sys->surface->i_cache != VA_INVALID_ID) &&
+    if (picture->p_sys &&
+        (picture->p_sys->i_cache != VA_INVALID_ID) &&
         (picture->format.i_chroma == VLC_CODEC_VAAPI_SURFACE))
     {
         assert(0);
 #if 0
-        vlc_va_surface_t *surface = picture->p_sys->surface;
+        picture_sys_t *surface = picture->p_sys;
         if (surface->i_cache != VA_INVALID_ID)
         {
             vlc_mutex_lock(&sys->cache_lock);
@@ -1038,7 +1038,7 @@ static void Render(vout_display_t *vd, picture_t *picture, subpicture_t *subpict
 
     assert(picture->format.i_chroma == VLC_CODEC_VAAPI_SURFACE);
 
-    if (!picture->p_sys->surface)
+    if (!picture->p_sys)
         return;
 
     /* count RGBA subpicture regions */
@@ -1121,6 +1121,16 @@ static void PictureRelease(picture_t *picture)
     if (picture->i_refcount > 0)
         picture->i_refcount--;
 
+    if (picture->p_sys &&
+       (picture->p_sys->i_id != VA_INVALID_SURFACE))
+    {
+        vout_display_t *vd = (vout_display_t *)picture->p_sys->obj;
+
+        vd->sys->conn->lock();
+        vaDestroySurfaces(vd->sys->conn->p_display, &picture->p_sys->i_id, 1);
+        vd->sys->conn->unlock();
+    }
+
     free(picture->p_sys);
     picture->p_sys = NULL;
 }
@@ -1165,7 +1175,7 @@ static void DisplayPicture(vout_display_t *vd, picture_t *pic, subpicture_t *sub
         return;
     }
 
-    if (pic->p_sys->surface == NULL)
+    if (pic->p_sys->i_id == VA_INVALID_SURFACE)
     {
         assert(pic->format.i_chroma == VLC_CODEC_VAAPI_SURFACE);
         msg_Err(vd, "discarding picture without surface information: %p (refcount %d, forced %s)",
@@ -1177,7 +1187,7 @@ static void DisplayPicture(vout_display_t *vd, picture_t *pic, subpicture_t *sub
     }
     assert(pic->format.i_chroma == VLC_CODEC_VAAPI_SURFACE);
 
-    vlc_va_surface_t *surface = pic->p_sys->surface;
+    picture_sys_t *surface = pic->p_sys;
 
     if (surface->i_state == DISPLAY)
         DisplayVASurface(vd, surface->i_id, pic);
@@ -1226,7 +1236,7 @@ static void DisplayPicture(vout_display_t *vd, picture_t *pic, subpicture_t *sub
  */
 static int PictureLock(picture_t *picture)
 {
-    vlc_va_surface_t *surface = picture->p_sys->surface;
+    picture_sys_t *surface = picture->p_sys;
     if (surface)
         surface->i_refcount++;
     return VLC_SUCCESS;
@@ -1236,8 +1246,7 @@ static void PictureUnlock(picture_t *picture)
 {
     assert(picture->p_sys);
 
-    vlc_va_surface_t *surface = (vlc_va_surface_t *) picture->p_sys->surface;
-    assert(surface);
+    picture_sys_t *surface = picture->p_sys;
     if (surface == NULL)
         return;
 
@@ -1275,8 +1284,6 @@ static void PictureUnlock(picture_t *picture)
     surface->i_refcount = 0;
     surface->i_cache = VA_INVALID_ID;
     surface->i_state = FREE;
-
-    picture->p_sys->surface = NULL;
 }
 
 /**
@@ -1301,26 +1308,53 @@ static picture_pool_t *Pool (vout_display_t *vd, unsigned requested_count)
         if (!pic)
             break;
         assert(pic->p_sys == NULL);
-        picture_sys_t *sys = (picture_sys_t *) malloc(sizeof(picture_sys_t));
-        if (!sys)
+        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;
 
-        sys->surface = NULL;
-        sys->obj = VLC_OBJECT(vd);
+        pic_sys->i_id = VA_INVALID_SURFACE;
+        pic_sys->i_refcount = 0; /* No longer needed ? */
+        pic_sys->i_order = 0;    /* No longer needed ? */
+        pic_sys->i_state = FREE; /* No longer needed ? */
+        pic_sys->i_cache = VA_INVALID_ID;
+        pic_sys->obj = VLC_OBJECT(vd);
 
         pic_array[count] = pic;
         pic_array[count]->b_progressive = true;
-        pic_array[count]->p_sys = sys;
+        pic_array[count]->p_sys = pic_sys;
         pic_array[count]->pf_release = PictureRelease;
     }
 
+    int i_surface_count = count;
+    VASurfaceID pi_surface_id[i_surface_count];
+
     if (count == 0)
         goto error;
 
+    /* Create surfaces */
+    sys->conn->lock();
+    if (vaCreateSurfaces(sys->conn->p_display, vd->fmt.i_visible_width,
+                         vd->fmt.i_visible_height, VA_RT_FORMAT_YUV420,
+                         i_surface_count, pi_surface_id))
+    {
+        sys->conn->unlock();
+        goto error;
+    }
+
+    for (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];
+    }
+    sys->conn->unlock();
+
+    /* Register pool with video output core */
     picture_pool_configuration_t cfg;
 
     memset(&cfg, 0, sizeof(cfg));

modules/codec/avcodec/video.c

@@ -703,7 +703,16 @@ picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )
         if( !b_drawpicture || ( !p_sys->p_va && !p_sys->p_ff_pic->linesize[0] ) )
             continue;
 
-        if( !p_sys->p_ff_pic->opaque )
+        if( p_sys->p_va && p_sys->p_ff_pic->opaque )
+        {
+            p_pic = (picture_t *)p_sys->p_ff_pic->opaque;
+            decoder_LinkPicture( p_dec, p_pic );
+
+            /* Fill p_picture_t from AVVideoFrame and do chroma conversion
+             * if needed */
+            ffmpeg_CopyPicture( p_dec, p_pic, p_sys->p_ff_pic );
+        }
+        else if( !p_sys->p_ff_pic->opaque )
         {
             /* Get a new picture */
             p_pic = ffmpeg_NewPictBuf( p_dec, p_context );
@@ -888,7 +897,7 @@ static void ffmpeg_CopyPicture( decoder_t *p_dec,
     if( p_sys->p_va )
     {
         if (p_pic->format.i_chroma == VLC_CODEC_VAAPI_SURFACE)
-            vlc_va_Display( p_sys->p_va, p_pic, p_ff_pic );
+            vlc_va_Display( p_sys->p_va, p_pic );
         else
             vlc_va_Extract( p_sys->p_va, p_pic, p_ff_pic );
     }
@@ -942,28 +951,42 @@ static int ffmpeg_GetFrameBuf( struct AVCodecContext *p_context,
 
     if( p_sys->p_va )
     {
-#ifdef HAVE_AVCODEC_VA
-        /* hwaccel_context is not present in old fffmpeg version */
-        if( vlc_va_Setup( p_sys->p_va,
-                          &p_sys->p_context->hwaccel_context, &p_dec->fmt_out.video.i_chroma,
-                          p_sys->p_context->width, p_sys->p_context->height ) )
+        p_pic = ffmpeg_NewPictBuf( p_dec, p_sys->p_context );
+        if( p_pic && p_pic->format.i_chroma == VLC_CODEC_VAAPI_SURFACE )
         {
-            msg_Err( p_dec, "vlc_va_Setup failed" );
-            return -1;
+            p_ff_pic->opaque = (void*)p_pic;
+            p_ff_pic->type = FF_BUFFER_TYPE_USER;
+            /* FIXME what is that, should give good value */
+            p_ff_pic->age = 256*256*256*64; // FIXME FIXME from ffmpeg
+
+            vlc_va_Put( p_sys->p_va, p_ff_pic, p_pic );
         }
+        else
+        {
+            if( p_pic ) picture_Release( p_pic );
+#ifdef HAVE_AVCODEC_VA
+            /* hwaccel_context is not present in old fffmpeg version */
+            if( vlc_va_Setup( p_sys->p_va,
+                              &p_sys->p_context->hwaccel_context, &p_dec->fmt_out.video.i_chroma,
+                              p_sys->p_context->width, p_sys->p_context->height ) )
+            {
+                msg_Err( p_dec, "vlc_va_Setup failed" );
+                return -1;
+            }
 #else
-        assert(0);
+            assert(0);
 #endif
 
-        /* */
-        p_ff_pic->type = FF_BUFFER_TYPE_USER;
-        /* FIXME what is that, should give good value */
-        p_ff_pic->age = 256*256*256*64; // FIXME FIXME from ffmpeg
+            /* */
+            p_ff_pic->type = FF_BUFFER_TYPE_USER;
+            /* FIXME what is that, should give good value */
+            p_ff_pic->age = 256*256*256*64; // FIXME FIXME from ffmpeg
 
-        if( vlc_va_Get( p_sys->p_va, p_ff_pic ) )
-        {
-            msg_Err( p_dec, "VaGrabSurface failed" );
-            return -1;
+            if( vlc_va_Get( p_sys->p_va, p_ff_pic ) )
+            {
+                msg_Err( p_dec, "VaGrabSurface failed" );
+                return -1;
+            }
         }
         return 0;
     }
@@ -1080,7 +1103,11 @@ static void ffmpeg_ReleaseFrameBuf( struct AVCodecContext *p_context,
 
     if( p_sys->p_va )
     {
-        vlc_va_Release( p_sys->p_va, p_ff_pic );
+        picture_t *p_pic = (picture_t*)p_ff_pic->opaque;
+        if( p_pic && p_pic->format.i_chroma == VLC_CODEC_VAAPI_SURFACE )
+           decoder_UnlinkPicture( p_dec, p_pic );
+        else
+            vlc_va_Release( p_sys->p_va, p_ff_pic );
     }
     else if( !p_ff_pic->opaque )
     {