[FFmpeg-devel] [PATCH 1/5] libavutil: Some VAAPI infrastructure

[wm4]

On Sun, 17 Jan 2016 18:13:50 +0000
Mark Thompson <sw at jkqxz.net> wrote:

> On 17/01/16 17:53, wm4 wrote:
> > On Sun, 17 Jan 2016 17:34:55 +0000
> > Mark Thompson <sw at jkqxz.net> wrote:
> >  
> >>   From 2442c1aca8778167c2e60a34d03ed452737f1366 Mon Sep 17 00:00:00 2001
> >> From: Mark Thompson <mrt at jkqxz.net>
> >> Date: Sun, 17 Jan 2016 15:48:54 +0000
> >> Subject: [PATCH 1/5] libavutil: Some VAAPI infrastructure
> >>  
> >  
> >> +
> >> +static AVVAAPIConnection *av_vaapi_connection_list;
> >> +
> >> +int av_vaapi_instance_init(AVVAAPIInstance *instance, const char *device)
> >> +{
> >> +    AVVAAPIConnection *ctx;
> >> +    int err;
> >> +
> >> +    for(ctx = av_vaapi_connection_list; ctx; ctx = ctx->next) {
> >> +        if((device == 0 && ctx->device_string == 0) ||
> >> +           (device && ctx->device_string &&
> >> +            !strcmp(device, ctx->device_string)))
> >> +            break;
> >> +    }  
> >
> > This won't work. Neither vaapi nor your patch are thread-safe, yet you
> > have them as very central global mutable state.
> >  
> 
> True.  That setup is all pretty nasty, and everything currently assumes 
> it happens on the same thread.  Since multiple instances have to use a 
> common connection to libva (because they have to be able to pass 
> surfaces between them), this is unfortunately pretty much required.
> 
> If multithreaded use is desirable immediately then we could just have a 
> big lock which anything VAAPI-related must take when it wants to do 
> anything?  (This would require changes to all existing VAAPI decoders as 
> well.)

There are two issues:
1. global state in libav* which is not synchronized
2. thread-safety within 

1. is is completely unacceptable, because it can trigger undefined
behavior if there is more than 1 libav* user in the same process. I'm
not really convinced that a "device string" is really reliably unique
enough that it won't be a problem across library users. (For example,
it's entirely possible enough to open 2 X11 Displays to the same X
server using the same display name.)

With 2. it's a bit more complicated. There should probably indeed be
something like a big lock around all uses of the same VADisplay, as
long as libva exhibits this problem.