[FFmpeg-devel] [PATCH] Add NVENC encoder

[Philip Langdale]

On Sat, 29 Nov 2014 15:52:00 -0800
Philip Langdale <philipl at overt.org> wrote:

> On Sun, 30 Nov 2014 00:04:37 +0100
> Timo Rothenpieler <timo at 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