On 01/18/2016 03:52 AM, Mark Thompson wrote: > On 17/01/16 19:46, Mark Thompson wrote: >> On 17/01/16 18:46, wm4 wrote: >>> >>> 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.) >> >> Ok, I'm happy with the first part of that (and that it is fixable by a >> simple lock around the connection initialisation, assuming this code >> stays in libavutil). >> >> Can you offer an example where the device strings actually create a >> problem? >> >> Multiple users within the same process /must/ be given the same >> connection if they ask for the same device, because we have no way to >> distinguish different sets of instances which want to be able to work >> together. Equally, two connections to the same device under different >> names are acceptably different, because they won't have come from the >> same instance set. > > Right, I see the problem. The user will want to do something with the > surface they get back under the same X11 display handle. We can't call > XOpenDisplay() in that case: the user has to be able to pass their own handle > in. So we need some other way to register that connection. > >> >>> 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. >> >> This is straightforward to do, if tedious. >> >> Can you explain the ABI and API constraints on changes to existing >> structures? >> >> For the existing decoders (and their users) to work, it will require >> either: >> (a) a global list of connections somewhere to map VADisplay to lock >> or >> (b) an additional member in struct vaapi_context to point to the lock. >> >> If ABI and API compatibility is required for existing users then (b) is >> out, and we have to have the global list (suitably locked). >> >> If we can break both then the right answer is probably to pass >> hwaccel_context to encoders as well, and add a similar field to >> AVFilterContext to use there too. >> >> If ABI compatibility is required but an API break is allowed then we >> could do horrible things to hack (b) into working. For example, replace >> the VADisplay pointer in the first member of struct vaapi_context to >> instead point at a new structure which contains some magic bytes at the >> start. If the magic bytes are where that pointer goes then we are using >> the new API and can lock using that, and if they are not found then it >> was a user-provided VADisplay and no locking is required. >> >> - Mark >> >> >> PS: I have no attachment to this piece of code (around connection >> initialisation) at all; it was just required to make everything else >> work. If you want to suggest a better and completely different approach >> then I am happy to throw it all away and start again. >>
I think you can supply VADisplay to AVCodecContext through av_opt_ptr and leave its initialization to user. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel