Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate hw_frames_ctx without destroy va_context
On 7/8/2019 2:45 PM, Yan Wang wrote:
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.
Update: the surface isn't destroyed when destroy va_context. But every
va_context maintains one independent map table: m_ddiDecodeCtx->RTtbl.
So the new context cannot find this surface in its map table.
My previous suggested solution should be available still.
Thanks.
Yan Wang
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
---
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_R
Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate hw_frames_ctx without destroy va_context
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
---
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
Re: [FFmpeg-devel] [PATCH] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.
On 5/30/2019 7:39 AM, Mark Thompson wrote:
On 28/05/2019 08:46, Yan Wang wrote:
On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:
On Tue, May 28, 2019 at 8:57 AM Yan Wang wrote:
When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.
Signed-off-by: Yan Wang
---
libavcodec/decode.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
for (;;) {
// Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
hwaccel_uninit(avctx);
+#endif
user_choice = avctx->get_format(avctx, choices);
if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
"missing configuration.\n", desc->name);
goto try_again;
}
+#if CONFIG_VP9_VAAPI_HWACCEL
+ if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
if (hw_config->hwaccel) {
+#endif
av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
"initialisation.\n", desc->name);
err = hwaccel_init(avctx, hw_config);
--
2.17.2
This change feels just wrong. First of all, preprocessors are
absolutely the wrong way to go about this.
Sorry for this. I am new guy for ffmpeg development. What way should be
better? I can refine it.
Secondly, if the frames need to change size, or surface format, then
this absolutely needs to be called, doesn't it?
Based on VP9 spec, the frame resolution can be changed per frame. But current
frame will need refer to previous frame still. So if destroy the VAAPI context,
it
will cause reference frame surface in VAAPI driver lost.
In fact, this patch is for the issue:
https://github.com/intel/media-driver/issues/629
its 2nd frame (128x128) will refer to the 1st frame (256x256).
Can you explain exactly what is going wrong here? The surface is definitely
still present - the reference frame list entry holds a reference to it.
IMHO,
for one VP9 clips with dynamic resolution changing which is legal based
on VP9 spec.
After the 1st frame (256x256) is decoded with vaapi-hw acceleration,
ffmpeg will parse and decode the frame header of the 2nd frame (128x128).
It will call update_size() when ffmpeg finds the resolution is changed
and destroy/recreate vaapi-hw context which will also destroy and
release the previous decoded 1st frame (256x256) saved in driver. The
saved 1st frame surface should be as reference frame normally.
So when ffmpeg ask vaapi-hw to decode the 2nd frame, it cannot find the
the 1st frame surface as reference frame and has to use dummy reference
frame although the reference frame index is right.
I am newbie for ffmpeg code. I am studying ffmpeg related code and hope
to compare libvpx path for looking for a better solution.
Thanks.
Yan Wang
- Mark
___
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".
Re: [FFmpeg-devel] [PATCH] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.
On 5/28/2019 4:43 PM, Hendrik Leppkes wrote:
On Tue, May 28, 2019 at 9:46 AM Yan Wang wrote:
On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:
On Tue, May 28, 2019 at 8:57 AM Yan Wang wrote:
When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.
Signed-off-by: Yan Wang
---
libavcodec/decode.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
for (;;) {
// Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
hwaccel_uninit(avctx);
+#endif
user_choice = avctx->get_format(avctx, choices);
if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
"missing configuration.\n", desc->name);
goto try_again;
}
+#if CONFIG_VP9_VAAPI_HWACCEL
+if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
if (hw_config->hwaccel) {
+#endif
av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
"initialisation.\n", desc->name);
err = hwaccel_init(avctx, hw_config);
--
2.17.2
This change feels just wrong. First of all, preprocessors are
absolutely the wrong way to go about this.
Sorry for this. I am new guy for ffmpeg development. What way should be
better? I can refine it.
Secondly, if the frames need to change size, or surface format, then
this absolutely needs to be called, doesn't it?
Based on VP9 spec, the frame resolution can be changed per frame. But
current
frame will need refer to previous frame still. So if destroy the VAAPI
context, it
will cause reference frame surface in VAAPI driver lost.
In fact, this patch is for the issue:
https://github.com/intel/media-driver/issues/629
its 2nd frame (128x128) will refer to the 1st frame (256x256).
This may work if the frame size decreases, but what if it increases?
Then the frame buffers in the pool are too small, and anything could
go wrong.
This won't be an easy issue to solve, and needs very careful design.
Agree. I should add [RFC] for this patch.
I can investigate frame buffer management of ffmpeg and submit new patch
for covering this situation.
Thanks for comments.
Yan Wang
- Hendrik
___
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".
Re: [FFmpeg-devel] [PATCH] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.
On 5/28/2019 3:16 PM, Hendrik Leppkes wrote:
On Tue, May 28, 2019 at 8:57 AM Yan Wang wrote:
When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.
Signed-off-by: Yan Wang
---
libavcodec/decode.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
for (;;) {
// Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
hwaccel_uninit(avctx);
+#endif
user_choice = avctx->get_format(avctx, choices);
if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
"missing configuration.\n", desc->name);
goto try_again;
}
+#if CONFIG_VP9_VAAPI_HWACCEL
+if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
if (hw_config->hwaccel) {
+#endif
av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
"initialisation.\n", desc->name);
err = hwaccel_init(avctx, hw_config);
--
2.17.2
This change feels just wrong. First of all, preprocessors are
absolutely the wrong way to go about this.
Sorry for this. I am new guy for ffmpeg development. What way should be
better? I can refine it.
Secondly, if the frames need to change size, or surface format, then
this absolutely needs to be called, doesn't it?
Based on VP9 spec, the frame resolution can be changed per frame. But
current
frame will need refer to previous frame still. So if destroy the VAAPI
context, it
will cause reference frame surface in VAAPI driver lost.
In fact, this patch is for the issue:
https://github.com/intel/media-driver/issues/629
its 2nd frame (128x128) will refer to the 1st frame (256x256).
Thanks.
Yan Wang
- Hendrik
___
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".
[FFmpeg-devel] [PATCH] libavcodec/vp9: Fix VP9 dynamic resolution changing decoding on VAAPI.
When the format change, the VAAPI context cannot be destroyed.
Otherwise, the reference frame surface will lost.
Signed-off-by: Yan Wang
---
libavcodec/decode.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6c31166ec2..3eda1dc42c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1397,7 +1397,9 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
for (;;) {
// Remove the previous hwaccel, if there was one.
+#if !CONFIG_VP9_VAAPI_HWACCEL
hwaccel_uninit(avctx);
+#endif
user_choice = avctx->get_format(avctx, choices);
if (user_choice == AV_PIX_FMT_NONE) {
@@ -1479,7 +1481,11 @@ int ff_get_format(AVCodecContext *avctx, const enum
AVPixelFormat *fmt)
"missing configuration.\n", desc->name);
goto try_again;
}
+#if CONFIG_VP9_VAAPI_HWACCEL
+if (hw_config->hwaccel && !avctx->hwaccel) {
+#else
if (hw_config->hwaccel) {
+#endif
av_log(avctx, AV_LOG_DEBUG, "Format %s requires hwaccel "
"initialisation.\n", desc->name);
err = hwaccel_init(avctx, hw_config);
--
2.17.2
___
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".
