[Mesa-dev] [PATCH] st/va: Make VAAPI_DISABLE_INTERLACE default true

[Andy Furniss]

Julien Isorce wrote:
> On 16 September 2016 at 08:43, Christian König
> <deathsimple at vodafone.de> wrote:
>
>> Am 15.09.2016 um 23:07 schrieb Julien Isorce:
>>
>>
>>
>> On 15 September 2016 at 16:02, Leo Liu <leo.liu at amd.com> wrote:
>>
>>>
>>>
>>> On 09/15/2016 10:43 AM, Andy Furniss wrote:
>>>
>>>> Since bf901a2 st/va: also honors interlaced preference when
>>>> providing a video format existing scripts and most use cases
>>>> will need true.
>>>>
>>>> Signed-off-by: Andy Furniss <adf.lists at gmail.com> ---
>>>> src/gallium/state_trackers/va/surface.c | 2 +- 1 file changed,
>>>> 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/gallium/state_trackers/va/surface.c
>>>> b/src/gallium/state_trackers/va/surface.c index
>>>> 00df69d..e73e17e 100644 ---
>>>> a/src/gallium/state_trackers/va/surface.c +++
>>>> b/src/gallium/state_trackers/va/surface.c @@ -43,7 +43,7 @@
>>>>
>>>> #include "va_private.h"
>>>>
>>>> -DEBUG_GET_ONCE_BOOL_OPTION(nointerlace,
>>>> "VAAPI_DISABLE_INTERLACE", FALSE);
>>>> +DEBUG_GET_ONCE_BOOL_OPTION(nointerlace,
>>>> "VAAPI_DISABLE_INTERLACE", TRUE);
>>>>
>>>
>>> Like being mentioned,  It'll still override the preferred
>>> interlaced format when this env is not explicitly used.
>>>
>>> Not sure this will be okay with other case. @Julien?
>>>
>>
>> Hi,
>>
>> So the problem is that with radeon,
>> PIPE_VIDEO_CAP_SUPPORTS_INTERLACED always returns false for
>> encoding but can return true for decoding (depending on the chipset
>> and the codec). Then when doing transcoding you need all to be non
>> interlaced to avoid extra copy/conversion and even a clash. Is it
>> correct ?
>>
>>
>> Yes correct. The decoder supports both interlaced as well as
>> progressive memory layout, but the encoder can only handle
>> progressive input.
>>
>> Interlaced memory layout is needed for things like adaptive
>> deinterlacing as well as VDPAU OpenGL interop.
>>
>
>> Should debug_get_option_nointerlace() be moved to
>> radeon_video.c::rvid_get_video_param ?
>>
>>
>> For the short term that sounds like a good idea to me.
>>
>
>
> Hi @Andy, could you update your patch to move that debug env flag to
> radeon ?  Thx

I don't know mesa code that well so maybe I misunderstand, but, if I
default to true and test in there it will affect vdpau decode as well
which I guess will disable de-interlace and maybe worse (IIRC messing
with progressive vs interlaced broke something vdpau last time I tried).

So I don't know how to make the flag only affect vaapi usage from there.

>> In the mid term we need to better handle this case. E.g. allocate
>> the video buffer with the layout the driver reports as preferred,
>> if that doesn't match the use case (transcoding, deinterlacing or
>> interop) convert as needed.
>>
>
> yes, also if the conversion is done in HW that should still
> acceptable. But also it should be a way to configure that from
> gstreamer-vaapi/libva using VA_SURFACE_ATTRIB_USAGE_HINT_ENCODER /
> VA_SURFACE_ATTRIB_USAGE_HINT_VPP_READ maybe ... and catch this flag
> in vlVaCreateSurfaces2.
>
>
>>
>>
>> Other question:
>>
>> case PIPE_VIDEO_CAP_SUPPORTS_INTERLACED: if (rscreen->family <
>> CHIP_PALM) { /* MPEG2 only with shaders and no support for
>> interlacing on R6xx style UVD */ return codec !=
>> PIPE_VIDEO_FORMAT_MPEG12 && rscreen->family > CHIP_RV770; } else {
>> if (u_reduce_video_profile(profile) == PIPE_VIDEO_FORMAT_HEVC)
>> return false; //The firmware doesn't support interlaced HEVC.
>> return true; } So if instead it would always return false then it
>> will really work on hardware for which above code says true ?
>>
>>
>> It would work, but the deinterlacing and interop use case I noted
>> above would break.
>>
>>
>
>> Because my understanding is that for nvidia hardware this is not a
>> preference but rather a requirement but I might be wrong.
>>
>>
>> Yes, as far as I know that is correct. AMD hardware can handle both
>> for most hardware/profile combinations, even the HEVC limit only
>> applies to a certain firmware version IIRC.
>>
>>
>> In any case, with current upstream code and
>> VAAPI_DISABLE_INTERLACE=1 it hits "assert(templat->interlaced);" in
>> nouveau_vp3_video_buffer_create. If I remove the asset it crashes
>> and can even stall the driver (just wanted to check ).
>>
>>
>> Crap, feel free to revert the patch setting it to true by default.
>>
>
> It has not been pushed:
> https://cgit.freedesktop.org/mesa/mesa/log/src/gallium/state_trackers/va
>
>
so still time for @Andy to update his patch.
>
>
> Cheers Julien
>
>
>> Regards, Christian.
>>
>> Cheers Julien
>>
>>
>>> Regards, Leo
>>>
>>>
>>>> #include <va/va_drmcommon.h>
>>>>
>>>>
>>>
>>
>>
>> _______________________________________________ mesa-dev mailing
>> listmesa-dev at lists.freedesktop.orghttps://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
>>
>
>>
>
>
> _______________________________________________ mesa-dev mailing
> list mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>