[Mesa-dev] [PATCH] st/va: add mpeg4 startcode workaround

[Emil Velikov]

On 8 November 2015 at 11:41, Christian König <deathsimple at vodafone.de> wrote:
> On 06.11.2015 20:28, Ilia Mirkin wrote:
>>
>> On Fri, Nov 6, 2015 at 2:15 PM, Christian König <deathsimple at vodafone.de>
>> wrote:
>>>
>>> On 06.11.2015 20:10, Ilia Mirkin wrote:
>>>>
>>>> On Fri, Nov 6, 2015 at 1:48 PM, Zhang, Boyuan <Boyuan.Zhang at amd.com>
>>>> wrote:
>>>>>
>>>>> Hi Emil,
>>>>>
>>>>> Please see the following information about this patch.
>>>>>
>>>>> - Issue: For Mpeg4, the VOP and GOV headers were truncated. With the
>>>>> existing workaround in st/va, playback shows massive corruptions.
>>>>> - This Patch: Provide another way to get the truncated headers back.
>>>>> Massive corruptions are gone with this patch. At the same time, add an
>>>>> environmental variable to allow user to decide whether to use this
>>>>> patch.
>>>>
>>>> Why would the user not want to use this? Sounds like a correctness
>>>> fix, no? Or is it some thing that a hypothetical gallium driver might
>>>> not need but the radeon uvd-based ones do? In that case it should be
>>>> behind a PIPE_VIDEO_CAP_bla (sorry, I'm still not too clear on what
>>>> "bla" is here...)
>>>
>>>
>>> The problem is that this is a rather extreme hack.
>>>
>>> As you probably knew VA-API didn't correctly specify which start code
>>> should
>>> be included and which shouldn't for MPEG-4. This is an issue for AMD as
>>> well
>>> as NVidia hardware and pretty much everybody which sticks close to an
>>> elementary stream.
>>>
>>> What we do in this hack is just searching the bytes *before* the pointer
>>> and
>>> size we got from the application for the stuff that's missing. E.g. we
>>> access memory the application didn't told us to access.
>>>
>>> This is rather speculative, but works surprisingly well with a lot of
>>> applications.
>>
>> Hm, that is a little dodgy indeed. But making user-selectable options
>> (provided via env var) for correct decoding... doesn't seem ideal
>> either. Is there some "correct" way to resolve this without changing
>> the va api?
>
>
> Unfortunately no. I wasn't involved in everything but we had a couple of
> people working on this which have more knowledge about MPEG-4 part 2 than
> me.
>
> A couple of month back somebody from a different team at AMD even tried to
> convince Intel to fix this, but as far as I know without success.
>
> The over all conclusion is that the interface definition of VA-API for
> MPEG-4 part 2 is just a bloody mess.
>
And precisely for the above reasons, I keep on going on like an old
hag to keep writing something in the commit logs.

<rant>

Adding workarounds is fine, as long as they are documented - what it
does (if not obvious), why we need it and even references when
available. Otherwise the next person will just come and remove this
code and/or do something that completely breaks things up, as there is
no information that can prevent (educate) them from doing do.

</rant>

Cheers
Emil