[Mesa-dev] [PATCH 2/2] st/va: Implement vaExportSurfaceHandle()

[Christian König]

Am 21.09.2017 um 21:00 schrieb Mark Thompson:
> On 20/09/17 09:14, Christian König wrote:
>> Am 20.09.2017 um 00:01 schrieb Mark Thompson:
>>> This is a new interface in libva2 to support wider use-cases of passing
>>> surfaces to external APIs.  In particular, this allows export of NV12 and
>>> P010 surfaces.
>> First of all thanks a lot for taking care of this.
>>
>>> Signed-off-by: Mark Thompson <sw at jkqxz.net>
>>> ---
>>> Trivial update for a minor change requested on libva side (1/2 identical).
>>>
>>> Still unsure on what to do about size and interlacing.  I'll have a look at the postproc code just posted soon, though I think it's pretty much entirely orthogonal to this.
>> Probably best to convert the interlaced representation into the progressive form before exporting.
> I'd prefer not to do that, because a use-case of this is to be able to modify surfaces in-place.  (E.g. decode, scale, export, blend something else onto the surface, fence, encode.)

My suggesting is to change the backing store of the surface while it is 
exported. So in place modifications should work and if the surface is 
then reused for decoding it will directly decode to the progressive format.

But in place modification is NOT something I would recommend here 
anyway. Video surfaces can of course be read and written by the 3D 
block, but it certainly isn't in any ideal memory layout for rendering.

Just copying it into a new surface while you compose things is usually 
way faster.

>> Only alternative I can think of is to define new DRM formats/modifiers/attributes, but then the application needs to be aware of this as well.
> I'm not really sure what form that would need to take.  The modifiers are meant for something else importing the surfaces, so what would support it and how?  That would also require keeping the format part the same, I think (so an NV12 surface would still be an R8 plane and a GR88 plane, but with some modifier meaning it is actually present as two fields).

That sounds similar to what I had in mind as well.

> It could work with the current construction by adding a way to indicate the surface is interlaced with separate fields.  For NV12 it would then export four handles, one for each field of each plane.

Actually that won't be the case. You still have one handle for each 
plane, the format is actually that of a 2D array texture where you can 
select in the sampler if you want to sample from the top or bottom field.

>    mpv at least should be able to handle this (there is already support for something similar for some nvidia vdpau cases),

The reason for this is that this is actually the more common format for 
(embedded) hardware decoders. It just doesn't have a fourcc assigned 
(AFAIK) and isn't used by Intel, so no support in VA-API.

>   but I don't know how acceptable that would be to other users.  Certainly it would be quite a lot harder to modify a surface in-place in a sensible way with that setup.
>
> I'm still unconvinced by the comments in the other thread about using interlaced surfaces by default - a lot of things would be easier if progressive were the default, and I have yet to find any case this actually fails in.

The reason why we use this interlaced memory layout by default is that 
it our deinterlacers only work with this format where you have separate 
top and bottom fields.

Regards,
Christian.

>
> Thanks,
>
> - Mark