[FFmpeg-devel] [PATCH 3/3] merge compute initialQP sections

2015-09-11 Thread Agatha Hu 
---
 libavcodec/nvenc.c |  101 ++--
 1 file changed, 42 insertions(+), 59 deletions(-)

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index fd90f7b..57aae33 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -800,69 +800,52 @@ static av_cold int nvenc_encode_init(AVCodecContext 
*avctx)
 
 avctx->qmin = -1;
 avctx->qmax = -1;
-} else if (avctx->qmin >= 0 && avctx->qmax >= 0) {
-if (ctx->twopass == 1) {
-ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_2_PASS_VBR;
-
-if (avctx->codec->id == AV_CODEC_ID_H264) {
-
ctx->encode_config.encodeCodecConfig.h264Config.adaptiveTransformMode = 
NV_ENC_H264_ADAPTIVE_TRANSFORM_ENABLE;
-ctx->encode_config.encodeCodecConfig.h264Config.fmoMode = 
NV_ENC_H264_FMO_DISABLE;
-}
-} else {
-ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_VBR_MINQP;
-}
-
-ctx->encode_config.rcParams.enableMinQP = 1;
-ctx->encode_config.rcParams.enableMaxQP = 1;
-
-ctx->encode_config.rcParams.minQP.qpInterB = avctx->qmin;
-ctx->encode_config.rcParams.minQP.qpInterP = avctx->qmin;
-ctx->encode_config.rcParams.minQP.qpIntra = avctx->qmin;
-
-ctx->encode_config.rcParams.maxQP.qpInterB = avctx->qmax;
-ctx->encode_config.rcParams.maxQP.qpInterP = avctx->qmax;
-ctx->encode_config.rcParams.maxQP.qpIntra = avctx->qmax;
-
-{
-uint32_t qpInterP = (avctx->qmax + 3*avctx->qmin)/4; // biased 
towards Qmin
-ctx->encode_config.rcParams.initialRCQP.qpInterP  = qpInterP;
-if(avctx->i_quant_factor != 0.0 && avctx->b_quant_factor != 0.0) {
-ctx->encode_config.rcParams.initialRCQP.qpIntra = qpInterP * 
fabs(avctx->i_quant_factor);
-ctx->encode_config.rcParams.initialRCQP.qpIntra += qpInterP * 
(avctx->i_quant_offset);
-ctx->encode_config.rcParams.initialRCQP.qpInterB = qpInterP * 
fabs(avctx->b_quant_factor);
-ctx->encode_config.rcParams.initialRCQP.qpInterB += qpInterP * 
(avctx->b_quant_offset);
-} else {
-ctx->encode_config.rcParams.initialRCQP.qpIntra = qpInterP;
-ctx->encode_config.rcParams.initialRCQP.qpInterB = qpInterP;
-}
-}
-ctx->encode_config.rcParams.enableInitialRCQP = 1;   
 } else {
-if (ctx->twopass < 1) {
-ctx->encode_config.rcParams.rateControlMode = NV_ENC_PARAMS_RC_VBR;
-} else {
-ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_2_PASS_VBR;
+uint32_t qpInterP = 26; // default to 26
+if (avctx->qmin >= 0 && avctx->qmax >= 0) { 
+ctx->encode_config.rcParams.enableMinQP = 1;
+ctx->encode_config.rcParams.enableMaxQP = 1;
+
+ctx->encode_config.rcParams.minQP.qpInterB = avctx->qmin;
+ctx->encode_config.rcParams.minQP.qpInterP = avctx->qmin;
+ctx->encode_config.rcParams.minQP.qpIntra = avctx->qmin;
+
+ctx->encode_config.rcParams.maxQP.qpInterB = avctx->qmax;
+ctx->encode_config.rcParams.maxQP.qpInterP = avctx->qmax;
+ctx->encode_config.rcParams.maxQP.qpIntra = avctx->qmax;
+qpInterP = (avctx->qmax + 3*avctx->qmin)/4; // biased towards Qmin
+
+if (ctx->twopass == 1) {
+ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_2_PASS_VBR;
+if (avctx->codec->id == AV_CODEC_ID_H264) {
+
ctx->encode_config.encodeCodecConfig.h264Config.adaptiveTransformMode = 
NV_ENC_H264_ADAPTIVE_TRANSFORM_ENABLE;
+ctx->encode_config.encodeCodecConfig.h264Config.fmoMode = 
NV_ENC_H264_FMO_DISABLE;
+}
+} else {
+ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_VBR_MINQP;
+} 
 }
-
-{
-uint32_t qpInterP = 26; // default to 26
-ctx->encode_config.rcParams.initialRCQP.qpInterP  = qpInterP;
-
-if(avctx->i_quant_factor != 0.0 && avctx->b_quant_factor != 0.0) {
-
-ctx->encode_config.rcParams.initialRCQP.qpIntra = qpInterP * 
fabs(avctx->i_quant_factor);
-ctx->encode_config.rcParams.initialRCQP.qpIntra += qpInterP * 
(avctx->i_quant_offset);
-
-ctx->encode_config.rcParams.initialRCQP.qpInterB = qpInterP * 
fabs(avctx->b_quant_factor);
-ctx->encode_config.rcParams.initialRCQP.qpInterB += qpInterP * 
(avctx->b_quant_offset);
+else {   
+if (ctx->twopass < 1) {
+ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_VBR;
 } else {
-ctx->encode_config.rcParams.initialRCQP.qpIntra = qpInterP;
-

Re: [FFmpeg-devel] [PATCH 3/3] merge compute initialQP sections

2015-09-11 Thread Carl Eugen Hoyos 
Agatha Hu  nvidia.com> writes:

> -ctx->encode_config.rcParams.enableMinQP = 1;
> -ctx->encode_config.rcParams.enableMaxQP = 1;

> +ctx->encode_config.rcParams.enableMinQP = 1;
> +ctx->encode_config.rcParams.enableMaxQP = 1;

If you do not reindent these lines in the same patch, 
the changes get much easier to read and review.

Carl Eugen

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

Re: [FFmpeg-devel] [PATCH 3/3] merge compute initialQP sections

2015-09-11 Thread Carl Eugen Hoyos 
On Friday 11 September 2015 02:32:32 pm Timo Rothenpieler wrote:
> >> -ctx->encode_config.rcParams.enableMinQP = 1;
> >> -ctx->encode_config.rcParams.enableMaxQP = 1;
> >>
> >> +ctx->encode_config.rcParams.enableMinQP = 1;
> >> +ctx->encode_config.rcParams.enableMaxQP = 1;
> >
> > If you do not reindent these lines in the same patch,
> > the changes get much easier to read and review.
>
> But the indentation level of those lines changed in this 
> patch, moving them around is the entire point of this patch.

Maybe you disagree but I find sttached much more readable.

You can commit a cosmetics-only patch as a follow-up.

Also please see the developer documentation (that for you as 
maintainer of the file is mostly a hint, not a requirement).

Carl Eugen

PS: Patch 2/3 contains trailing whitespace, this is why 
attached also contains it (it cannot be pushed to our git 
repository).
diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index fd90f7b..eadc96f 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -800,18 +800,9 @@ static av_cold int nvenc_encode_init(AVCodecContext *avctx)
 
 avctx->qmin = -1;
 avctx->qmax = -1;
-} else if (avctx->qmin >= 0 && avctx->qmax >= 0) {
-if (ctx->twopass == 1) {
-ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_2_PASS_VBR;
-
-if (avctx->codec->id == AV_CODEC_ID_H264) {
-
ctx->encode_config.encodeCodecConfig.h264Config.adaptiveTransformMode = 
NV_ENC_H264_ADAPTIVE_TRANSFORM_ENABLE;
-ctx->encode_config.encodeCodecConfig.h264Config.fmoMode = 
NV_ENC_H264_FMO_DISABLE;
-}
 } else {
-ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_VBR_MINQP;
-}
-
+uint32_t qpInterP = 26; // default to 26
+if (avctx->qmin >= 0 && avctx->qmax >= 0) { 
 ctx->encode_config.rcParams.enableMinQP = 1;
 ctx->encode_config.rcParams.enableMaxQP = 1;
 
@@ -822,30 +813,26 @@ static av_cold int nvenc_encode_init(AVCodecContext 
*avctx)
 ctx->encode_config.rcParams.maxQP.qpInterB = avctx->qmax;
 ctx->encode_config.rcParams.maxQP.qpInterP = avctx->qmax;
 ctx->encode_config.rcParams.maxQP.qpIntra = avctx->qmax;
+qpInterP = (avctx->qmax + 3*avctx->qmin)/4; // biased towards Qmin
 
-{
-uint32_t qpInterP = (avctx->qmax + 3*avctx->qmin)/4; // biased 
towards Qmin
-ctx->encode_config.rcParams.initialRCQP.qpInterP  = qpInterP;
-if(avctx->i_quant_factor != 0.0 && avctx->b_quant_factor != 0.0) {
-ctx->encode_config.rcParams.initialRCQP.qpIntra = qpInterP * 
fabs(avctx->i_quant_factor);
-ctx->encode_config.rcParams.initialRCQP.qpIntra += qpInterP * 
(avctx->i_quant_offset);
-ctx->encode_config.rcParams.initialRCQP.qpInterB = qpInterP * 
fabs(avctx->b_quant_factor);
-ctx->encode_config.rcParams.initialRCQP.qpInterB += qpInterP * 
(avctx->b_quant_offset);
-} else {
-ctx->encode_config.rcParams.initialRCQP.qpIntra = qpInterP;
-ctx->encode_config.rcParams.initialRCQP.qpInterB = qpInterP;
+if (ctx->twopass == 1) {
+ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_2_PASS_VBR;
+if (avctx->codec->id == AV_CODEC_ID_H264) {
+
ctx->encode_config.encodeCodecConfig.h264Config.adaptiveTransformMode = 
NV_ENC_H264_ADAPTIVE_TRANSFORM_ENABLE;
+ctx->encode_config.encodeCodecConfig.h264Config.fmoMode = 
NV_ENC_H264_FMO_DISABLE;
 }
+} else {
+ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_VBR_MINQP;
 } 
-ctx->encode_config.rcParams.enableInitialRCQP = 1;   
 } else {   
 if (ctx->twopass < 1) {
 ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_VBR;
 } else {
 ctx->encode_config.rcParams.rateControlMode = 
NV_ENC_PARAMS_RC_2_PASS_VBR;
 }   
+}
 
-{
-uint32_t qpInterP = 26; // default to 26
+ctx->encode_config.rcParams.enableInitialRCQP = 1;
 ctx->encode_config.rcParams.initialRCQP.qpInterP  = qpInterP;
 
 if(avctx->i_quant_factor != 0.0 && avctx->b_quant_factor != 0.0) {
@@ -860,8 +847,6 @@ static av_cold int nvenc_encode_init(AVCodecContext *avctx)
 ctx->encode_config.rcParams.initialRCQP.qpInterB = qpInterP;
 }  
 } 
-ctx->encode_config.rcParams.enableInitialRCQP = 1;
-}
 
 if (avctx->rc_buffer_size > 0)
 ctx->encode_config.rcParams.vbvBufferSize = avctx->rc_buffer_size;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH 3/3] merge compute initialQP sections

2015-09-11 Thread Timo Rothenpieler 

-ctx->encode_config.rcParams.enableMinQP = 1;
-ctx->encode_config.rcParams.enableMaxQP = 1;



+ctx->encode_config.rcParams.enableMinQP = 1;
+ctx->encode_config.rcParams.enableMaxQP = 1;


If you do not reindent these lines in the same patch,
the changes get much easier to read and review.

Carl Eugen


But the indentation level of those lines changed in this patch, moving 
them around is the entire point of this patch.


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