diff mbox

[FFmpeg-devel] Remove qmin and qmax constraints for nvenc vbr

Message ID 1487900547839.27497@nvidia.com
State New
Headers show

Commit Message

Ganapathy Raman Kasi Feb. 24, 2017, 1:42 a.m. UTC 
Hi,


qmin and qmax are not necessary for nvenc vbr. Enforcing this constraint, doesn't allow user to use vbr 2 pass mode (

NV_ENC_PARAMS_RC_2_PASS_VBR) without explicity setting the qmin and qmax options and reverts to the default vbr mode (

NV_ENC_PARAMS_RC_VBR). If needed user can still set qmin and qmax and those will be obeyed. Please review. Thanks.


Regards

Ganapathy Kasi

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Comments

Ganapathy Raman Kasi Feb. 24, 2017, 2:01 a.m. UTC | #1 
Adding on to the previous mail,
To test this patch, please try :

i) ffmpeg -y -i $INPUT -vcodec h264_nvenc -preset medium $OUT_medium.mp4
ii) ffmpeg -y -i $INPUT -vcodec h264_nvenc -preset medium $OUT_slow.mp4

Without patch : $OUT_medium and $OUT_slow will be binary identical since both use NV_ENC_PARAMS_RC_VBR
With patch : $OUT_medium and $OUT_slow will be different and ii) will take more time to run and usually results in better quality. Thanks

Ganapathy


-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Ganapathy Raman Kasi
Sent: Thursday, February 23, 2017 5:42 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: [FFmpeg-devel] [PATCH] Remove qmin and qmax constraints for nvenc vbr

Hi,


qmin and qmax are not necessary for nvenc vbr. Enforcing this constraint, doesn't allow user to use vbr 2 pass mode (

NV_ENC_PARAMS_RC_2_PASS_VBR) without explicity setting the qmin and qmax options and reverts to the default vbr mode (

NV_ENC_PARAMS_RC_VBR). If needed user can still set qmin and qmax and those will be obeyed. Please review. Thanks.


Regards

Ganapathy Kasi

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information.  Any unauthorized review, use, disclosure or distribution is prohibited.  If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
Ganapathy Raman Kasi Feb. 24, 2017, 2:02 a.m. UTC | #2 
Sorry, typo below:
Change -preset medium to -preset slow in ii)

Ganapathy

-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Ganapathy Raman Kasi

Sent: Thursday, February 23, 2017 6:01 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] Remove qmin and qmax constraints for nvenc vbr

Adding on to the previous mail,
To test this patch, please try :

i) ffmpeg -y -i $INPUT -vcodec h264_nvenc -preset medium $OUT_medium.mp4
ii) ffmpeg -y -i $INPUT -vcodec h264_nvenc -preset slow $OUT_slow.mp4

Without patch : $OUT_medium and $OUT_slow will be binary identical since both use NV_ENC_PARAMS_RC_VBR With patch : $OUT_medium and $OUT_slow will be different and ii) will take more time to run and usually results in better quality. Thanks

Ganapathy


-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Ganapathy Raman Kasi

Sent: Thursday, February 23, 2017 5:42 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: [FFmpeg-devel] [PATCH] Remove qmin and qmax constraints for nvenc vbr

Hi,


qmin and qmax are not necessary for nvenc vbr. Enforcing this constraint, doesn't allow user to use vbr 2 pass mode (

NV_ENC_PARAMS_RC_2_PASS_VBR) without explicity setting the qmin and qmax options and reverts to the default vbr mode (

NV_ENC_PARAMS_RC_VBR). If needed user can still set qmin and qmax and those will be obeyed. Please review. Thanks.


Regards

Ganapathy Kasi

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information.  Any unauthorized review, use, disclosure or distribution is prohibited.  If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Ganapathy Raman Kasi Feb. 28, 2017, 12:24 a.m. UTC | #3 
Can someone please help review patch. Thanks.

Ganapathy

-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Ganapathy Raman Kasi
Sent: Thursday, February 23, 2017 5:42 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: [FFmpeg-devel] [PATCH] Remove qmin and qmax constraints for nvenc vbr

Hi,


qmin and qmax are not necessary for nvenc vbr. Enforcing this constraint, doesn't allow user to use vbr 2 pass mode (

NV_ENC_PARAMS_RC_2_PASS_VBR) without explicity setting the qmin and qmax options and reverts to the default vbr mode (

NV_ENC_PARAMS_RC_VBR). If needed user can still set qmin and qmax and those will be obeyed. Please review. Thanks.


Regards

Ganapathy Kasi

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain confidential information.  Any unauthorized review, use, disclosure or distribution is prohibited.  If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
Timo Rothenpieler Feb. 28, 2017, 9:44 a.m. UTC | #4 
Am 28.02.2017 um 01:24 schrieb Ganapathy Raman Kasi:
> Can someone please help review patch. Thanks.

I have seen an marked it, but I don't have time to review and test stuff
right now.
Will get to it eventually.
Miroslav Slugeň Feb. 28, 2017, 10:54 a.m. UTC | #5 
Dne 28.2.2017 v 10:44 Timo Rothenpieler napsal(a):
> Am 28.02.2017 um 01:24 schrieb Ganapathy Raman Kasi:
>> Can someone please help review patch. Thanks.
> I have seen an marked it, but I don't have time to review and test stuff
> right now.
> Will get to it eventually.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
I tested this patch, it looks like there is no difference when you set 
-qmin 5 -qmax 51 or leave it unset.

BUT this patch is correct, NVENC doesn't require QMIN and QMAX for 2pass 
vbr.

M.
Philip Langdale Feb. 28, 2017, 1:53 p.m. UTC | #6 
On Tue, 28 Feb 2017 11:54:32 +0100
Miroslav Slugeň <thunder.m@email.cz> wrote:

> Dne 28.2.2017 v 10:44 Timo Rothenpieler napsal(a):
> > Am 28.02.2017 um 01:24 schrieb Ganapathy Raman Kasi:  
> >> Can someone please help review patch. Thanks.  
> > I have seen an marked it, but I don't have time to review and test
> > stuff right now.
> > Will get to it eventually.
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel  
> I tested this patch, it looks like there is no difference when you
> set -qmin 5 -qmax 51 or leave it unset.
> 
> BUT this patch is correct, NVENC doesn't require QMIN and QMAX for
> 2pass vbr.
> 
> M.
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Patch looks good to me too. Timo, I can push it if you don't mind not
testing it yourself.


--phil
Timo Rothenpieler Feb. 28, 2017, 3:23 p.m. UTC | #7 
> Patch looks good to me too. Timo, I can push it if you don't mind not
> testing it yourself.

I have some stilistic nits, otherwise it's ok.
It leaves the fall through comment dangling, even though there is no
fall through anymore.
And it could use fallthrough to in the new location of RC_(2_PASS_)VBR,
but that's clearly optional and I have no strong opinion on what's better.


> 
> --phil
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Ganapathy Raman Kasi March 1, 2017, 11:15 p.m. UTC | #8 
Thanks for review. If qmin and qmax are not set, nvenc will assume default of qmin 15 and qmax 51 which is good enough for most content. Setting qmin 5 would be similar to qmin of 15 at lower bitrates as ratecontrol may not  reach below QP 15. 

Ganapathy

-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Philip Langdale

Sent: Tuesday, February 28, 2017 5:54 AM
To: Miroslav Slugeň <thunder.m@email.cz>
Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] Remove qmin and qmax constraints for nvenc vbr

On Tue, 28 Feb 2017 11:54:32 +0100
Miroslav Slugeň <thunder.m@email.cz> wrote:

> Dne 28.2.2017 v 10:44 Timo Rothenpieler napsal(a):

> > Am 28.02.2017 um 01:24 schrieb Ganapathy Raman Kasi:  

> >> Can someone please help review patch. Thanks.  

> > I have seen an marked it, but I don't have time to review and test 

> > stuff right now.

> > Will get to it eventually.

> > _______________________________________________

> > ffmpeg-devel mailing list

> > ffmpeg-devel@ffmpeg.org

> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

> I tested this patch, it looks like there is no difference when you set 

> -qmin 5 -qmax 51 or leave it unset.

> 

> BUT this patch is correct, NVENC doesn't require QMIN and QMAX for 

> 2pass vbr.

> 

> M.

> 

> 

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Patch looks good to me too. Timo, I can push it if you don't mind not testing it yourself.


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

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
diff mbox

Patch

From 41f4541a1137f5a49da2ffb0013d2574f3ad46a0 Mon Sep 17 00:00:00 2001
From: Ganapathy Kasi <gkasi@nvidia.com>
Date: Thu, 23 Feb 2017 17:32:21 -0800
Subject: [PATCH] Remove qmin and qmax constraints for nvenc vbr

qmin and qmax are not necessary for nvenc vbr. Enforcing this constraint, doesn't allow user to use vbr 2 pass mode without explicity setting the qmin and qmax options
---
 libavcodec/nvenc.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index ba2647b..3c24bc5 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -589,15 +589,6 @@  static void nvenc_override_rate_control(AVCodecContext *avctx)
         }
         set_constqp(avctx);
         return;
-    case NV_ENC_PARAMS_RC_2_PASS_VBR:
-    case NV_ENC_PARAMS_RC_VBR:
-        if (avctx->qmin < 0 && avctx->qmax < 0) {
-            av_log(avctx, AV_LOG_WARNING,
-                   "The variable bitrate rate-control requires "
-                   "the 'qmin' and/or 'qmax' option set.\n");
-            set_vbr(avctx);
-            return;
-        }
         /* fall through */
     case NV_ENC_PARAMS_RC_VBR_MINQP:
         if (avctx->qmin < 0) {
@@ -609,6 +600,10 @@  static void nvenc_override_rate_control(AVCodecContext *avctx)
         }
         set_vbr(avctx);
         break;
+    case NV_ENC_PARAMS_RC_2_PASS_VBR:
+    case NV_ENC_PARAMS_RC_VBR:
+        set_vbr(avctx);
+        break;
     case NV_ENC_PARAMS_RC_CBR:
     case NV_ENC_PARAMS_RC_2_PASS_QUALITY:
     case NV_ENC_PARAMS_RC_2_PASS_FRAMESIZE_CAP:
-- 
2.7.4