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

[Hendrik Leppkes]

On Thu, Oct 26, 2017 at 8:03 PM, Mironov, Mikhail
<Mikhail.Mironov at amd.com> wrote:
>> -----Original Message-----
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of wm4
>> Sent: September 29, 2017 12:57 PM
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] Added HW accelerated H.264 and HEVC
>> encoding for AMD GPUs based on AMF SDK
>>
>> On Fri, 29 Sep 2017 15:04:00 +0000
>> "Mironov, Mikhail" <Mikhail.Mironov at amd.com> wrote:
>>
>> > I would like to understand better the nature of the concern. The license is
>> MIT. The paragraph in question is a notice, not limiting the usage of the SDK.
>> > I can definitely reduce number of headers. I can merge all necessary
>> interfaces into one header, though maintenance will take more resources.
>> Which way would you prefer?
>>
>> Ideally, these headers would just be easily installable by whoever wants to
>> build FFmpeg with AMF. This is how it works normally for external libraries.
>>
>> I don't even understand why we added those NVIDIA and avisynth headers
>> (the other things in compat are for basic OS compatibility, so not
>> comparable). For NVIDIA in particular it's probably because installing their
>> SDK is a major PITA and there was something about license issues.
>> Maybe someone else could chime in why this was done?
>>
>> At least for nvenc there was an explanation given in the commit message:
>>
>>     As Nvidia has put the most recent Video Codec SDK behind a double
>>     registration wall, of which one needs manual approval of a lenghty
>>     application, bundling this header saves everyone trying to use NVENC
>>     from that headache.
>>
>>     The header is still MIT licensed and thus fine to bundle with ffmpeg.
>>
>>     Not bundling this header would get ffmpeg stuck at SDK v6, which is
>>     still freely available, holding back future development of the NVENC
>>     encoder.
>>
>> So basically, NVIDIA being... let's say, "not nice". I don't think this will be a
>> problem with AMD.
>>
>> Again, we generally don't add headers for external libraries in-tree.
> I understand your concerns but keeping AMF headers outside of FFmpeg tree would put Nvidia into unfair advantage. And this will happen only because they obscured access to their SDK. I plan to resubmit the patch with a single header file with reduced AMF API. The license will be pure MIT.  Mikhail

Lets try to stay with proper arguments, being "fair" or "unfair" is
unlikely to get you any allowances for a less-then-ideal approach.

So from where I'm standing, I see two major criteria to consider inclusion:

1) Is there a "need" to include the headers, so the feature can be used?

Obviously this is a bit of a flexible argument, but if for example
headers are publicly and freely available, or even packaged and
distributed by linux distributions, the "need" isn't really there. I
didn't look much into this specific case, but you are free to make an
argument here that doesn't revolve around "but NVIDIA got it".

2) Is it feasible to include them from a maintenance/development perspective?

A giant header dump of a lot of unneeded files is clearly not ideal,
and you plan to clean that up, so we'll re-visit that later.
Another part here is that any headers in-tree should be directly
available from official sources "as-is" with no or very little
modifications. If for example this special single header file you're
crafting is something you just made specifically for FFmpeg, and its
not being maintained as part of the official SDK, then this puts an
extra burden on maintaining this header, as we can't just get any
future versions of the SDK and copy the header into FFmpeg - for
example for adding support for a new codec.

At least that is how I would judge any patch that tries to include
third-party headers into the tree, if anyone disagrees with those
criteria or has any additions, feel free to add.

- Hendrik