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

[Xiaolei Yu]

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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel