[FFmpeg-devel] Added HW H.264 and HEVC encoding for AMD GPUs based on AMF SDK

[Marton Balint]

On Wed, 22 Nov 2017, Mark Thompson wrote:

> On 22/11/17 22:53, Philip Langdale wrote:
>> On Wed, 22 Nov 2017 23:36:23 +0100
>> Timo Rothenpieler <timo at rothenpieler.org> wrote:
>> 
>>>>> I'd like to look through it again and test a bit more (will try to
>>>>> do so tomorrow, certainly by the end of the week), but I think it
>>>>> should be ready to commit with the external header removed. 
>>>>
>>>> Are you planning to remove Nvidia headers as well? 
>>>
>>> No, I am very much against this.
>>> And others have also voiced against that.
>>>
>>> Also, I don't see a problem with including this AMD header. It very
>>> much increases the accessibility and maintainability(no need to watch
>>> out for potential breaking upstream changes, however likely that
>>> might be). Having those vendor-specific hwaccels enabled in otherwise
>>> generic builds gives a lot of people easy access, or access at all.
>>>
>>> It also might make them potential usage candidates for browsers, no
>>> idea how their weird lawyer-code-review system works though, and if
>>> it has any impact to avoid external dependencies.
>>>
>> 
>> I feel the same way; I'm fine with the header in-tree.
>
> Well, why should this external header be in the ffmpeg tree?
>
> In my opinion indiscriminately placing upstream headers in the tree is just not what we want to do:
> * It increases the maintenance burden, because upstream changes need to be checked and then reflected locally.
> * Breaking changes are completely fatal - we have to choose one side or the other of the change and abandon the other completely.
> * It makes it harder for users to build against different versions which they may have (especially for users who may want to stick with older versions they have verified for their use-cases).
> * It makes it harder to developers to modify code on either side of the API, because more places have to be updated.
>
> And what is the benefit?  A minor convenience gain for users who can't be bothered to get the headers themselves in the same way that they do for every other external package they use.  I don't think that's worth it.
>
> (For comparison, including the Nvidia headers allows the stuff there to be built at all in the face of a hostile upstream.  That's quite different to a minor convenience gain.)

All your points apply to Nvidia external headers as well, and you can't 
set aside the fact that AMD and Nvidia deserve equal treatment. IMHO we 
either include the AMD headers, or we remove the Nvidia headers and make 
the configure error message point the user to an external github repo from 
where the Nvidia headers are downloadable.

Regards,
Marton