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

[Mironov, Mikhail]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Hendrik Leppkes
> Sent: October 26, 2017 5:59 PM
> To: FFmpeg development discussions and patches <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 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

Several points here for consideration:
1. I am trying to avoid scenario when in the default build NV encoder is available and AMD's is not. I am kind of repeating the argument but I care about users who bought AMD card and may use all kind of builds.
2. The proposed header will be maintained by AMD since I work as AMF SDK architect and can make this decision. I can include it into SDK distribution on GitHub if needed at any time. Also, it will likely be used in other projects that are "C" and don’t need full set of headers. Should this address your second point?
Mikhail