diff mbox

[FFmpeg-devel] lavc/vaapi_encode: fix p_per_i calculate issue.

Message ID 860effb6-e4bc-49bd-8654-361ffd6eb9c9@gmail.com
State Accepted
Commit 08087f54626780b6955e470d59b1c7eff6c57f72
Headers show

Commit Message

Jun Zhao March 29, 2017, 9:35 a.m. UTC
From 3afe863771fdb1ebf2316d2f6ae5ea2351e7cb4f Mon Sep 17 00:00:00 2001
From: Jun Zhao <jun.zhao@intel.com>
Date: Wed, 29 Mar 2017 17:18:59 +0800
Subject: [PATCH] lavc/vaapi_encode: fix p_per_i calculate issue.

now gop_size <= (max_b_frames + 1) * p_per_i + 1 (IDR frame),
so celing p_per_i = (gop_size - 1 + max_b_frames) / (max_b_frames + 1)

Signed-off-by: Jun Zhao <jun.zhao@intel.com>
Signed-off-by: Leilei <leilei.shang@intel.com>
---
 libavcodec/vaapi_encode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jun Zhao April 1, 2017, 12:08 a.m. UTC | #1
Ping ?

On 2017/3/29 17:35, Jun Zhao wrote:
Mark Thompson April 2, 2017, 10:46 p.m. UTC | #2
On 29/03/17 10:35, Jun Zhao wrote:
> From 3afe863771fdb1ebf2316d2f6ae5ea2351e7cb4f Mon Sep 17 00:00:00 2001
> From: Jun Zhao <jun.zhao@intel.com>
> Date: Wed, 29 Mar 2017 17:18:59 +0800
> Subject: [PATCH] lavc/vaapi_encode: fix p_per_i calculate issue.
> 
> now gop_size <= (max_b_frames + 1) * p_per_i + 1 (IDR frame),
> so celing p_per_i = (gop_size - 1 + max_b_frames) / (max_b_frames + 1)
> 
> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
> Signed-off-by: Leilei <leilei.shang@intel.com>
> ---
>  libavcodec/vaapi_encode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 070ff5f..7e9c00f 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1433,7 +1433,7 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>      ctx->output_order = - ctx->output_delay - 1;
>  
>      // Currently we never generate I frames, only IDR.
> -    ctx->p_per_i = ((avctx->gop_size + avctx->max_b_frames) /
> +    ctx->p_per_i = ((avctx->gop_size - 1 + avctx->max_b_frames) /
>                      (avctx->max_b_frames + 1));
>      ctx->b_per_p = avctx->max_b_frames;
>  
> -- 
> 2.9.3

Hmm, yep, right.  (After staring at edge cases for a while - the error for being too small is far worse than for being too large.)

Applied for correctness, though I'm not sure it will have much effect.  Eliminating the other use of the variable entirely and then setting it to infinity seems preferable - <https://lists.libav.org/pipermail/libav-devel/2017-April/083204.html> perhaps?  (It still works to generate GOPs like IDR B P B I B P ... if set manually, but there isn't a switch for that because it isn't useful in the current setup.)

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 070ff5f..7e9c00f 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -1433,7 +1433,7 @@  av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
     ctx->output_order = - ctx->output_delay - 1;
 
     // Currently we never generate I frames, only IDR.
-    ctx->p_per_i = ((avctx->gop_size + avctx->max_b_frames) /
+    ctx->p_per_i = ((avctx->gop_size - 1 + avctx->max_b_frames) /
                     (avctx->max_b_frames + 1));
     ctx->b_per_p = avctx->max_b_frames;