From patchwork Thu Mar 28 17:28:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shaofei Wang X-Patchwork-Id: 12507 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id D9DED44865F for ; Thu, 28 Mar 2019 06:29:30 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B8338689D26; Thu, 28 Mar 2019 06:29:30 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 40997689C22 for ; Thu, 28 Mar 2019 06:29:23 +0200 (EET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Mar 2019 21:29:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,278,1549958400"; d="scan'208";a="129320668" Received: from unknown (HELO localhost.localdomain.bj.intel.com) ([172.16.113.186]) by orsmga008.jf.intel.com with ESMTP; 27 Mar 2019 21:29:20 -0700 From: Shaofei Wang To: ffmpeg-devel@ffmpeg.org Date: Thu, 28 Mar 2019 13:28:13 -0400 Message-Id: <1553794093-19942-1-git-send-email-shaofei.wang@intel.com> X-Mailer: git-send-email 1.8.3.1 Subject: [FFmpeg-devel] [PATCH v2] libavcodec/vp8dec: fix the multi-thread HWAccel decode error X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Shaofei Wang MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 Reviewed-by: Jun, Zhao Reviewed-by: Haihao Xiang --- Previous code reviews: 2019-03-06 9:25 GMT+01:00, Wang, Shaofei : >> -----Original Message----- >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf >> Of Carl Eugen Hoyos >> Sent: Wednesday, March 6, 2019 3:49 PM >> To: FFmpeg development discussions and patches >> >> Subject: Re: [FFmpeg-devel] [PATCH] libavcodec/vp8dec: fix the >> multi-thread HWAccel decode error >> >> 2018-08-09 9:09 GMT+02:00, Jun Zhao : >> > 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 >> > Reviewed-by: Jun, Zhao >> > --- >> > 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);