[FFmpeg-devel] [PATCH 1/5] libavutil: Some VAAPI infrastructure
sw at jkqxz.net
Sun Jan 17 20:52:12 CET 2016
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
> 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
> For the existing decoders (and their users) to work, it will require
> (a) a global list of connections somewhere to map VADisplay to lock
> (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.
More information about the ffmpeg-devel