diff mbox

[FFmpeg-devel] lavc/vaapi_encode: reconfigure sequence parameter in case it changes

Message ID 20191113121100.16793-1-linjie.fu@intel.com
State New
Headers show

Commit Message

Fu, Linjie Nov. 13, 2019, 12:11 p.m. UTC
Aspect ratio change should be preserved in the output result of
transcoding. A better choice is to reconfigure when parameter changing
is detected, but a fix to reconfigure on IDR frame is simple and effective
with no performance drop.

Fix #6276 for VAAPI.

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavcodec/vaapi_encode.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Mark Thompson Nov. 17, 2019, 10:59 p.m. UTC | #1
On 13/11/2019 12:11, Linjie Fu wrote:
> Aspect ratio change should be preserved in the output result of
> transcoding. A better choice is to reconfigure when parameter changing
> is detected, but a fix to reconfigure on IDR frame is simple and effective
> with no performance drop.
> 
> Fix #6276 for VAAPI.
> 
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavcodec/vaapi_encode.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 3be9159d37..c202113cb9 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -226,6 +226,12 @@ static int vaapi_encode_issue(AVCodecContext *avctx,
>      pic->nb_param_buffers = 0;
>  
>      if (pic->type == PICTURE_TYPE_IDR && ctx->codec->init_sequence_params) {
> +        err = ctx->codec->init_sequence_params(avctx);
> +        if (err < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Failed to reinit per-sequence "
> +            "parameters: %d.\n", err);
> +            goto fail;
> +        }
>          err = vaapi_encode_make_param_buffer(avctx, pic,
>                                               VAEncSequenceParameterBufferType,
>                                               ctx->codec_sequence_params,
> 

It's wrong to do this in the global header case, because you can't rewrite the parameter sets at the start there.  (Though I guess it could warn that it's ignored the change, like it currently does if you don't have packed header support.)

It might be ok in the freeform bitstream case (like TS or annex B)?  I think it needs a proper reconfigure rather than just blindly calling the initial setup function again, though - e.g. in H.264 it's probably going to want to change the parameter set ids, but on the other hand you don't want to rerun most of the header construction stuff.

- Mark
diff mbox

Patch

diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
index 3be9159d37..c202113cb9 100644
--- a/libavcodec/vaapi_encode.c
+++ b/libavcodec/vaapi_encode.c
@@ -226,6 +226,12 @@  static int vaapi_encode_issue(AVCodecContext *avctx,
     pic->nb_param_buffers = 0;
 
     if (pic->type == PICTURE_TYPE_IDR && ctx->codec->init_sequence_params) {
+        err = ctx->codec->init_sequence_params(avctx);
+        if (err < 0) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to reinit per-sequence "
+            "parameters: %d.\n", err);
+            goto fail;
+        }
         err = vaapi_encode_make_param_buffer(avctx, pic,
                                              VAEncSequenceParameterBufferType,
                                              ctx->codec_sequence_params,