[Mesa-stable] [Mesa-dev] [PATCH] st/vdpau: allow progressive video surface with interop

[Leo Liu]

On 05/07/2018 09:14 AM, Christian König wrote:
> Am 07.05.2018 um 15:03 schrieb Leo Liu:
>>
>>
>> On 05/07/2018 05:10 AM, Christian König wrote:
>>> Am 02.05.2018 um 16:51 schrieb Leo Liu:
>>>> mpv now interop with video surface instead of output surface 
>>>> previously,
>>>> so it fails with "vlVdpVideoSurfaceDMABuf", this's fine for Mesa 
>>>> GL, since
>>>> the code path will fall back to "vlVdpVideoSurfaceGallium", but this's
>>>> not the case for others
>>>>
>>>> Signed-off-by: Leo Liu <leo.liu at amd.com>
>>>> Cc: Christian König <christian.koenig at amd.com>
>>>> Cc: "18.1 18.0" <mesa-stable at lists.freedesktop.org>
>>>
>>> That won't work correctly.
>>>
>>> The NV_VDPAU_interop extension we implement with that needs 
>>> interlaced layout or otherwise can't correctly work with the surfaces.
>> I thought the same way in the beginning, then later I asked myself 
>> why it's working with Mesa interop, then I found it resolved with:
>>
>> static struct pipe_resource *st_vdpau_video_surface_gallium(struct 
>> gl_context *ctx, const void *vdpSurface,
>> ...
>>
>>    samplers = buffer->get_sampler_view_planes(buffer);
>>    if (!samplers)
>>       return NULL;
>>
>>    sv = samplers[index >> 1];
>>    if (!sv)
>>       return NULL;
>>
>>    pipe_resource_reference(&res, sv->texture);
>> }
>>
>> The above code gave me the hint for this patch, and the patch is 
>> tested okay with Mesa GL and other GL.
>
> That is just a hack I introduced for testing which also doesn't work 
> correctly. E.g. it won't sample correctly from the odd/even fields and 
> instead interpolate from the whole frame. In other words you get an 
> more or less correct output with that, it's just not binary identical.
>
> But for the DMA-buf interop case that won't work because the importer 
> expects for surfaces with X height and not two with 2*X height. That 
> you don't get a corrupted picture in your test properly pure 
> coincident because of how we share the DMA-buf tilling data in the 
> background.
>
> When you want to work with DMA-buf sharing the exported surface *MUST* 
> be in interlaced form.
>
> That either leaves us with copying from the progressive to the 
> interlaced form during export,
That's my original thought for the solution. we might have to go this path.
Thanks for the explanation.

Regards,
Leo



> or change the firmware to directly decode everything into the 
> interlaced format.
>
> Regards,
> Christian.
>
>>
>> Regards,
>> Leo
>>
>>
>>>
>>> It's probably pure coincident that you don't get a messed up picture 
>>> with that.
>>>
>>> Christian.
>>>
>>>> ---
>>>>   src/gallium/state_trackers/vdpau/surface.c | 5 ++++-
>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/gallium/state_trackers/vdpau/surface.c 
>>>> b/src/gallium/state_trackers/vdpau/surface.c
>>>> index 012d303641..d63e761350 100644
>>>> --- a/src/gallium/state_trackers/vdpau/surface.c
>>>> +++ b/src/gallium/state_trackers/vdpau/surface.c
>>>> @@ -513,12 +513,15 @@ VdpStatus 
>>>> vlVdpVideoSurfaceDMABuf(VdpVideoSurface surface,
>>>>      }
>>>>        /* Check if surface match interop requirements */
>>>> -   if (p_surf->video_buffer == NULL || 
>>>> !p_surf->video_buffer->interlaced ||
>>>> +   if (p_surf->video_buffer == NULL ||
>>>>          p_surf->video_buffer->buffer_format != PIPE_FORMAT_NV12) {
>>>>         mtx_unlock(&p_surf->device->mutex);
>>>>         return VDP_STATUS_NO_IMPLEMENTATION;
>>>>      }
>>>>   +   if (!p_surf->video_buffer->interlaced)
>>>> +      plane >>= 1;
>>>> +
>>>>      surf = 
>>>> p_surf->video_buffer->get_surfaces(p_surf->video_buffer)[plane];
>>>>      if (!surf) {
>>>>         mtx_unlock(&p_surf->device->mutex);
>>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>