Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-12-10 Thread Michael Niedermayer
On Sat, Nov 29, 2014 at 10:29:50AM -0800, Philip Langdale wrote:
 On Sat, 29 Nov 2014 19:00:02 +0100
 Timo Rothenpieler t...@rothenpieler.org wrote:
 
   does supporting these additional features need the extra complexity
   that the nvidia version has ?
   or could these features be added into your version while keeping its
   simplicity ? (note do not copy any code from the nvidia one as its
   not redistriutable nor *GPL compatible with the current license
   headers)
  
  I haven't even looked at their code for quite exactly that reason.
  The primary problem with NVENC is, that there is only very poor
  documentation available, so I can only implement stuff based on
  reading the header and experimenting with the API and the available
  examples. NVIDIA employees clearly are in a better position for this,
  as they most likely have a much larger insight into how NVENC works
  internaly. I'm quite sure all of their features could be added to my
  version.
 
 I'm already 'polluted' until they correct their licensing, but I can at
 least describe what they have.
 
 The main piece of complexity in their implementation is they have an
 additional abstraction layer between the ffmpeg encoder implementation
 and the nvenc API. So this layer 'insulates' the two APIs. From a
 licensing point of view it's meaningless, but I think that's the reason
 why it exists, based on a brief email exchange with them. If that was
 squashed, it would make theirs more straight forward. There's also a
 bunch of utility code that handles argument mapping between x264 args
 and nvenc args. As I've written elsewhere - I think this is a very
 useful thing to do, as people are, broadly, familiar with how to tell
 x264 to do stuff.
 
 And as I mentioned before, they have a directx linked initialisation
 path as an alternative to the cuda one. I'm not sure what conditions
 that would be more useful under, but it's documented in the SDK.
 
 Hopefully, they will respond positively on Monday and will get engaged
 here, and sort out their licensing so that we can all work on a single
 implementation.

monday passed long ago
what was their response ?
is there a reason to wait / not apply the patch in this thread ?


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-12-10 Thread Philip Langdale

On 2014-12-10 15:17, Michael Niedermayer wrote:

On Sat, Nov 29, 2014 at 10:29:50AM -0800, Philip Langdale wrote:

[...]
Hopefully, they will respond positively on Monday and will get engaged
here, and sort out their licensing so that we can all work on a single
implementation.


monday passed long ago
what was their response ?
is there a reason to wait / not apply the patch in this thread ?


Sorry, all I got was a message from them saying they are talking to 
their

internal legal people about getting the licensing right, and they would
ping me when something happened - and nothing since then. I don't see 
any

reason not to apply Timo's encoder. The nvidia people can hopefully
collaborate on that to add functionality.

There has been some technical feedback from me, and others, which Timo
hasn't all responded to, but I'm ok with dealing with that after 
committing

the initial implementation.

Thanks,

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Nicolas George
Le septidi 7 frimaire, an CCXXIII, Timo Rothenpieler a écrit :
 Done that primarily to keep things cleaned up and easier to read.
 Can as well put it all in one huge file.

IMHO, your choice in the end.

 Propably, will split that out when i get to it.

Thanks.

 Most of this code is ported from a C++ Project where everything had
 to be camel case, so those and the C++ malloc casts are some of the
 leftovers.

Idem, IMHO your choice.

 The maximum is the number of adjacent B Frames plus one. So 3 in the
 case of NVENC, unless they change the supported gop patterns.

In that case, I suspect to take some margin and use a hardcoded size for the
queue: with 8 or 16, there is plenty of time to update the size if nvidia
releases new GOP patterns.

 I basically need a fifo like structure, where i can queue output
 surfaces until NVENC is done filling them.
 An array doesn't realy reflect that usage, as new elements are added
 to the front, and taken from the back.

An array used as a circular buffer is perfect for that.

 Is a simple assert on the return value enough? Can't continue in a
 sane way anyway if it ever fails.

No: an assert can only be used when the error is not possible except for a
bug in the code. A malloc failure is always possible, so a proper error
handling, eventually returning ENOMEM, is necessary. That is annoying but
mostly unavoidable.

 I renamed it to enqueue/dequeue.

Thanks.

 This definitely can't be replaced, its purpose isn't just a plain
 list, but sorting of the input timestamps, so the dts is still
 monotonic after re-ordering for B frames.

As far as I can see, you do not need the sorting random access to the sorted
timestamps but only to be able to extract the lowest one. This is precisely
what a heap is very good at. I assure you, in this case a heap will be
almost as simple to implement (certainly simpler if you make it fixed-size)
and way more efficient.

 Only adding the CUDA error code, which then has to be looked up manualy.

Too bad ;(

 What do you mean by that? Printing which presets are available in
 the error message?

Yes. Something like that is always nice:

# x264 [error]: invalid preset 'list'
# [libx264 @ 0x172a560] Error setting preset/tune list/(null).
# [libx264 @ 0x172a560] Possible presets: ultrafast superfast veryfast faster
# fast medium slow slower veryslow placebo
# [libx264 @ 0x172a560] Possible tunes: film animation grain stillimage psnr
# ssim fastdecode zerolatency

 Not entirely sure why i did that this way. Copied it straight from
 the libx264 encoder, without thinking too much about it. I can just
 set the profileGUID straight from that switch and can remove the
 second profile variable(which the libx264 encoder has in exactly the
 same conflicting way) entirely.

It is entirely possible that libx264 has reasons that I do not know to do
things that way. It is also possible that the options are there just for
historic reasons.

 +default:
 +avctx-coded_frame-pict_type = AV_PICTURE_TYPE_NONE;
 Not that I'm aware of. But i don't know what else to assume in this case.

Then I believe some kind of feedback would be needed. If it is really never
supposed to happen, an assert is acceptable; otherwise, an error requesting
a sample may be better.

 The maximum supported number of surfaces is allocated, if it'd ever
 run out, there'd be a bug in the code managing the surfaces.

Then an assert is exactly what is needed.

 ff_cuCtxCreate is a library function loaded from the CUDA dll/so.
 Same for all the other ff_cu* functions, there is no way to change
 what it returns, as it's not my function.

I missed that, sorry.

 +#define ifav_log(...) if (avctx) { av_log(__VA_ARGS__); }
 Looks strange: why no error message when there is no context?
 Is it possible to call av_log without a context?

Yes, it just prints the message without the [libfoo @ 0x42] prefix.

 No, does it need to be? Can multiple threads create the coded at the
 same time?

It looks like it is called from encode_init, which can definitely be called
by multiple threads.

Thanks for your efforts. Hope this helps.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Michael Niedermayer
On Sat, Nov 29, 2014 at 12:13:18PM +0100, Timo Rothenpieler wrote:
  I've spent a lot more time looking at the nvidia patch, but from a
  quick look through Timo's version, I'd say the following:
  
  * Timo's is more concise but not as feature complete.
  * nvidia one has windows support
  * The nvidia patch doesn't handle b-frames correctly, but I wrote a
fix patch for them. I'm not sure whether Timo's works correctly
  * Timo's looks like it will handle interlaced input correctly. Nvidia
definitely does not.
  * nvidia one implements argument compatibility with x264 - so it uses
the same args as much as possible - it even does preset/tune mapping.
I think this is pretty nice.
  
  The main issue with the nvidia patch, as it exists today, is that they
  have not put any licence header on the files at all - but I've told
  them they need to do that, and asked Stephen Warren if he can help them
  out. The other slight complexity is that it requires cuda.h (Timo seems
  to have avoided that by independently defining the necessary constants
  but you need even more of cuda.h for the windows support). But
  nvEncoderAPI.h is already so awkward (restrictive license, not properly
  distributed) that an extra header isn't any more inconvenient.
 
 My implementation also works fine on Windows. (Tested is with MSVC,
 MinGW and Cygwin)
 The required functions from CUDA are minimal, and i just dynamicaly load
 them, avoiding any (linked) external dependencies except the nvEncodeAPI.h
 
 The main difference is that they support way more features, but they
 seem to need an intermediate library, while mine is now just a single
 new C file, which needs one header from the nvenc SDK.

does supporting these additional features need the extra complexity
that the nvidia version has ?
or could these features be added into your version while keeping its
simplicity ? (note do not copy any code from the nvidia one as its
not redistriutable nor *GPL compatible with the current license
headers)

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Philip Langdale
On 29 Nov 2014 03:13, Timo Rothenpieler t...@rothenpieler.org wrote:

  I've spent a lot more time looking at the nvidia patch, but from a 
  quick look through Timo's version, I'd say the following: 
  
  * Timo's is more concise but not as feature complete. 
  * nvidia one has windows support 
  * The nvidia patch doesn't handle b-frames correctly, but I wrote a 
    fix patch for them. I'm not sure whether Timo's works correctly 
  * Timo's looks like it will handle interlaced input correctly. Nvidia 
    definitely does not. 
  * nvidia one implements argument compatibility with x264 - so it uses 
    the same args as much as possible - it even does preset/tune mapping. 
    I think this is pretty nice. 
  
  The main issue with the nvidia patch, as it exists today, is that they 
  have not put any licence header on the files at all - but I've told 
  them they need to do that, and asked Stephen Warren if he can help them 
  out. The other slight complexity is that it requires cuda.h (Timo seems 
  to have avoided that by independently defining the necessary constants 
  but you need even more of cuda.h for the windows support). But 
  nvEncoderAPI.h is already so awkward (restrictive license, not properly 
  distributed) that an extra header isn't any more inconvenient. 

 My implementation also works fine on Windows. (Tested is with MSVC, 
 MinGW and Cygwin) 
 The required functions from CUDA are minimal, and i just dynamicaly load 
 them, avoiding any (linked) external dependencies except the nvEncodeAPI.h 

 The main difference is that they support way more features, but they 
 seem to need an intermediate library, while mine is now just a single 
 new C file, which needs one header from the nvenc SDK. 

Fair enough. They have a set of nvenc aware files that
expose an api to the ffmpeg aware encoder. I think they
did this for licensing reasons but it's pointless and I'd
recommend they drop the split. It's pointless because all
those files are inside the ffmpeg patch anyway.

Once they get back to work, hopefully they can get
involved here and you can all work together. 

BTW, can you verify that interlaced encoding works for 
you? I get initialization errors back from the hardware. 



 ___ 
 ffmpeg-devel mailing list 
 ffmpeg-devel@ffmpeg.org 
 http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Timo Rothenpieler
 BTW, can you verify that interlaced encoding works for 
 you? I get initialization errors back from the hardware. 
 

Doesn't work for me either. It looks like it's not (yet?) supported and
just available in the API.




signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Timo Rothenpieler
 does supporting these additional features need the extra complexity
 that the nvidia version has ?
 or could these features be added into your version while keeping its
 simplicity ? (note do not copy any code from the nvidia one as its
 not redistriutable nor *GPL compatible with the current license
 headers)

I haven't even looked at their code for quite exactly that reason.
The primary problem with NVENC is, that there is only very poor
documentation available, so I can only implement stuff based on reading
the header and experimenting with the API and the available examples.
NVIDIA employees clearly are in a better position for this, as they most
likely have a much larger insight into how NVENC works internaly.
I'm quite sure all of their features could be added to my version.




signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Philip Langdale
On Sat, 29 Nov 2014 19:00:02 +0100
Timo Rothenpieler t...@rothenpieler.org wrote:

  does supporting these additional features need the extra complexity
  that the nvidia version has ?
  or could these features be added into your version while keeping its
  simplicity ? (note do not copy any code from the nvidia one as its
  not redistriutable nor *GPL compatible with the current license
  headers)
 
 I haven't even looked at their code for quite exactly that reason.
 The primary problem with NVENC is, that there is only very poor
 documentation available, so I can only implement stuff based on
 reading the header and experimenting with the API and the available
 examples. NVIDIA employees clearly are in a better position for this,
 as they most likely have a much larger insight into how NVENC works
 internaly. I'm quite sure all of their features could be added to my
 version.

I'm already 'polluted' until they correct their licensing, but I can at
least describe what they have.

The main piece of complexity in their implementation is they have an
additional abstraction layer between the ffmpeg encoder implementation
and the nvenc API. So this layer 'insulates' the two APIs. From a
licensing point of view it's meaningless, but I think that's the reason
why it exists, based on a brief email exchange with them. If that was
squashed, it would make theirs more straight forward. There's also a
bunch of utility code that handles argument mapping between x264 args
and nvenc args. As I've written elsewhere - I think this is a very
useful thing to do, as people are, broadly, familiar with how to tell
x264 to do stuff.

And as I mentioned before, they have a directx linked initialisation
path as an alternative to the cuda one. I'm not sure what conditions
that would be more useful under, but it's documented in the SDK.

Hopefully, they will respond positively on Monday and will get engaged
here, and sort out their licensing so that we can all work on a single
implementation.

Thanks!

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Timo Rothenpieler
Did some refactoring, now using a dynamic ring-buffer for both the
surface lists as well as the timestamp list.

There should be no thread safety problem anymore, as there are no
non-constant static global variables anymore.

I think i addressed most of the issues now, new patch is attached.
commit f7840ac3e9ae279e6665986a122e95b8bf9a7f6a
Author: Timo Rothenpieler t...@rothenpieler.org
Date:   Wed Nov 26 11:08:11 2014 +0100

Add NVENC encoder

diff --git a/Changelog b/Changelog
index 7172d0c..d26b7fa 100644
--- a/Changelog
+++ b/Changelog
@@ -17,6 +17,7 @@ version next:
 - WebP muxer with animated WebP support
 - zygoaudio decoding support
 - APNG demuxer
+- nvenc encoder
 
 
 version 2.4:
diff --git a/MAINTAINERS b/MAINTAINERS
index 15b976f..f2ff47d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -226,6 +226,7 @@ Codecs:
   msvideo1.cMike Melanson
   nellymoserdec.c   Benjamin Larsson
   nuv.c Reimar Doeffinger
+  nvenc.c   Timo Rothenpieler
   paf.* Paul B Mahol
   pcx.c Ivo van Poorten
   pgssubdec.c   Reimar Doeffinger
diff --git a/configure b/configure
index d4a86c0..1cafe4b 100755
--- a/configure
+++ b/configure
@@ -261,6 +261,7 @@ External library support:
   --enable-libzvbi enable teletext support via libzvbi [no]
   --disable-lzma   disable lzma [autodetect]
   --enable-decklinkenable Blackmagick DeckLink I/O support [no]
+  --enable-nvenc   enable NVIDIA NVENC support [no]
   --enable-openal  enable OpenAL 1.1 capture support [no]
   --enable-opencl  enable OpenCL code
   --enable-opengl  enable OpenGL rendering [no]
@@ -1393,6 +1394,7 @@ EXTERNAL_LIBRARY_LIST=
 libzmq
 libzvbi
 lzma
+nvenc
 openal
 opencl
 opengl
@@ -2390,6 +2392,7 @@ libxvid_encoder_deps=libxvid
 libutvideo_decoder_deps=libutvideo
 libutvideo_encoder_deps=libutvideo
 libzvbi_teletext_decoder_deps=libzvbi
+nvenc_encoder_deps=nvenc
 
 # demuxers / muxers
 ac3_demuxer_select=ac3_parser
@@ -4342,6 +4345,7 @@ die_license_disabled gpl x11grab
 
 die_license_disabled nonfree libaacplus
 die_license_disabled nonfree libfaac
+die_license_disabled nonfree nvenc
 enabled gpl  die_license_disabled_gpl nonfree libfdk_aac
 enabled gpl  die_license_disabled_gpl nonfree openssl
 
@@ -4651,6 +4655,7 @@ fi
 frei0r_filter_extralibs='$ldl'
 frei0r_src_filter_extralibs='$ldl'
 ladspa_filter_extralibs='$ldl'
+nvenc_encoder_extralibs='$ldl'
 
 if ! disabled network; then
 check_func getaddrinfo $network_extralibs
@@ -4916,6 +4921,7 @@ enabled libxavsrequire libxavs xavs.h 
xavs_encoder_encode -lxavs
 enabled libxvidrequire libxvid xvid.h xvid_global -lxvidcore
 enabled libzmq require_pkg_config libzmq zmq.h zmq_ctx_new
 enabled libzvbirequire libzvbi libzvbi.h vbi_decoder_new -lzvbi
+enabled nvenc  { check_header nvEncodeAPI.h || die ERROR: 
nvEncodeAPI.h not found.; }
 enabled openal { { for al_libs in ${OPENAL_LIBS} -lopenal 
-lOpenAL32; do
check_lib 'AL/al.h' alGetError ${al_libs}  
break; done } ||
die ERROR: openal not found; } 
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index fa0f53d..cc393f9 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -347,6 +347,7 @@ OBJS-$(CONFIG_MXPEG_DECODER)   += mxpegdec.o
 OBJS-$(CONFIG_NELLYMOSER_DECODER)  += nellymoserdec.o nellymoser.o
 OBJS-$(CONFIG_NELLYMOSER_ENCODER)  += nellymoserenc.o nellymoser.o
 OBJS-$(CONFIG_NUV_DECODER) += nuv.o rtjpeg.o
+OBJS-$(CONFIG_NVENC_ENCODER)   += nvenc.o
 OBJS-$(CONFIG_ON2AVC_DECODER)  += on2avc.o on2avcdata.o
 OBJS-$(CONFIG_OPUS_DECODER)+= opusdec.o opus.o opus_celt.o \
   opus_imdct.o opus_silk.o \
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 0d39d33..8ceee2f 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -223,6 +223,7 @@ void avcodec_register_all(void)
 REGISTER_DECODER(MVC2,  mvc2);
 REGISTER_DECODER(MXPEG, mxpeg);
 REGISTER_DECODER(NUV,   nuv);
+REGISTER_ENCODER(NVENC, nvenc);
 REGISTER_DECODER(PAF_VIDEO, paf_video);
 REGISTER_ENCDEC (PAM,   pam);
 REGISTER_ENCDEC (PBM,   pbm);
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
new file mode 100644
index 000..dc14594
--- /dev/null
+++ b/libavcodec/nvenc.c
@@ -0,0 +1,1189 @@
+/*
+ * H.264 hardware encoding using nvidia nvenc
+ * Copyright (c) 2014 Timo Rothenpieler t...@rothenpieler.org
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU 

Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Philip Langdale
On Sun, 30 Nov 2014 00:04:37 +0100
Timo Rothenpieler t...@rothenpieler.org wrote:

 Did some refactoring, now using a dynamic ring-buffer for both the
 surface lists as well as the timestamp list.
 
 There should be no thread safety problem anymore, as there are no
 non-constant static global variables anymore.
 
 I think i addressed most of the issues now, new patch is attached.

I agree with Nicholas that the dynamically allocated buffer list is
overkill. You can just allocate a fixed length 20 entry array and use
circular indexing (16 bframes + 4 extra as claimed by API docs).

Speaking of bframes, you're not allowing them to be selected as flexibly
as the hardware allows. According to docs, and reading the nvidia
patch, you should read the number of bframes requested by the user
(-bf) and set frameIntervalP equal to that number + 1 (so 17 in the
pathological case). From my limited testing, you really can request an
arbitrary number of bframes up to 16 and it will encode them. But
you're not respecting that config option.

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-29 Thread Philip Langdale
On Sat, 29 Nov 2014 15:52:00 -0800
Philip Langdale phil...@overt.org wrote:

 On Sun, 30 Nov 2014 00:04:37 +0100
 Timo Rothenpieler t...@rothenpieler.org wrote:
 
  Did some refactoring, now using a dynamic ring-buffer for both the
  surface lists as well as the timestamp list.
  
  There should be no thread safety problem anymore, as there are no
  non-constant static global variables anymore.
  
  I think i addressed most of the issues now, new patch is attached.
 
 I agree with Nicholas that the dynamically allocated buffer list is
 overkill. You can just allocate a fixed length 20 entry array and use
 circular indexing (16 bframes + 4 extra as claimed by API docs).
 
 Speaking of bframes, you're not allowing them to be selected as
 flexibly as the hardware allows. According to docs, and reading the
 nvidia patch, you should read the number of bframes requested by the
 user (-bf) and set frameIntervalP equal to that number + 1 (so 17 in
 the pathological case). From my limited testing, you really can
 request an arbitrary number of bframes up to 16 and it will encode
 them. But you're not respecting that config option.

Oh, and did you check your non-square pixel aspect ratio handling?
You're setting the dar Width/Height to the same as the pixel
width/height. The nvidia patch is the same and it produces wrong
results for non-square content (like DVDs). You need to scale by
the sample aspect ratio, which makes sense, but I also needed to
apply a 1.02 scale factor to the height. That's insane but it
definitely produces slightly incorrect results without it, and correct
results with it. Test for yourself.

darWidth = 1024 *
   avctx-width *
   avctx-sample_aspect_ratio.num /
   avctx-sample_aspect_ratio.den;
darHeight = 1024 * avctx-height * 1.02;

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-28 Thread Philip Langdale
On Wed, 26 Nov 2014 11:21:26 +0100
Timo Rothenpieler t...@rothenpieler.org wrote:

 This patch adds support for encoding with Nvidia NVENC on Windows and
 Linux.
 
 I'm not sure if this needs to be flagged as nonfree. As far as I'm 
 aware, it should not affect how the resulting binaries can be
 redistributed.
 
 The only dependency this has is the nvEncodeAPI.h from the NVENC SDK, 
 which can be downloaded from Nvidia:
 
 https://developer.nvidia.com/nvidia-video-codec-sdk
 
 The header is somewhat hidden in the Samples directory.
 
 I attached the current version of the patch.
 It can also be found on my github fork:
 
 https://github.com/BtbN/FFmpeg/tree/nvenc

For what it's worth, some engineers at Nvidia are also working on an
nvenc implementation for ffmpeg. That initial work is here:

https://github.com/agathah/ffmpeg_libnvenc

I've been corresponding with them about the lack of correct license
headers but there's not much going on given the thanksgiving holiday in
the US.

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-28 Thread Michael Niedermayer
On Fri, Nov 28, 2014 at 10:30:24AM -0800, Philip Langdale wrote:
 On Wed, 26 Nov 2014 11:21:26 +0100
 Timo Rothenpieler t...@rothenpieler.org wrote:
 
  This patch adds support for encoding with Nvidia NVENC on Windows and
  Linux.
  
  I'm not sure if this needs to be flagged as nonfree. As far as I'm 
  aware, it should not affect how the resulting binaries can be
  redistributed.
  
  The only dependency this has is the nvEncodeAPI.h from the NVENC SDK, 
  which can be downloaded from Nvidia:
  
  https://developer.nvidia.com/nvidia-video-codec-sdk
  
  The header is somewhat hidden in the Samples directory.
  
  I attached the current version of the patch.
  It can also be found on my github fork:
  
  https://github.com/BtbN/FFmpeg/tree/nvenc
 
 For what it's worth, some engineers at Nvidia are also working on an
 nvenc implementation for ffmpeg. That initial work is here:
 
 https://github.com/agathah/ffmpeg_libnvenc

whats the difference in features between the 2 implementations ?

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-28 Thread Philip Langdale
On Sat, 29 Nov 2014 00:51:21 +0100
Michael Niedermayer michae...@gmx.at wrote:

  
  For what it's worth, some engineers at Nvidia are also working on an
  nvenc implementation for ffmpeg. That initial work is here:
  
  https://github.com/agathah/ffmpeg_libnvenc
 
 whats the difference in features between the 2 implementations ?
 

I've spent a lot more time looking at the nvidia patch, but from a
quick look through Timo's version, I'd say the following:

* Timo's is more concise but not as feature complete.
* nvidia one has windows support
* The nvidia patch doesn't handle b-frames correctly, but I wrote a
  fix patch for them. I'm not sure whether Timo's works correctly
* Timo's looks like it will handle interlaced input correctly. Nvidia
  definitely does not.
* nvidia one implements argument compatibility with x264 - so it uses
  the same args as much as possible - it even does preset/tune mapping.
  I think this is pretty nice.

The main issue with the nvidia patch, as it exists today, is that they
have not put any licence header on the files at all - but I've told
them they need to do that, and asked Stephen Warren if he can help them
out. The other slight complexity is that it requires cuda.h (Timo seems
to have avoided that by independently defining the necessary constants
but you need even more of cuda.h for the windows support). But
nvEncoderAPI.h is already so awkward (restrictive license, not properly
distributed) that an extra header isn't any more inconvenient.

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-27 Thread Timo Rothenpieler

I haven't looked in detail, but this doesn't seem like a good idea to me.
For example it can't handle having different encoders using different 
cards/hardware.
I don't know what other options there might be, maybe this is the best, but it 
seems not particularly good.


The only other option would be to assume it's always only NV12, which is 
true for all hardware currently in existence.
Unfortunately it needs a full CUDA and NVENC init to query the supported 
formats.




signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-27 Thread Jean-Baptiste Kempf
On 26 Nov, Nicolas George wrote :
 Le sextidi 6 frimaire, an CCXXIII, Hendrik Leppkes a écrit :
  Thats just wrong. There is not one line of proprietary/non-free code that
  gets included or linked in libavcodec.
 
 (IANAL)
 
 I do not think it works that way. You could apply the same reasoning when
 linking a proprietary software with a GPL shared library and deduce that it
 is ok to distribute the resulting binary: this is not the usual doctrine.

Unfortunately, I have to agree with Nicolas.

This is the reason why the MingW and Wine projects reimplemented all the
headers based on the MSDN documentation. Because using non-free headers
is very debatable and usually considered as not ok by US lawyers (and
nVidia is a US company)

  Additionally, the API in use is part of the GPU driver, which I would
  strongly consider to be a system library (if a driver is not system, what
  is?), and as such falls under the system library exemption in any case.
 
 This one looks valid to me. It only covers the GPL side of the question
 though.

Yes, but not for the header.

With my kindest regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Carl Eugen Hoyos
Timo Rothenpieler timo at rothenpieler.org writes:

 I'm not sure if this needs to be flagged as nonfree.

Please mark it as non-free, the header clearly says 
that it must not be used for open-source software.

Please use tools/patcheck on your patchfile: It shows 
many issues most of which you should be able to fix 
quickly, skip the ones that make no sense to you, the 
tool is not error-free.
(Tabs and trailing whitespace cannot be committed.)

Thank you, Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Hendrik Leppkes
On Wed, Nov 26, 2014 at 11:38 AM, Carl Eugen Hoyos ceho...@ag.or.at wrote:

 Timo Rothenpieler timo at rothenpieler.org writes:

  I'm not sure if this needs to be flagged as nonfree.

 Please mark it as non-free, the header clearly says
 that it must not be used for open-source software


Where does it say that?

Its just a API header, using a certain API doesn't make your code non-free,
you are not including any code or other protected things in the
distribution.
Only the header itself is subject to the NVIDIA license, a compiled program
that uses the header, but doesn't distribute it, is not.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Timo Rothenpieler

Please mark it as non-free, the header clearly says
that it must not be used for open-source software.


It only says not to distribute the header, the compiled binary and the 
code using it should be free to distribute.



Please use tools/patcheck on your patchfile: It shows
many issues most of which you should be able to fix
quickly, skip the ones that make no sense to you, the
tool is not error-free.
(Tabs and trailing whitespace cannot be committed.)


Fixed now, new patch is attached and on github.

From 4239af3cf66cdaa9ad99386bf728af4c1d1aca8a Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler t...@rothenpieler.org
Date: Wed, 26 Nov 2014 11:08:11 +0100
Subject: [PATCH] Add NVENC encoder

---
 Changelog   |   1 +
 configure   |  11 +-
 libavcodec/Makefile |   1 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/nvenc.c  | 932 
 libavcodec/nvenc_api.c  | 275 ++
 libavcodec/nvenc_api.h  |  35 ++
 libavcodec/nvenc_cuda.h |  62 
 8 files changed, 1316 insertions(+), 2 deletions(-)
 create mode 100644 libavcodec/nvenc.c
 create mode 100644 libavcodec/nvenc_api.c
 create mode 100644 libavcodec/nvenc_api.h
 create mode 100644 libavcodec/nvenc_cuda.h

diff --git a/Changelog b/Changelog
index 7172d0c..d26b7fa 100644
--- a/Changelog
+++ b/Changelog
@@ -17,6 +17,7 @@ version next:
 - WebP muxer with animated WebP support
 - zygoaudio decoding support
 - APNG demuxer
+- nvenc encoder
 
 
 version 2.4:
diff --git a/configure b/configure
index 38619c4..d0b790c 100755
--- a/configure
+++ b/configure
@@ -261,6 +261,7 @@ External library support:
   --enable-libzvbi enable teletext support via libzvbi [no]
   --disable-lzma   disable lzma [autodetect]
   --enable-decklinkenable Blackmagick DeckLink I/O support [no]
+  --enable-nvenc   enable NVIDIA NVENC support [no]
   --enable-openal  enable OpenAL 1.1 capture support [no]
   --enable-opencl  enable OpenCL code
   --enable-opengl  enable OpenGL rendering [no]
@@ -1393,6 +1394,7 @@ EXTERNAL_LIBRARY_LIST=
 libzmq
 libzvbi
 lzma
+nvenc
 openal
 opencl
 opengl
@@ -2389,6 +2391,7 @@ libxvid_encoder_deps=libxvid
 libutvideo_decoder_deps=libutvideo
 libutvideo_encoder_deps=libutvideo
 libzvbi_teletext_decoder_deps=libzvbi
+nvenc_encoder_deps=nvenc
 
 # demuxers / muxers
 ac3_demuxer_select=ac3_parser
@@ -2569,9 +2572,7 @@ drawtext_filter_deps=libfreetype
 ebur128_filter_deps=gpl
 flite_filter_deps=libflite
 frei0r_filter_deps=frei0r dlopen
-frei0r_filter_extralibs='$ldl'
 frei0r_src_filter_deps=frei0r dlopen
-frei0r_src_filter_extralibs='$ldl'
 geq_filter_deps=gpl
 histeq_filter_deps=gpl
 hqdn3d_filter_deps=gpl
@@ -4650,6 +4651,11 @@ elif check_func dlopen -ldl; then
 ldl=-ldl
 fi
 
+# set a few flags which depend on ldl and can't be set earlier
+nvenc_encoder_extralibs='$ldl'
+frei0r_filter_extralibs='$ldl'
+frei0r_src_filter_extralibs='$ldl'
+
 if ! disabled network; then
 check_func getaddrinfo $network_extralibs
 check_func getservbyport $network_extralibs
@@ -4913,6 +4919,7 @@ enabled libxavsrequire libxavs xavs.h xavs_encoder_encode -lxavs
 enabled libxvidrequire libxvid xvid.h xvid_global -lxvidcore
 enabled libzmq require_pkg_config libzmq zmq.h zmq_ctx_new
 enabled libzvbirequire libzvbi libzvbi.h vbi_decoder_new -lzvbi
+enabled nvenc  { check_header nvEncodeAPI.h || die ERROR: nvEncodeAPI.h not found.; }
 enabled openal { { for al_libs in ${OPENAL_LIBS} -lopenal -lOpenAL32; do
check_lib 'AL/al.h' alGetError ${al_libs}  break; done } ||
die ERROR: openal not found; } 
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index fa0f53d..cc41564 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -347,6 +347,7 @@ OBJS-$(CONFIG_MXPEG_DECODER)   += mxpegdec.o
 OBJS-$(CONFIG_NELLYMOSER_DECODER)  += nellymoserdec.o nellymoser.o
 OBJS-$(CONFIG_NELLYMOSER_ENCODER)  += nellymoserenc.o nellymoser.o
 OBJS-$(CONFIG_NUV_DECODER) += nuv.o rtjpeg.o
+OBJS-$(CONFIG_NVENC_ENCODER)   += nvenc.o nvenc_api.o
 OBJS-$(CONFIG_ON2AVC_DECODER)  += on2avc.o on2avcdata.o
 OBJS-$(CONFIG_OPUS_DECODER)+= opusdec.o opus.o opus_celt.o \
   opus_imdct.o opus_silk.o \
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 0d39d33..8ceee2f 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -223,6 +223,7 @@ void avcodec_register_all(void)
 REGISTER_DECODER(MVC2,  mvc2);
 REGISTER_DECODER(MXPEG, mxpeg);
 REGISTER_DECODER(NUV,   nuv);
+REGISTER_ENCODER(NVENC, nvenc);
 REGISTER_DECODER(PAF_VIDEO, paf_video);
 REGISTER_ENCDEC (PAM,   pam);
 

Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Carl Eugen Hoyos
Timo Rothenpieler timo at rothenpieler.org writes:

  Please mark it as non-free, the header clearly says
  that it must not be used for open-source software.
 
 It only says not to distribute the header, the 
 compiled binary and the code using it should be free 
 to distribute.

I cannot read it like this but I am not a native 
speaker.
Maybe you could mark it as non-free until the FSF 
tells us their interpretation?

  Please use tools/patcheck on your patchfile

 Fixed now, new patch is attached and on github.

Thank you.

ff_nvenc_encoder is missing pix_fmts afaict but 
consider waiting for a real review.

Carl Eugen

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Timo Rothenpieler

I cannot read it like this but I am not a native
speaker.
Maybe you could mark it as non-free until the FSF
tells us their interpretation?


Allright, new patch is attached.


ff_nvenc_encoder is missing pix_fmts afaict but
consider waiting for a real review.


Yes, that's intended.
It uses init_static_data to dynamicaly ask the nvidia driver for the 
supported pixel formats instead.
From 793271822a5f52c3aed876fcedc7c6d8edd3c10c Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler t...@rothenpieler.org
Date: Wed, 26 Nov 2014 11:08:11 +0100
Subject: [PATCH] Add NVENC encoder

---
 Changelog   |   1 +
 configure   |  12 +-
 libavcodec/Makefile |   1 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/nvenc.c  | 932 
 libavcodec/nvenc_api.c  | 275 ++
 libavcodec/nvenc_api.h  |  35 ++
 libavcodec/nvenc_cuda.h |  62 
 8 files changed, 1317 insertions(+), 2 deletions(-)
 create mode 100644 libavcodec/nvenc.c
 create mode 100644 libavcodec/nvenc_api.c
 create mode 100644 libavcodec/nvenc_api.h
 create mode 100644 libavcodec/nvenc_cuda.h

diff --git a/Changelog b/Changelog
index 7172d0c..d26b7fa 100644
--- a/Changelog
+++ b/Changelog
@@ -17,6 +17,7 @@ version next:
 - WebP muxer with animated WebP support
 - zygoaudio decoding support
 - APNG demuxer
+- nvenc encoder
 
 
 version 2.4:
diff --git a/configure b/configure
index 38619c4..05bce5d 100755
--- a/configure
+++ b/configure
@@ -261,6 +261,7 @@ External library support:
   --enable-libzvbi enable teletext support via libzvbi [no]
   --disable-lzma   disable lzma [autodetect]
   --enable-decklinkenable Blackmagick DeckLink I/O support [no]
+  --enable-nvenc   enable NVIDIA NVENC support [no]
   --enable-openal  enable OpenAL 1.1 capture support [no]
   --enable-opencl  enable OpenCL code
   --enable-opengl  enable OpenGL rendering [no]
@@ -1393,6 +1394,7 @@ EXTERNAL_LIBRARY_LIST=
 libzmq
 libzvbi
 lzma
+nvenc
 openal
 opencl
 opengl
@@ -2389,6 +2391,7 @@ libxvid_encoder_deps=libxvid
 libutvideo_decoder_deps=libutvideo
 libutvideo_encoder_deps=libutvideo
 libzvbi_teletext_decoder_deps=libzvbi
+nvenc_encoder_deps=nvenc
 
 # demuxers / muxers
 ac3_demuxer_select=ac3_parser
@@ -2569,9 +2572,7 @@ drawtext_filter_deps=libfreetype
 ebur128_filter_deps=gpl
 flite_filter_deps=libflite
 frei0r_filter_deps=frei0r dlopen
-frei0r_filter_extralibs='$ldl'
 frei0r_src_filter_deps=frei0r dlopen
-frei0r_src_filter_extralibs='$ldl'
 geq_filter_deps=gpl
 histeq_filter_deps=gpl
 hqdn3d_filter_deps=gpl
@@ -4344,6 +4345,7 @@ die_license_disabled gpl x11grab
 
 die_license_disabled nonfree libaacplus
 die_license_disabled nonfree libfaac
+die_license_disabled nonfree nvenc
 enabled gpl  die_license_disabled_gpl nonfree libfdk_aac
 enabled gpl  die_license_disabled_gpl nonfree openssl
 
@@ -4650,6 +4652,11 @@ elif check_func dlopen -ldl; then
 ldl=-ldl
 fi
 
+# set a few flags which depend on ldl and can't be set earlier
+nvenc_encoder_extralibs='$ldl'
+frei0r_filter_extralibs='$ldl'
+frei0r_src_filter_extralibs='$ldl'
+
 if ! disabled network; then
 check_func getaddrinfo $network_extralibs
 check_func getservbyport $network_extralibs
@@ -4913,6 +4920,7 @@ enabled libxavsrequire libxavs xavs.h xavs_encoder_encode -lxavs
 enabled libxvidrequire libxvid xvid.h xvid_global -lxvidcore
 enabled libzmq require_pkg_config libzmq zmq.h zmq_ctx_new
 enabled libzvbirequire libzvbi libzvbi.h vbi_decoder_new -lzvbi
+enabled nvenc  { check_header nvEncodeAPI.h || die ERROR: nvEncodeAPI.h not found.; }
 enabled openal { { for al_libs in ${OPENAL_LIBS} -lopenal -lOpenAL32; do
check_lib 'AL/al.h' alGetError ${al_libs}  break; done } ||
die ERROR: openal not found; } 
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index fa0f53d..cc41564 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -347,6 +347,7 @@ OBJS-$(CONFIG_MXPEG_DECODER)   += mxpegdec.o
 OBJS-$(CONFIG_NELLYMOSER_DECODER)  += nellymoserdec.o nellymoser.o
 OBJS-$(CONFIG_NELLYMOSER_ENCODER)  += nellymoserenc.o nellymoser.o
 OBJS-$(CONFIG_NUV_DECODER) += nuv.o rtjpeg.o
+OBJS-$(CONFIG_NVENC_ENCODER)   += nvenc.o nvenc_api.o
 OBJS-$(CONFIG_ON2AVC_DECODER)  += on2avc.o on2avcdata.o
 OBJS-$(CONFIG_OPUS_DECODER)+= opusdec.o opus.o opus_celt.o \
   opus_imdct.o opus_silk.o \
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index 0d39d33..8ceee2f 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -223,6 +223,7 @@ void avcodec_register_all(void)
 REGISTER_DECODER(MVC2,  mvc2);
 REGISTER_DECODER(MXPEG, mxpeg);
 REGISTER_DECODER(NUV,   nuv);
+

Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Hendrik Leppkes
On Wed, Nov 26, 2014 at 2:35 PM, Carl Eugen Hoyos ceho...@ag.or.at wrote:

 Timo Rothenpieler timo at rothenpieler.org writes:

   Please mark it as non-free, the header clearly says
   that it must not be used for open-source software.
 
  It only says not to distribute the header, the
  compiled binary and the code using it should be free
  to distribute.

 I cannot read it like this but I am not a native
 speaker.
 Maybe you could mark it as non-free until the FSF
 tells us their interpretation?


Thats just wrong. There is not one line of proprietary/non-free code that
gets included or linked in libavcodec.
Additionally, the API in use is part of the GPU driver, which I would
strongly consider to be a system library (if a driver is not system, what
is?), and as such falls under the system library exemption in any case.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Nicolas George
Le sextidi 6 frimaire, an CCXXIII, Hendrik Leppkes a écrit :
 Thats just wrong. There is not one line of proprietary/non-free code that
 gets included or linked in libavcodec.

(IANAL)

I do not think it works that way. You could apply the same reasoning when
linking a proprietary software with a GPL shared library and deduce that it
is ok to distribute the resulting binary: this is not the usual doctrine.

For things to make sense, I believe you have to consider that the assembly
code for a function call is a mix between the source code for the function
call and the function prototype; therefore, the license for the prototype
applies.

(For linking, just copying the SONAME from the library to the binary is
probably not enough to elicit copyright; I do not know what happens exactly
with symbols versioning.)

 Additionally, the API in use is part of the GPU driver, which I would
 strongly consider to be a system library (if a driver is not system, what
 is?), and as such falls under the system library exemption in any case.

This one looks valid to me. It only covers the GPL side of the question
though.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add NVENC encoder

2014-11-26 Thread Reimar Döffinger
On 26.11.2014, at 14:58, Timo Rothenpieler t...@rothenpieler.org wrote:
 I cannot read it like this but I am not a native
 speaker.
 Maybe you could mark it as non-free until the FSF
 tells us their interpretation?
 
 Allright, new patch is attached.
 
 ff_nvenc_encoder is missing pix_fmts afaict but
 consider waiting for a real review.
 
 Yes, that's intended.
 It uses init_static_data to dynamicaly ask the nvidia driver for the 
 supported pixel formats instead.

I haven't looked in detail, but this doesn't seem like a good idea to me.
For example it can't handle having different encoders using different 
cards/hardware.
I don't know what other options there might be, maybe this is the best, but it 
seems not particularly good.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel