[FFmpeg-devel] [PATCH v1] avcodec/vaapi_av1: correct data size when create slice data buffer

[Wang, Fei W]

On Tue, 2021-05-18 at 12:26 +0300, Jan Ekström wrote:
> On Tue, May 18, 2021 at 5:55 AM Wang, Fei W <fei.w.wang at intel.com>
> wrote:
> > 
> > On Mon, 2021-05-17 at 11:59 +0300, Jan Ekström wrote:
> > > On Mon, May 17, 2021 at 4:50 AM Fei Wang <fei.w.wang at intel.com>
> > > wrote:
> > > > 
> > > > Set all tiles size to create slice data buffer, hardware will
> > > > use
> > > > slice_data_offset/slice_data_size in slice parameter buffer to
> > > > get
> > > > each tile's data.
> > > > 
> > > > This change will let it success to decode clip which has multi
> > > > tiles data inside one OBU.
> > > > 
> > > > Signed-off-by: Fei Wang <fei.w.wang at intel.com>
> > > > ---
> > > >  libavcodec/vaapi_av1.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/libavcodec/vaapi_av1.c b/libavcodec/vaapi_av1.c
> > > > index 1809b485aa..16b7e35747 100644
> > > > --- a/libavcodec/vaapi_av1.c
> > > > +++ b/libavcodec/vaapi_av1.c
> > > > @@ -292,7 +292,7 @@ static int
> > > > vaapi_av1_decode_slice(AVCodecContext *avctx,
> > > >          err = ff_vaapi_decode_make_slice_buffer(avctx, pic,
> > > > &slice_param,
> > > >                                                  sizeof(VASlice
> > > > Para
> > > > meterBufferAV1),
> > > >                                                  buffer,
> > > > -                                                s-
> > > > > tile_group_info[i].tile_size);
> > > > 
> > > > +                                                size);
> > > >          if (err) {
> > > >              ff_vaapi_decode_cancel(avctx, pic);
> > > >              return err;
> > > > --
> > > > 2.17.1
> > > 
> > > So basically this fixes setting the size of the passed buffer to
> > > the
> > > whole size of the buffer (in which tile offset and tile size
> > > should
> > > always be located). As such it makes sense.
> > > 
> > > It seems like at some point there was an idea to only pass
> > > "buffer +
> > > tile_offset", but that never materialized completely (and then
> > > setting
> > > VASliceParameterBufferAV1's tile_offset to 0)? Would that lead to
> > > smaller buffers being utilized? Or does libva already extract the
> > > tiles into their own buffers based on the information?
> > 
> > Buffer+tile_offset can fix the issue too. I chose change the size
> > only
> > is to try to follow current logic and do the smallest change to fix
> > this.
> > 
> 
> Yes, and as I noted it makes sense. It was just interesting to see
> how
> these two things seemed to be intermixed.
> 
> Functionally this patch is LGTM, maybe something a la
> 
> """
> avcodec/vaapi_av1: pass full buffer size for each tile
> 
> Previously, only the size of a given tile was passed, making the
> offset
> and size marked in VASliceParameterBufferAV1 invalid with multiple
> tiles.
> """
> 
> for the commit message?

Thanks, I can update the commit message like this.

> 
> This should probably then also be back-ported to release/4.4 as I
> think 3308bbf7761a2723f784fcd6bb921dcbadc18c6e (original addition of
> the vaapi AV1 hwaccel is included in it).

Yes, this is a important fix. Appreciate if you who has write access
can help back-port it to release/4.4.(The patch can be cherry-pick to
release/4.4 without any conflict)

Fei
Thanks

> 
> Thanks.
> 
> > With buffer+tile_offset way, the buffers will be smaller, but the
> > effect can be ignore, since bitstream size is small. For 8K
> > resolution
> > bitstream, its each frame size is only ~100KB.
> > 
> > Fei
> 
> Yup. Of course this depends on the bit rate of the stream, but I do
> not consider this a required change.
> 
> Jan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".