On Sat, 29 Nov 2014 15:52:00 -0800 Philip Langdale <[email protected]> wrote:
> On Sun, 30 Nov 2014 00:04:37 +0100 > Timo Rothenpieler <[email protected]> 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 [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
