On Thu, 2019-06-06 at 15:21 +0800, Wang, Shaofei wrote: > > -----Original Message----- > > From: Xiang, Haihao > > Sent: Thursday, June 6, 2019 11:57 AM > > To: ffmpeg-devel@ffmpeg.org; Wang, Shaofei <shaofei.w...@intel.com> > > Subject: Re: [FFmpeg-devel] [PATCH v2] libavcodec/vp8dec: fix the > > multi-thread HWAccel decode error > > > > On Tue, 2019-06-04 at 15:21 +0800, Wang, Shaofei wrote: > > > > -----Original Message----- > > > > From: Xiang, Haihao > > > > Sent: Tuesday, May 28, 2019 12:23 PM > > > > To: ffmpeg-devel@ffmpeg.org > > > > Cc: Wang, Shaofei <shaofei.w...@intel.com> > > > > Subject: Re: [FFmpeg-devel] [PATCH v2] libavcodec/vp8dec: fix the > > > > multi-thread HWAccel decode error > > > > > > > > On Thu, 2019-03-28 at 13:28 -0400, Shaofei Wang wrote: > > > > > 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; > > > > > > > > s/Dimention/dimension ? > > > > > > OK, should be dimension > > > > > > > BTW this version of patch doesn't address the concern provided when > > > > reviewing the first version of patch. > > > > > > > > When (width != s->avctx->width || height != s->avctx->height) is > > > > true, > > > > ff_set_dimensions() is called even if s->macroblocks_base is not > > > > allocated, so why set dim_reset to (s->macroblocks_base != NULL)? I > > > > think dim_reset should be set to 1. > > > > > > If s->macroblocks_base is available, it means macroblocks_base of the > > > context has been already allocated by one of threads, so it's a reset > > > operation when (width != s->avctx->width... > > > If s->macroblocks_base is null, it's not allocated yet, and (width != > > > s->avctx->width..., just dimension need to be updated but it's not a > > > dim reset operation. Since we only call get_pixel_format() in the > > > first thread or in the reset operation > > > > Is it reasonable when dimension is updated however the low level frame still > > use stale dimension info? > > > > Thanks > > Haihao > > The low level frame, especially hw frame will just need to be updated once. > For example, the init phase of the first thread will call update_dimension and > init hwaccel by get_pixel_format, but not needed to update low level frame for > the follow-up threads.
I get your point, thanks for detailed explanation. _______________________________________________ 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".
