[FFmpeg-devel] [PATCH v2] libavcodec/vp8dec: fix the multi-thread HWAccel decode error

[Wang, Shaofei]

> -----Original Message-----
> From: Xiang, Haihao
> Sent: Thursday, June 6, 2019 11:57 AM
> To: ffmpeg-devel at ffmpeg.org; Wang, Shaofei <shaofei.wang at 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 at ffmpeg.org
> > > Cc: Wang, Shaofei <shaofei.wang at 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.