[Mesa-dev] RFC: Fixing XBMC crash with NV_vdpau_interop

[Christian König]

Am 24.03.2014 18:54, schrieb Roland Scheidegger:
> Am 23.03.2014 12:36, schrieb Christian König:
>> Am 22.03.2014 23:33, schrieb Brian Paul:
>>> On Sat, Mar 22, 2014 at 2:49 PM, Christian König
>>> <deathsimple at vodafone.de <mailto:deathsimple at vodafone.de>> wrote:
>>>
>>>      Hi guys,
>>>
>>>      recently some XBMC users complained about crashes with the
>>>      relatively new NV_vdpau_interop support.
>>>
>>>      That turned out to be a problem with how st_atom_texture.c caches
>>>      the sampler view for a texture. Since the texture in question is
>>>      shared between two GLX contexts the pipe object the sampler view
>>>      was originally created for doesn't necessary match the pipe object
>>>      it is used with.
>>>
>>>      Now my question is am I missing something or is this case really
>>>      not correctly supported? Where is the check if a texture is used
>>>      in more than one context? The attached patch fixes the issue, but
>>>      I'm not sure if it is the right approach.
>>>
>>>
>>> I've run into this too.  I think the best thing to do (short of
>>> removing the sampler view from the st_texture_object) is to walk over
>>> all the textures in the share group at context-destroy time, looking
>>> for sampler views belonging to the context being destroyed, then free
>>> those sampler views.
>> Yeah, that's also a problem, but not what I'm currently dealing with.
>>
>> My problem is that we pass a sampler view to a pipe context which
>> doesn't belong to this context. Surprisingly the driver doesn't crash
>> immediately, but instead works fine for at least some time.
>>
>> Our radeonsi driver keeps an internal reference to all sample views
>> bound to it, and so when some times later a new sampler view is bound we
>> unreference the sampler view in question and crash because the context
>> where this sampler view was created with no longer exists.
>>
>> So it's reference inside the driver that crashes, not the one in the
>> mesa state tracker. So there is probably nothing I can do except for
>> what the attached patch does and try to never bind a sampler view to a
>> context it doesn't belong to.
>>
>>> I could probably whip up a patch next week.
>> Would you mind if I try to fix that? Doesn't sounds so complicated to me.
>>
> Some texture sharing problems crop up from time to time due to this
> (textures being sharable by GL, but sampler views are strictly
> per-context in gallium).
> If the sampler view would be destroyed when the context is destroyed,
> wouldn't that also get rid of the reference in the driver (though I
> guess you should unbind that texture if it's still bound at that point)?
> Though arguably, you are not supposed to use sampler views belonging to
> another context even if said context still exists.
> So, I think recreating the sampler view when the current one belongs to
> a different context is fine, though maybe it would be better if the
> views would be separated from the st_texture_object and stored somewhere
> else in the context.
>
> Roland

Completely agree. I just send out two patches to the list to fix this.

The first one now properly destroys the sampler views when the contexts 
are destroyed.

The second one adds a small dynamically increasing array of samplers 
views to the texture, so that each context gets it's own entry in that 
array.

Please review,
Christian.