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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
