On 7/7/2019 9:49 PM, Fu, Linjie wrote:
-----Original Message-----
From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
Of Mark Thompson
Sent: Sunday, July 7, 2019 19:51
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate
hw_frames_ctx without destroy va_context

On 07/07/2019 17:38, Linjie Fu wrote:
VP9 allows resolution changes per frame. Currently in VAAPI, resolution
changes leads to va context destroy and reinit.
Which is correct - it needs to remake the context because the old one is for
the wrong resolution.
It seems that we don't need to remake context, remaking the surface is enough
for resolution changing.
Currently in libva, surface is able to be recreated separately without remaking 
context.
And this way is used in libyami to cope with resolution changing cases.

                                                 This will cause
reference frame surface lost and produce garbage.
This isn't right - the reference frame surfaces aren't lost.  See
VP9Context.s.refs[], which holds references to the frames (including their
hardware frame contexts) until the stream indicates that they can be
discarded by overwriting their reference frame slots.
I'm not quite sure about this, at least the result shows they are not used 
correctly
in current way.
Will look deeper into it.

In vaapi_vp9_start_frame() of libavcodec/vaapi_vp9.c, it only passes VASurfaceID into pic_param.reference_frames[i].

But when destroy va_context, the surface/render target based on this VASurfaceID has been destroyed.

So the new va_context cannot find the corresponding surface based on this surface ID.

IMHO, one possible solution is to create one the VA surfaces including VP9Context.s.refs[] data which is AVFrame in fact and pass them into libva when re-creating new va_context.

Thanks.

Yan Wang


As libva allows re-create surface separately without changing the
context, this issue could be handled by only recreating hw_frames_ctx.

Could be verified by:
ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
./resolutions.ivf
-pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv

Signed-off-by: Linjie Fu <linjie...@intel.com>
---
  libavcodec/decode.c       |  8 ++++----
  libavcodec/vaapi_decode.c | 40 ++++++++++++++++++++++----------------
--
  2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 0863b82..a81b857 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1254,7 +1254,6 @@ int
ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
      frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;

-
      if (frames_ctx->initial_pool_size) {
          // We guarantee 4 base work surfaces. The function above guarantees
1
          // (the absolute minimum), so add the missing count.
@@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx,
          return AVERROR_PATCHWELCOME;
      }

-    if (hwaccel->priv_data_size) {
+    if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) {
          avctx->internal->hwaccel_priv_data =
              av_mallocz(hwaccel->priv_data_size);
          if (!avctx->internal->hwaccel_priv_data)
@@ -1397,8 +1396,9 @@ int ff_get_format(AVCodecContext *avctx, const
enum AVPixelFormat *fmt)
      for (;;) {
          // Remove the previous hwaccel, if there was one.
-        hwaccel_uninit(avctx);
-
+        // VAAPI allows reinit hw_frames_ctx only
+        if (choices[0] != AV_PIX_FMT_VAAPI_VLD)
Including codec-specific conditions in the generic code suggests that
something very shady is going on.
Yes, will try to avoid this.

+            hwaccel_uninit(avctx);>          user_choice = avctx-
get_format(avctx, choices);
          if (user_choice == AV_PIX_FMT_NONE) {
              // Explicitly chose nothing, give up.
diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
index 69512e1..13f0cf0 100644
--- a/libavcodec/vaapi_decode.c
+++ b/libavcodec/vaapi_decode.c
@@ -613,8 +613,10 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
      VAStatus vas;
      int err;

-    ctx->va_config  = VA_INVALID_ID;
-    ctx->va_context = VA_INVALID_ID;
+    if (!ctx->va_config && !ctx->va_context){
+        ctx->va_config  = VA_INVALID_ID;
+        ctx->va_context = VA_INVALID_ID;
+    }

  #if FF_API_STRUCT_VAAPI_CONTEXT
      if (avctx->hwaccel_context) {
@@ -642,7 +644,6 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
          // present, so set it here to avoid the behaviour changing.
          ctx->hwctx->driver_quirks =
              AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS;
-
      }
  #endif

@@ -655,7 +656,8 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
                 "context: %#x/%#x.\n", ctx->va_config, ctx->va_context);
      } else {
  #endif
-
+    // Get a new hw_frames_ctx even if there is already one
+    // recreate surface without reconstuct va_context
      err = ff_decode_get_hw_frames_ctx(avctx,
AV_HWDEVICE_TYPE_VAAPI);
      if (err < 0)
          goto fail;
@@ -670,21 +672,23 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
      if (err)
          goto fail;

-    vas = vaCreateContext(ctx->hwctx->display, ctx->va_config,
-                          avctx->coded_width, avctx->coded_height,
-                          VA_PROGRESSIVE,
-                          ctx->hwfc->surface_ids,
-                          ctx->hwfc->nb_surfaces,
-                          &ctx->va_context);
-    if (vas != VA_STATUS_SUCCESS) {
-        av_log(avctx, AV_LOG_ERROR, "Failed to create decode "
-               "context: %d (%s).\n", vas, vaErrorStr(vas));
-        err = AVERROR(EIO);
-        goto fail;
-    }
+    if (ctx->va_context == VA_INVALID_ID) {
+        vas = vaCreateContext(ctx->hwctx->display, ctx->va_config,
+                              avctx->coded_width, avctx->coded_height,
+                              VA_PROGRESSIVE,
+                              ctx->hwfc->surface_ids,
+                              ctx->hwfc->nb_surfaces,
+                              &ctx->va_context);
+        if (vas != VA_STATUS_SUCCESS) {
+            av_log(avctx, AV_LOG_ERROR, "Failed to create decode "
+                   "context: %d (%s).\n", vas, vaErrorStr(vas));
+            err = AVERROR(EIO);
+            goto fail;
+        }

-    av_log(avctx, AV_LOG_DEBUG, "Decode context initialised: "
-           "%#x/%#x.\n", ctx->va_config, ctx->va_context);
+        av_log(avctx, AV_LOG_DEBUG, "Decode context initialised: "
+               "%#x/%#x.\n", ctx->va_config, ctx->va_context);
+    }
If I'm reading this correctly, this changes to only creating one context ever
even when the resolution changes.

How can that work if the resolution increases?  For example you start
decoding at 1280x720 and create a context for that, then the resolution
changes to 3840x2160 and it tries to decode using the 1280x720 context.

Recreating hw_frame_ctx can cope with this.
Verified in one case with resolution increasing:
Resolution changes from 495x168 -> 328 x 307 -> 395 x376, the pipeline works
and the output image is good.

As my understanding, you decoding output should be 495x168 always for every frame. Right?

When the resolution of the decoded frame is different with the first frame, the scaling is fired.

The scaling should come from media driver VP pipeline because ffmpeg use vaGetImage() to get decoded frame.

This is dependent on ffmpeg requirement. If ffmpeg hopes to get original decoded frame, this behavior should be changed.

Thanks.

Yan Wang

It's not increasing on both width and height, but as long as one of them 
increases,
current pipeline will be broken ff we don’t reinit the context(or just don't 
recreate
hw_frame_ctx).
(also did experiment as a contrast that we use the same context without recreate
hw_frame_ctx, the pipeline will be blocked when mapping surface in resolution 
increase)

ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -v verbose -i 
./test3232.ivf
-pix_fmt p010le -f rawvideo -vf scale=w=416:h=168 -vframes 10 -y md5.yuv

log is attached.

Thanks.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to