[FFmpeg-devel] [PATCH v2] libavcodec/vp8dec: fix the multi-thread HWAccel decode error
Shaofei Wang
shaofei.wang at intel.com
Thu Mar 28 19:28:13 EET 2019
Fix the issue: https://github.com/intel/media-driver/issues/317
the root cause is update_dimensions will be called multple times
when decoder thread number is not only 1, but update_dimensions
call get_pixel_format in each decode thread will trigger the
hwaccel_uninit/hwaccel_init more than once. But only one hwaccel
should be shared with all decode threads.
in current context,
there are 3 situations in the update_dimensions():
1. First time calling. No matter single thread or multithread,
get_pixel_format() should be called after dimensions were
set;
2. Dimention changed at the runtime. Dimention need to be
updated when macroblocks_base is already allocated,
get_pixel_format() should be called to recreate new frames
according to updated dimention;
3. Multithread first time calling. After decoder init, the
other threads will call update_dimensions() at first time
to allocate macroblocks_base and set dimensions.
But get_pixel_format() is shouldn't be called due to low
level frames and context are already created.
In this fix, we only call update_dimensions as need.
Signed-off-by: Wang, Shaofei <shaofei.wang at intel.com>
Reviewed-by: Jun, Zhao <jun.zhao at intel.com>
Reviewed-by: Haihao Xiang <haihao.xiang at intel.com>
---
Previous code reviews:
2019-03-06 9:25 GMT+01:00, Wang, Shaofei <shaofei.wang at intel.com>:
>> -----Original Message-----
>> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
>> Of Carl Eugen Hoyos
>> Sent: Wednesday, March 6, 2019 3:49 PM
>> To: FFmpeg development discussions and patches
>> <ffmpeg-devel at ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/vp8dec: fix the
>> multi-thread HWAccel decode error
>>
>> 2018-08-09 9:09 GMT+02:00, Jun Zhao <mypopydev at gmail.com>:
>> > the root cause is update_dimentions call get_pixel_format will
>> > trigger the hwaccel_uninit/hwaccel_init , in current context, there
>> > are 3 situations in the update_dimentions():
>> > 1. First time calling. No matter single thread or multithread,
>> > get_pixel_format() should be called after dimentions were
>> > set;
>> > 2. Dimention changed at the runtime. Dimention need to be
>> > updated when macroblocks_base is already allocated,
>> > get_pixel_format() should be called to recreate new frames
>> > according to updated dimention;
>> > 3. Multithread first time calling. After decoder init, the
>> > other threads will call update_dimentions() at first time
>> > to allocate macroblocks_base and set dimentions.
>> > But get_pixel_format() is shouldn't be called due to low
>> > level frames and context are already created.
>> > In this fix, we only call update_dimentions as need.
>> >
>> > Signed-off-by: Wang, Shaofei <shaofei.wang at intel.com>
>> > Reviewed-by: Jun, Zhao <jun.zhao at intel.com>
>> > ---
>> > libavcodec/vp8.c | 7 +++++--
>> > 1 files changed, 5 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c index
>> > 3adfeac..18d1ada 100644
>> > --- a/libavcodec/vp8.c
>> > +++ b/libavcodec/vp8.c
>> > @@ -187,7 +187,7 @@ static av_always_inline int
>> > update_dimensions(VP8Context *s, int width, int height, int is_vp7) {
>> > AVCodecContext *avctx = s->avctx;
>> > - int i, ret;
>> > + int i, ret, dim_reset = 0;
>> >
>> > if (width != s->avctx->width || ((width+15)/16 != s->mb_width
>> > ||
>> > (height+15)/16 != s->mb_height) && s->macroblocks_base ||
>> > height != s->avctx->height) { @@ -196,9 +196,12 @@ int
>> > update_dimensions(VP8Context *s, int width, int height, int is_vp7)
>> > ret = ff_set_dimensions(s->avctx, width, height);
>> > if (ret < 0)
>> > return ret;
>> > +
>> > + dim_reset = (s->macroblocks_base != NULL);
>> > }
>> >
>> > - if (!s->actually_webp && !is_vp7) {
>> > + if ((s->pix_fmt == AV_PIX_FMT_NONE || dim_reset) &&
>> > + !s->actually_webp && !is_vp7) {
>>
>> Why is the new variable dim_reset needed?
>> Wouldn't the patch be simpler if you used s->macroblocks_base here?
> Since dim_reset was set in the "if" segment, it equal to (width !=
> s->avctx->width || ((width+15)/16 != s->mb_width ||
> (height+15)/16 != s->mb_height) || height != s->avctx->height) &&
> s->macroblocks_base
Thank you!
Carl Eugen
libavcodec/vp8.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index ba79e5f..0a7f38b 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -187,7 +187,7 @@ static av_always_inline
int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
{
AVCodecContext *avctx = s->avctx;
- int i, ret;
+ int i, ret, dim_reset = 0;
if (width != s->avctx->width || ((width+15)/16 != s->mb_width || (height+15)/16 != s->mb_height) && s->macroblocks_base ||
height != s->avctx->height) {
@@ -196,9 +196,12 @@ int update_dimensions(VP8Context *s, int width, int height, int is_vp7)
ret = ff_set_dimensions(s->avctx, width, height);
if (ret < 0)
return ret;
+
+ dim_reset = (s->macroblocks_base != NULL);
}
- if (!s->actually_webp && !is_vp7) {
+ if ((s->pix_fmt == AV_PIX_FMT_NONE || dim_reset) &&
+ !s->actually_webp && !is_vp7) {
s->pix_fmt = get_pixel_format(s);
if (s->pix_fmt < 0)
return AVERROR(EINVAL);
--
1.8.3.1
More information about the ffmpeg-devel
mailing list